Merge pull request #55 from topcoder-platform/reviewer_checks

jmgasper · web-flow · commit 19eb10dd9344 · 2025-11-25T11:11:20.000+11:00
Additional checks for reviewer setup before allowing activation (PM-3…
diff --git a/config/default.js b/config/default.js
@@ -49,7 +49,7 @@ module.exports = {
   // TODO: change this to localhost
   RESOURCE_ROLES_API_URL:
     process.env.RESOURCE_ROLES_API_URL || "http://api.topcoder-dev.com/v5/resource-roles",
-  GROUPS_API_URL: process.env.GROUPS_API_URL || "http://localhost:4000/v5/groups",
+  GROUPS_API_URL: process.env.GROUPS_API_URL || "http://localhost:4000/v6/groups",
   PROJECTS_API_URL: process.env.PROJECTS_API_URL || "http://localhost:4000/v5/projects",
   TERMS_API_URL: process.env.TERMS_API_URL || "http://localhost:4000/v5/terms",
   CUSTOMER_PAYMENTS_URL:
diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js
@@ -73,6 +73,7 @@ const REVIEW_PHASE_NAMES = Object.freeze([
   "approval",
 ]);
 const REVIEW_PHASE_NAME_SET = new Set(REVIEW_PHASE_NAMES);
+const REQUIRED_REVIEW_PHASE_NAME_SET = new Set([...REVIEW_PHASE_NAMES, "iterative review"]);
 
 /**
  * Enrich skills data with full details from standardized skills API.
@@ -2356,6 +2357,8 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) {
   validateTask(currentUser, challenge, data, challengeResources);
   const taskCompletionInfo = prepareTaskCompletionData(challenge, challengeResources, data);
 
+  const isStatusChangingToActive =
+    data.status === ChallengeStatusEnum.ACTIVE && challenge.status !== ChallengeStatusEnum.ACTIVE;
   let sendActivationEmail = false;
   let sendSubmittedEmail = false;
   let sendCompletedEmail = false;
@@ -2459,8 +2462,6 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) {
 
   /* END self-service stuffs */
 
-  const isStatusChangingToActive =
-    data.status === ChallengeStatusEnum.ACTIVE && challenge.status !== ChallengeStatusEnum.ACTIVE;
   let isChallengeBeingActivated = isStatusChangingToActive;
   let isChallengeBeingCancelled = false;
   if (data.status) {
@@ -2806,6 +2807,57 @@ async function updateChallenge(currentUser, challengeId, data, options = {}) {
     });
   }
 
+  if (
+    isStatusChangingToActive &&
+    (challenge.status === ChallengeStatusEnum.NEW || challenge.status === ChallengeStatusEnum.DRAFT)
+  ) {
+    const effectiveReviewers = Array.isArray(data.reviewers)
+      ? data.reviewers
+      : Array.isArray(challenge.reviewers)
+      ? challenge.reviewers
+      : [];
+    const reviewerPhaseIds = new Set(
+      effectiveReviewers
+        .filter((reviewer) => reviewer && reviewer.phaseId)
+        .map((reviewer) => String(reviewer.phaseId))
+    );
+
+    if (reviewerPhaseIds.size === 0) {
+      throw new errors.BadRequestError(
+        "Cannot activate a challenge without at least one reviewer configured"
+      );
+    }
+
+    const normalizePhaseName = (name) => String(name || "").trim().toLowerCase();
+    const effectivePhases =
+      (Array.isArray(phasesForUpdate) && phasesForUpdate.length > 0
+        ? phasesForUpdate
+        : challenge.phases) || [];
+
+    const missingPhaseNames = new Set();
+    for (const phase of effectivePhases) {
+      if (!phase) {
+        continue;
+      }
+      const normalizedName = normalizePhaseName(phase.name);
+      if (!REQUIRED_REVIEW_PHASE_NAME_SET.has(normalizedName)) {
+        continue;
+      }
+      const phaseId = _.get(phase, "phaseId");
+      if (!phaseId || !reviewerPhaseIds.has(String(phaseId))) {
+        missingPhaseNames.add(phase.name || "Unknown phase");
+      }
+    }
+
+    if (missingPhaseNames.size > 0) {
+      throw new errors.BadRequestError(
+        `Cannot activate challenge; missing reviewers for phase(s): ${Array.from(
+          missingPhaseNames
+        ).join(", ")}`
+      );
+    }
+  }
+
   // convert data to prisma models
   const updateData = prismaHelper.convertChallengeSchemaToPrisma(
     currentUser,
diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js
@@ -672,6 +672,152 @@ describe('challenge service unit tests', () => {
   })
 
   describe('update challenge tests', () => {
+    const ensureSkipTimelineTemplate = async () => {
+      const templateId = config.SKIP_PROJECT_ID_BY_TIMLINE_TEMPLATE_ID
+      let template = await prisma.timelineTemplate.findUnique({ where: { id: templateId } })
+      if (!template) {
+        template = await prisma.timelineTemplate.create({
+          data: {
+            id: templateId,
+            name: `skip-template-${templateId}`,
+            description: 'Template used to bypass project requirements in activation tests',
+            isActive: true,
+            createdBy: 'activation-test',
+            updatedBy: 'activation-test'
+          }
+        })
+      }
+
+      const existingPhases = await prisma.timelineTemplatePhase.findMany({
+        where: { timelineTemplateId: templateId }
+      })
+      const existingPhaseIds = new Set(existingPhases.map(p => p.phaseId))
+      const phaseRows = []
+      if (!existingPhaseIds.has(data.phase.id)) {
+        phaseRows.push({
+          timelineTemplateId: templateId,
+          phaseId: data.phase.id,
+          defaultDuration: 1000,
+          createdBy: 'activation-test',
+          updatedBy: 'activation-test'
+        })
+      }
+      if (!existingPhaseIds.has(data.phase2.id)) {
+        phaseRows.push({
+          timelineTemplateId: templateId,
+          phaseId: data.phase2.id,
+          predecessor: data.phase.id,
+          defaultDuration: 1000,
+          createdBy: 'activation-test',
+          updatedBy: 'activation-test'
+        })
+      }
+      if (phaseRows.length > 0) {
+        await prisma.timelineTemplatePhase.createMany({ data: phaseRows })
+      }
+      return templateId
+    }
+
+    const createChallengeWithRequiredReviewPhases = async () => {
+      await ensureSkipTimelineTemplate()
+
+      const phaseNames = ['Screening', 'Review']
+      const createdPhaseIds = []
+      const phaseRecords = []
+
+      for (const phaseName of phaseNames) {
+        let phaseRecord = await prisma.phase.findFirst({ where: { name: phaseName } })
+        if (!phaseRecord) {
+          phaseRecord = await prisma.phase.create({
+            data: {
+              id: uuid(),
+              name: phaseName,
+              description: `${phaseName} phase`,
+              isOpen: false,
+              duration: 86400,
+              createdBy: 'activation-test',
+              updatedBy: 'activation-test'
+            }
+          })
+          createdPhaseIds.push(phaseRecord.id)
+        }
+        phaseRecords.push(phaseRecord)
+      }
+
+      const challenge = await prisma.challenge.create({
+        data: {
+          id: uuid(),
+          name: `Activation coverage ${Date.now()}`,
+          description: 'Activation coverage test',
+          typeId: data.challenge.typeId,
+          trackId: data.challenge.trackId,
+          timelineTemplateId: config.SKIP_PROJECT_ID_BY_TIMLINE_TEMPLATE_ID,
+          startDate: new Date(),
+          status: ChallengeStatusEnum.DRAFT,
+          tags: [],
+          groups: [],
+          createdBy: 'activation-test',
+          updatedBy: 'activation-test'
+        }
+      })
+
+      const challengePhaseIds = []
+      await prisma.challengePhase.createMany({
+        data: phaseRecords.map((phase) => {
+          const cpId = uuid()
+          challengePhaseIds.push(cpId)
+          return {
+            id: cpId,
+            challengeId: challenge.id,
+            phaseId: phase.id,
+            name: phase.name,
+            duration: phase.duration || 86400,
+            isOpen: false,
+            createdBy: 'activation-test',
+            updatedBy: 'activation-test'
+          }
+        })
+      })
+
+      return { challenge, phaseRecords, createdPhaseIds, challengePhaseIds }
+    }
+
+    const cleanupChallengeWithRequiredReviewPhases = async ({
+      challenge,
+      createdPhaseIds = [],
+      challengePhaseIds = []
+    }) => {
+      if (challengePhaseIds.length > 0) {
+        await prisma.challengePhase.deleteMany({ where: { id: { in: challengePhaseIds } } })
+      }
+      if (challenge) {
+        await prisma.challenge.delete({ where: { id: challenge.id } })
+      }
+      if (createdPhaseIds.length > 0) {
+        await prisma.phase.deleteMany({ where: { id: { in: createdPhaseIds } } })
+      }
+    }
+
+    const createActivationChallenge = async (status = ChallengeStatusEnum.NEW) => {
+      const timelineTemplateId = await ensureSkipTimelineTemplate()
+      return prisma.challenge.create({
+        data: {
+          id: uuid(),
+          name: `Activation reviewer check ${Date.now()}`,
+          description: 'activation reviewer check',
+          typeId: data.challenge.typeId,
+          trackId: data.challenge.trackId,
+          timelineTemplateId,
+          startDate: new Date(),
+          status,
+          tags: [],
+          groups: [],
+          createdBy: 'activation-test',
+          updatedBy: 'activation-test'
+        }
+      })
+    }
+
     it('update challenge successfully 1', async () => {
       const challengeData = testChallengeData
       const result = await service.updateChallenge({ isMachine: true, sub: 'sub3', userId: 22838965 }, id, {
@@ -1128,6 +1274,107 @@ describe('challenge service unit tests', () => {
       throw new Error('should not reach here')
     })
 
+    it('update challenge - prevent activating without reviewers', async () => {
+      const activationChallenge = await createActivationChallenge(ChallengeStatusEnum.DRAFT)
+      try {
+        await service.updateChallenge({ isMachine: true, sub: 'sub-activate' }, activationChallenge.id, {
+          status: ChallengeStatusEnum.ACTIVE
+        })
+      } catch (e) {
+        should.equal(e.message.indexOf('reviewer configured') >= 0, true)
+        return
+      } finally {
+        await prisma.challenge.delete({ where: { id: activationChallenge.id } })
+      }
+      throw new Error('should not reach here')
+    })
+
+    it('update challenge - allow activating with reviewers provided', async () => {
+      const activationChallenge = await createActivationChallenge()
+      try {
+        const updated = await service.updateChallenge(
+          { isMachine: true, sub: 'sub-activate', userId: 22838965 },
+          activationChallenge.id,
+          {
+            status: ChallengeStatusEnum.ACTIVE,
+            reviewers: [
+              {
+                phaseId: data.phase.id,
+                scorecardId: 'activation-scorecard',
+                isMemberReview: false,
+                aiWorkflowId: 'workflow-123'
+              }
+            ]
+          }
+        )
+        should.equal(updated.status, ChallengeStatusEnum.ACTIVE)
+        should.exist(updated.reviewers)
+        should.equal(updated.reviewers.length, 1)
+        should.equal(updated.reviewers[0].scorecardId, 'activation-scorecard')
+      } finally {
+        await prisma.challenge.delete({ where: { id: activationChallenge.id } })
+      }
+    })
+
+    it('update challenge - enforce reviewers for required phases', async () => {
+      const setup = await createChallengeWithRequiredReviewPhases()
+      try {
+        await service.updateChallenge(
+          { isMachine: true, sub: 'sub-activate', userId: 22838965 },
+          setup.challenge.id,
+          {
+            status: ChallengeStatusEnum.ACTIVE,
+            reviewers: [
+              {
+                phaseId: setup.phaseRecords[0].id,
+                scorecardId: 'screening-scorecard',
+                isMemberReview: false,
+                aiWorkflowId: 'workflow-screening'
+              }
+            ]
+          }
+        )
+      } catch (e) {
+        should.equal(e.message.indexOf('missing reviewers for phase(s): Review') >= 0, true)
+        return
+      } finally {
+        await cleanupChallengeWithRequiredReviewPhases(setup)
+      }
+      throw new Error('should not reach here')
+    })
+
+    it('update challenge - allow activation when required phases have reviewers', async () => {
+      const setup = await createChallengeWithRequiredReviewPhases()
+      try {
+        const updated = await service.updateChallenge(
+          { isMachine: true, sub: 'sub-activate', userId: 22838965 },
+          setup.challenge.id,
+          {
+            status: ChallengeStatusEnum.ACTIVE,
+            reviewers: [
+              {
+                phaseId: setup.phaseRecords[0].id,
+                scorecardId: 'screening-scorecard',
+                isMemberReview: false,
+                aiWorkflowId: 'workflow-screening'
+              },
+              {
+                phaseId: setup.phaseRecords[1].id,
+                scorecardId: 'review-scorecard',
+                isMemberReview: false,
+                aiWorkflowId: 'workflow-review'
+              }
+            ]
+          }
+        )
+        should.equal(updated.status, ChallengeStatusEnum.ACTIVE)
+        should.exist(updated.reviewers)
+        should.equal(updated.reviewers.length, 2)
+      } finally {
+        await cleanupChallengeWithRequiredReviewPhases(setup)
+      }
+    })
+
     it('update challenge - set winners with non-completed Active status', async () => {
       try {
         await service.updateChallenge({ isMachine: true, sub: 'sub3' }, id, {
