Skip to content

Commit 3f96faa

Browse files
committed
Enrich skills data for PUT / PATCH response and add blocks on review type phases to ensure they can't be manually opened without at least 1 resource assigned to the review type role associated with the phase type.
1 parent 80170e9 commit 3f96faa

File tree

3 files changed

+158
-0
lines changed

3 files changed

+158
-0
lines changed

src/services/ChallengePhaseService.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ const REVIEW_PHASE_NAMES = Object.freeze([
2424
"approval",
2525
]);
2626
const REVIEW_PHASE_NAME_SET = new Set(REVIEW_PHASE_NAMES.map((name) => name.toLowerCase()));
27+
const PHASE_RESOURCE_ROLE_REQUIREMENTS = Object.freeze({
28+
"iterative review": "Iterative Reviewer",
29+
"checkpoint screening": "Checkpoint Screener",
30+
screening: "Screener",
31+
review: "Reviewer",
32+
"checkpoint review": "Checkpoint Reviewer",
33+
});
2734

2835
async function hasPendingScorecardsForPhase(challengePhaseId) {
2936
if (!config.REVIEW_DB_URL) {
@@ -201,6 +208,48 @@ async function postChallengeUpdatedNotification(challengeId) {
201208
}
202209
}
203210

211+
async function ensureRequiredResourcesBeforeOpeningPhase(challengeId, phaseName) {
212+
const normalizedPhaseName = _.toLower(_.trim(phaseName || ""));
213+
const requiredRoleName = PHASE_RESOURCE_ROLE_REQUIREMENTS[normalizedPhaseName];
214+
if (!requiredRoleName) {
215+
return;
216+
}
217+
218+
const challengeResources = await helper.getChallengeResources(challengeId);
219+
const requiredRoleNameLower = _.toLower(requiredRoleName);
220+
const hasRequiredRoleByName = (challengeResources || []).some((resource) => {
221+
const roleName =
222+
resource.roleName ||
223+
resource.role ||
224+
_.get(resource, "role.name") ||
225+
_.get(resource, "resourceRoleName") ||
226+
_.get(resource, "resourceRole.name");
227+
return roleName && _.toLower(roleName) === requiredRoleNameLower;
228+
});
229+
230+
let hasRequiredRole = hasRequiredRoleByName;
231+
232+
if (!hasRequiredRole) {
233+
const resourceRoles = await helper.getResourceRoles();
234+
const requiredRoleIds = (resourceRoles || [])
235+
.filter((role) => role.name && _.toLower(role.name) === requiredRoleNameLower)
236+
.map((role) => _.toString(role.id));
237+
if (requiredRoleIds.length > 0) {
238+
const requiredRoleIdSet = new Set(requiredRoleIds);
239+
hasRequiredRole = (challengeResources || []).some(
240+
(resource) => resource.roleId && requiredRoleIdSet.has(_.toString(resource.roleId))
241+
);
242+
}
243+
}
244+
245+
if (!hasRequiredRole) {
246+
const displayPhaseName = phaseName || "phase";
247+
throw new errors.BadRequestError(
248+
`Cannot open ${displayPhaseName} phase because the challenge does not have any resource with the ${requiredRoleName} role`
249+
);
250+
}
251+
}
252+
204253
/**
205254
* Get all phase information for that challenge
206255
* @param {String} challengeId the challenge id
@@ -330,6 +379,11 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data)
330379
}
331380
}
332381

382+
if (isOpeningPhase) {
383+
const phaseName = data.name || challengePhase.name;
384+
await ensureRequiredResourcesBeforeOpeningPhase(challengeId, phaseName);
385+
}
386+
333387
if (data["scheduledStartDate"] || data["scheduledEndDate"]) {
334388
const startDate = data["scheduledStartDate"] || challengePhase.scheduledStartDate;
335389
const endDate = data["scheduledEndDate"] || challengePhase.scheduledEndDate;

src/services/ChallengeService.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,7 @@ async function createChallenge(currentUser, challenge, userToken) {
15151515
// No conversion needed - values are already in dollars in the database
15161516

15171517
prismaHelper.convertModelToResponse(ret);
1518+
await enrichSkillsData(ret);
15181519
enrichChallengeForResponse(ret, track, type);
15191520

15201521
// If the challenge is self-service, add the creating user as the "client manager", *not* the manager
@@ -2683,6 +2684,7 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) {
26832684

26842685
// Convert to response shape before any business-logic checks that expect it
26852686
prismaHelper.convertModelToResponse(updatedChallenge);
2687+
await enrichSkillsData(updatedChallenge);
26862688
enrichChallengeForResponse(updatedChallenge);
26872689

26882690
if (_.get(updatedChallenge, "legacy.selfService")) {

test/unit/ChallengePhaseService.test.js

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const uuid = require('uuid/v4')
1313
const { getReviewClient } = require('../../src/common/review-prisma')
1414
const prisma = require('../../src/common/prisma').getClient()
1515
const service = require('../../src/services/ChallengePhaseService')
16+
const helper = require('../../src/common/helper')
1617
const testHelper = require('../testHelper')
1718

1819
const should = chai.should()
@@ -802,6 +803,107 @@ describe('challenge phase service unit tests', () => {
802803
const challengePhase = await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, { isOpen: true })
803804
should.equal(challengePhase.isOpen, true)
804805
})
806+
807+
it('partially update challenge phase - cannot open review phase without reviewer resource', async () => {
808+
const reviewPhase = await prisma.phase.create({
809+
data: {
810+
id: uuid(),
811+
name: 'Review',
812+
description: 'desc',
813+
isOpen: false,
814+
duration: 86400,
815+
createdBy: 'admin',
816+
updatedBy: 'admin'
817+
}
818+
})
819+
const reviewChallengePhaseId = uuid()
820+
await prisma.challengePhase.create({
821+
data: {
822+
id: reviewChallengePhaseId,
823+
challengeId: data.challenge.id,
824+
phaseId: reviewPhase.id,
825+
name: 'Review',
826+
isOpen: false,
827+
createdBy: 'admin',
828+
updatedBy: 'admin'
829+
}
830+
})
831+
832+
const originalGetChallengeResources = helper.getChallengeResources
833+
const originalGetResourceRoles = helper.getResourceRoles
834+
helper.getChallengeResources = async () => ([
835+
{ roleId: 'some-other-role-id' }
836+
])
837+
helper.getResourceRoles = async () => ([
838+
{ id: 'reviewer-role-id', name: 'Reviewer' }
839+
])
840+
841+
try {
842+
await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, reviewChallengePhaseId, { isOpen: true })
843+
} catch (e) {
844+
should.equal(e.httpStatus || e.statusCode, 400)
845+
should.equal(
846+
e.message,
847+
'Cannot open Review phase because the challenge does not have any resource with the Reviewer role'
848+
)
849+
return
850+
} finally {
851+
helper.getChallengeResources = originalGetChallengeResources
852+
helper.getResourceRoles = originalGetResourceRoles
853+
await prisma.challengePhase.delete({ where: { id: reviewChallengePhaseId } })
854+
await prisma.phase.delete({ where: { id: reviewPhase.id } })
855+
}
856+
857+
throw new Error('should not reach here')
858+
})
859+
860+
it('partially update challenge phase - opens review phase when reviewer resource exists', async () => {
861+
const reviewPhase = await prisma.phase.create({
862+
data: {
863+
id: uuid(),
864+
name: 'Review',
865+
description: 'desc',
866+
isOpen: false,
867+
duration: 86400,
868+
createdBy: 'admin',
869+
updatedBy: 'admin'
870+
}
871+
})
872+
const reviewChallengePhaseId = uuid()
873+
await prisma.challengePhase.create({
874+
data: {
875+
id: reviewChallengePhaseId,
876+
challengeId: data.challenge.id,
877+
phaseId: reviewPhase.id,
878+
name: 'Review',
879+
isOpen: false,
880+
createdBy: 'admin',
881+
updatedBy: 'admin'
882+
}
883+
})
884+
885+
const originalGetChallengeResources = helper.getChallengeResources
886+
const originalGetResourceRoles = helper.getResourceRoles
887+
helper.getChallengeResources = async () => ([
888+
{
889+
roleId: 'reviewer-role-id',
890+
resourceRole: { name: 'Reviewer' }
891+
}
892+
])
893+
helper.getResourceRoles = async () => ([
894+
{ id: 'reviewer-role-id', name: 'Reviewer' }
895+
])
896+
897+
try {
898+
const challengePhase = await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, reviewChallengePhaseId, { isOpen: true })
899+
should.equal(challengePhase.isOpen, true)
900+
} finally {
901+
helper.getChallengeResources = originalGetChallengeResources
902+
helper.getResourceRoles = originalGetResourceRoles
903+
await prisma.challengePhase.delete({ where: { id: reviewChallengePhaseId } })
904+
await prisma.phase.delete({ where: { id: reviewPhase.id } })
905+
}
906+
})
805907
})
806908

807909
describe('delete challenge phase tests', () => {

0 commit comments

Comments
 (0)