Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/services/ChallengeService.js
Original file line number Diff line number Diff line change
Expand Up @@ -2816,6 +2816,30 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) {
: Array.isArray(challenge.reviewers)
? challenge.reviewers
: [];

const reviewersMissingFields = [];
effectiveReviewers.forEach((reviewer, index) => {
const hasScorecardId =
reviewer && !_.isNil(reviewer.scorecardId) && String(reviewer.scorecardId).trim() !== "";
const hasPhaseId =
reviewer && !_.isNil(reviewer.phaseId) && String(reviewer.phaseId).trim() !== "";

if (!hasScorecardId || !hasPhaseId) {
const missing = [];
if (!hasScorecardId) missing.push("scorecardId");
if (!hasPhaseId) missing.push("phaseId");
reviewersMissingFields.push(`reviewer[${index}] missing ${missing.join(" and ")}`);
}
});

if (reviewersMissingFields.length > 0) {
throw new errors.BadRequestError(
`Cannot activate challenge; reviewers are missing required fields: ${reviewersMissingFields.join(
"; "
)}`
);
}

const reviewerPhaseIds = new Set(
effectiveReviewers
.filter((reviewer) => reviewer && reviewer.phaseId)
Expand Down
32 changes: 32 additions & 0 deletions test/unit/ChallengeService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,38 @@ describe('challenge service unit tests', () => {
}
})

it('update challenge - prevent activating when reviewer is missing required fields', async () => {
const activationChallenge = await createActivationChallenge()
await prisma.challengeReviewer.create({
data: {
id: uuid(),
challengeId: activationChallenge.id,
scorecardId: '',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The scorecardId is set to an empty string. Ensure that this is intentional and that the system can handle an empty scorecardId without causing issues. If a scorecardId is required, consider adding validation to prevent empty values.

isMemberReview: false,
phaseId: data.phase.id,
aiWorkflowId: 'wf-missing',
createdBy: 'activation-test',
updatedBy: 'activation-test'
}
})

try {
await service.updateChallenge(
{ isMachine: true, sub: 'sub-activate', userId: 22838965 },
activationChallenge.id,
{
status: ChallengeStatusEnum.ACTIVE
}
)
} catch (e) {
should.equal(e.message.indexOf('reviewers are missing required fields') >= 0, true)
return
} finally {
await prisma.challenge.delete({ where: { id: activationChallenge.id } })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The finally block deletes the activationChallenge regardless of whether the catch block is executed. Ensure that this deletion is intended in all cases, as it may affect test isolation or subsequent tests.

}
throw new Error('should not reach here')
})

it('update challenge - enforce reviewers for required phases', async () => {
const setup = await createChallengeWithRequiredReviewPhases()
try {
Expand Down
Loading