Block reopen submissions and changing review scorecard ID if there are already reviews being worked on

jmgasper · jmgasper · commit 20261d43aa15 · 2025-10-23T11:37:16.000+11:00
diff --git a/src/services/ChallengePhaseService.js b/src/services/ChallengePhaseService.js
@@ -16,6 +16,14 @@ const { indexChallengeAndPostToKafka } = require("./ChallengeService");
 const { getClient } = require("../common/prisma");
 const prisma = getClient();
 const PENDING_REVIEW_STATUSES = Object.freeze(["PENDING", "IN_PROGRESS", "DRAFT", "SUBMITTED"]);
+const REVIEW_PHASE_NAMES = Object.freeze([
+  "checkpoint review",
+  "checkpoint screening",
+  "screening",
+  "review",
+  "approval",
+]);
+const REVIEW_PHASE_NAME_SET = new Set(REVIEW_PHASE_NAMES.map((name) => name.toLowerCase()));
 
 async function hasPendingScorecardsForPhase(challengePhaseId) {
   if (!config.REVIEW_DB_URL) {
@@ -370,6 +378,31 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data)
   }
   if (isReopeningPhase) {
     const phaseName = challengePhase.name;
+    const openPhases = await prisma.challengePhase.findMany({
+      where: {
+        challengeId: challengePhase.challengeId,
+        isOpen: true,
+      },
+      select: {
+        id: true,
+        phaseId: true,
+        predecessor: true,
+        name: true,
+      },
+    });
+    const activeReviewPhase = openPhases.find((phase) =>
+      REVIEW_PHASE_NAME_SET.has(String(phase?.name || "").toLowerCase())
+    );
+    if (activeReviewPhase) {
+      const hasActiveScorecards = await hasCompletedReviewsForPhase(
+        activeReviewPhase.id
+      );
+      if (hasActiveScorecards) {
+        throw new errors.BadRequestError(
+          `Cannot reopen ${phaseName} because the currently open phase '${activeReviewPhase.name}' has reviews in progress or completed`
+        );
+      }
+    }
     if (phaseName === "Submission" || phaseName === "Registration") {
       const hasCompletedReviews = await hasCompletedReviewsForPhase(challengePhase.id);
       if (hasCompletedReviews) {
@@ -401,50 +434,36 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data)
     const hasActualStartDate = !_.isNil(challengePhase.actualStartDate);
     const hasActualEndDate = !_.isNil(challengePhase.actualEndDate);
 
-    if (hasActualStartDate && hasActualEndDate) {
-      const openPhases = await prisma.challengePhase.findMany({
-        where: {
-          challengeId: challengePhase.challengeId,
-          isOpen: true,
-        },
-        select: {
-          id: true,
-          phaseId: true,
-          predecessor: true,
-          name: true,
-        },
-      });
+    if (hasActualStartDate && hasActualEndDate && openPhases.length > 0) {
+      const reopenedPhaseIdentifiers = new Set([String(challengePhase.id)]);
+      if (!_.isNil(challengePhase.phaseId)) {
+        reopenedPhaseIdentifiers.add(String(challengePhase.phaseId));
+      }
 
-      if (openPhases.length > 0) {
-        const reopenedPhaseIdentifiers = new Set([String(challengePhase.id)]);
-        if (!_.isNil(challengePhase.phaseId)) {
-          reopenedPhaseIdentifiers.add(String(challengePhase.phaseId));
-        }
-        const dependentOpenPhases = openPhases.filter((phase) => {
-          if (!phase || _.isNil(phase.predecessor)) {
-            return false;
-          }
-          return reopenedPhaseIdentifiers.has(String(phase.predecessor));
-        });
-
-        if (dependentOpenPhases.length === 0) {
-          throw new errors.ForbiddenError(
-            `Cannot reopen ${phaseName} because no currently open phase depends on it`,
-          );
+      const dependentOpenPhases = openPhases.filter((phase) => {
+        if (!phase || _.isNil(phase.predecessor)) {
+          return false;
         }
+        return reopenedPhaseIdentifiers.has(String(phase.predecessor));
+      });
 
-        const appealsDependentPhaseExists = dependentOpenPhases.some(
-          (phase) => String(phase.name || "").toLowerCase() === "appeals"
+      if (dependentOpenPhases.length === 0) {
+        throw new errors.ForbiddenError(
+          `Cannot reopen ${phaseName} because no currently open phase depends on it`
+        );
+      }
+
+      const appealsDependentPhaseExists = dependentOpenPhases.some(
+        (phase) => String(phase.name || "").toLowerCase() === "appeals"
+      );
+      if (appealsDependentPhaseExists) {
+        const hasSubmittedAppeals = await hasSubmittedAppealsForChallenge(
+          challengePhase.challengeId
         );
-        if (appealsDependentPhaseExists) {
-          const hasSubmittedAppeals = await hasSubmittedAppealsForChallenge(
-            challengePhase.challengeId
+        if (hasSubmittedAppeals) {
+          throw new errors.ForbiddenError(
+            `Cannot reopen ${phaseName} because submitted appeals already exist in the Appeals phase`
           );
-          if (hasSubmittedAppeals) {
-            throw new errors.ForbiddenError(
-              `Cannot reopen ${phaseName} because submitted appeals already exist in the Appeals phase`
-            );
-          }
         }
       }
     }
diff --git a/src/services/ChallengeService.js b/src/services/ChallengeService.js
@@ -65,6 +65,14 @@ const challengeDomain = {
 const phaseAdvancer = new PhaseAdvancer(challengeDomain);
 
 const REVIEW_STATUS_BLOCKING = Object.freeze(["IN_PROGRESS", "COMPLETED"]);
+const REVIEW_PHASE_NAMES = Object.freeze([
+  "checkpoint review",
+  "checkpoint screening",
+  "screening",
+  "review",
+  "approval",
+]);
+const REVIEW_PHASE_NAME_SET = new Set(REVIEW_PHASE_NAMES);
 
 /**
  * Enrich skills data with full details from standardized skills API.
@@ -3080,6 +3088,84 @@ async function ensureScorecardChangeDoesNotConflict({
     );
   }
 
+  const openReviewPhaseNameByKey = new Map();
+  for (const phase of originalChallengePhases || []) {
+    if (!phase || !phase.isOpen) {
+      continue;
+    }
+    const phaseName = String(phase.name || "").trim();
+    if (!phaseName) {
+      continue;
+    }
+    const normalizedName = phaseName.toLowerCase();
+    if (!REVIEW_PHASE_NAME_SET.has(normalizedName)) {
+      continue;
+    }
+    const phaseKey = phase.phaseId ? String(phase.phaseId) : null;
+    if (!phaseKey) {
+      continue;
+    }
+    openReviewPhaseNameByKey.set(phaseKey, phaseName);
+  }
+
+  const reviewPhaseKeysToCheck = Array.from(removedScorecardsByPhase.keys()).filter((phaseKey) =>
+    openReviewPhaseNameByKey.has(phaseKey)
+  );
+
+  if (reviewPhaseKeysToCheck.length > 0) {
+    for (const phaseKey of reviewPhaseKeysToCheck) {
+      const challengePhaseIds = Array.from(phaseIdToChallengePhaseIds.get(phaseKey) || []);
+      if (challengePhaseIds.length === 0) {
+        logger.debug(
+          `Skipping active phase scorecard guard for challenge ${challengeId} phase ${phaseKey} because no matching challenge phases were found`
+        );
+        continue;
+      }
+
+      let activePhaseReviewCount = 0;
+
+      if (hasReviewModel) {
+        activePhaseReviewCount = await reviewClient.review.count({
+          where: {
+            phaseId: { in: challengePhaseIds },
+            status: { in: REVIEW_STATUS_BLOCKING },
+          },
+        });
+      } else if (typeof reviewClient?.$queryRaw === "function") {
+        try {
+          const queryResult = await reviewClient.$queryRaw`
+            SELECT COUNT(*)::int AS count
+            FROM "review"
+            WHERE "phaseId" IN (${Prisma.join(challengePhaseIds)})
+              AND "status" IN (${Prisma.join(REVIEW_STATUS_BLOCKING)})
+          `;
+          const rawCount =
+            Array.isArray(queryResult) && queryResult.length > 0
+              ? Number(queryResult[0]?.count || 0)
+              : 0;
+          activePhaseReviewCount = Number.isFinite(rawCount) ? rawCount : 0;
+        } catch (error) {
+          logger.warn(
+            `Skipping active phase scorecard guard for challenge ${challengeId} phase ${phaseKey} due to review DB query failure: ${error.message}`
+          );
+          continue;
+        }
+      } else {
+        logger.warn(
+          `Skipping active phase scorecard guard for challenge ${challengeId} phase ${phaseKey} because review Prisma client does not support raw queries`
+        );
+        continue;
+      }
+
+      if (activePhaseReviewCount > 0) {
+        const phaseName = openReviewPhaseNameByKey.get(phaseKey) || "phase";
+        throw new BadRequestError(
+          `Cannot change the scorecard for phase '${phaseName}' because reviews are already in progress or completed`
+        );
+      }
+    }
+  }
+
   for (const [phaseKey, scorecardIds] of removedScorecardsByPhase.entries()) {
     const challengePhaseIds = Array.from(phaseIdToChallengePhaseIds.get(phaseKey) || []);
 
diff --git a/test/unit/ChallengePhaseService.test.js b/test/unit/ChallengePhaseService.test.js
@@ -366,6 +366,82 @@ describe('challenge phase service unit tests', () => {
       throw new Error('should not reach here')
     })
 
+    it('partially update challenge phase - cannot reopen when review phase has active scorecards', async () => {
+      const reviewPhase = await prisma.phase.create({
+        data: {
+          id: uuid(),
+          name: 'Review',
+          description: 'desc',
+          isOpen: true,
+          duration: 86400,
+          createdBy: 'admin',
+          updatedBy: 'admin'
+        }
+      })
+
+      const reviewChallengePhaseId = uuid()
+      const registrationStart = new Date('2025-08-01T00:00:00.000Z')
+      const registrationEnd = new Date('2025-08-02T00:00:00.000Z')
+
+      await prisma.challengePhase.update({
+        where: { id: data.challengePhase1Id },
+        data: {
+          isOpen: false,
+          actualStartDate: registrationStart,
+          actualEndDate: registrationEnd
+        }
+      })
+
+      await prisma.challengePhase.create({
+        data: {
+          id: reviewChallengePhaseId,
+          challengeId: data.challenge.id,
+          phaseId: reviewPhase.id,
+          name: 'Review',
+          isOpen: true,
+          predecessor: data.challengePhase1Id,
+          actualStartDate: new Date('2025-08-02T06:00:00.000Z'),
+          createdBy: 'admin',
+          updatedBy: 'admin'
+        }
+      })
+
+      const reviewId = uuid()
+      await reviewClient.$executeRaw(
+        Prisma.sql`
+          INSERT INTO ${reviewTable} ("id", "phaseId", "status")
+          VALUES (${reviewId}, ${reviewChallengePhaseId}, ${'IN_PROGRESS'})
+        `
+      )
+
+      try {
+        await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, {
+          isOpen: true
+        })
+      } catch (e) {
+        should.equal(e.httpStatus || e.statusCode, 400)
+        should.equal(
+          e.message,
+          "Cannot reopen Registration because the currently open phase 'Review' has reviews in progress or completed"
+        )
+        return
+      } finally {
+        await reviewClient.$executeRaw(Prisma.sql`DELETE FROM ${reviewTable} WHERE "id" = ${reviewId}`)
+        await prisma.challengePhase.delete({ where: { id: reviewChallengePhaseId } })
+        await prisma.phase.delete({ where: { id: reviewPhase.id } })
+        await prisma.challengePhase.update({
+          where: { id: data.challengePhase1Id },
+          data: {
+            isOpen: false,
+            actualStartDate: null,
+            actualEndDate: null
+          }
+        })
+      }
+
+      throw new Error('should not reach here')
+    })
+
     it('partially update challenge phase - cannot reopen predecessor when appeals have submitted appeals', async () => {
       const reviewPhase = await prisma.phase.create({
         data: {
diff --git a/test/unit/ChallengeService.test.js b/test/unit/ChallengeService.test.js
@@ -896,6 +896,84 @@ describe('challenge service unit tests', () => {
 
         throw new Error('should not reach here')
       })
+
+      it('blocks scorecard change when an active review phase has started reviews', async () => {
+        if (!reviewClient) {
+          return
+        }
+
+        let reviewPhase
+        let reviewChallengePhaseId
+
+        const insertedReviewId = uuid()
+
+        try {
+          reviewPhase = await prisma.phase.create({
+            data: {
+              id: uuid(),
+              name: 'Review',
+              description: 'desc',
+              isOpen: true,
+              duration: 86400,
+              createdBy: 'admin',
+              updatedBy: 'admin'
+            }
+          })
+
+          reviewChallengePhaseId = uuid()
+          const now = new Date()
+
+          await prisma.challengePhase.create({
+            data: {
+              id: reviewChallengePhaseId,
+              challengeId: data.challenge.id,
+              phaseId: reviewPhase.id,
+              name: 'Review',
+              isOpen: true,
+              actualStartDate: now,
+              createdBy: 'admin',
+              updatedBy: 'admin'
+            }
+          })
+
+          await prisma.challengeReviewer.updateMany({
+            where: { challengeId: data.challenge.id },
+            data: { phaseId: reviewPhase.id }
+          })
+
+          await reviewClient.$executeRawUnsafe(
+            `INSERT INTO ${reviewTableName} ("id", "phaseId", "scorecardId", "status") VALUES ('${insertedReviewId}', '${reviewChallengePhaseId}', '${originalScorecardId}', 'COMPLETED')`
+          )
+
+          await service.updateChallenge({ isMachine: true, sub: 'sub3', userId: 22838965 }, data.challenge.id, {
+            reviewers: [
+              {
+                phaseId: reviewPhase.id,
+                scorecardId: newScorecardId,
+                isMemberReview: false
+              }
+            ]
+          })
+        } catch (e) {
+          should.equal(
+            e.message,
+            "Cannot change the scorecard for phase 'Review' because reviews are already in progress or completed"
+          )
+          return
+        } finally {
+          await reviewClient.$executeRawUnsafe(
+            `DELETE FROM ${reviewTableName} WHERE "id" = '${insertedReviewId}'`
+          )
+          if (reviewChallengePhaseId) {
+            await prisma.challengePhase.delete({ where: { id: reviewChallengePhaseId } })
+          }
+          if (reviewPhase) {
+            await prisma.phase.delete({ where: { id: reviewPhase.id } })
+          }
+        }
+
+        throw new Error('should not reach here')
+      })
     })
 
     it('update challenge - creator memberId can modify without matching handle', async () => {
