Additional fixes for returning screenings if a user is a reviewer

jmgasper · jmgasper · commit ef42e7eb5fba · 2025-10-28T18:26:22.000+11:00
diff --git a/src/api/review/review.service.spec.ts b/src/api/review/review.service.spec.ts
@@ -1686,6 +1686,139 @@ describe('ReviewService.getReviews reviewer visibility', () => {
     expect(otherReview?.reviewerMaxRating).toBe(1800);
   });
 
+  it('exposes screening review items and scores to other reviewers', async () => {
+    const now = new Date();
+    const reviewerUser: JwtUser = {
+      userId: '101',
+      roles: [],
+      isMachine: false,
+    };
+
+    const reviewerResource = {
+      ...buildResource('Reviewer', reviewerUser.userId),
+      id: 'resource-self',
+    };
+
+    resourceApiServiceMock.getMemberResourcesRoles.mockResolvedValue([
+      reviewerResource,
+    ]);
+
+    const reviewRecords = [
+      {
+        id: 'review-self',
+        resourceId: 'resource-self',
+        submissionId: baseSubmission.id,
+        phaseId: 'phase-review',
+        scorecardId: 'scorecard-1',
+        typeId: 'type-1',
+        status: ReviewStatus.COMPLETED,
+        reviewDate: now,
+        committed: true,
+        metadata: null,
+        reviewItems: [
+          {
+            id: 'item-self',
+            scorecardQuestionId: 'question-1',
+            initialAnswer: 'Yes',
+            finalAnswer: 'Yes',
+            managerComment: null,
+            createdAt: now,
+            createdBy: 'reviewer',
+            updatedAt: now,
+            updatedBy: 'reviewer',
+            reviewItemComments: [],
+          },
+        ],
+        createdAt: now,
+        createdBy: 'creator',
+        updatedAt: now,
+        updatedBy: 'updater',
+        finalScore: 98,
+        initialScore: 96,
+        submission: {
+          id: baseSubmission.id,
+          memberId: '301',
+          challengeId: 'challenge-1',
+        },
+      },
+      {
+        id: 'review-screening',
+        resourceId: 'resource-other',
+        submissionId: baseSubmission.id,
+        phaseId: 'phase-screening',
+        scorecardId: 'scorecard-2',
+        typeId: 'type-2',
+        status: ReviewStatus.COMPLETED,
+        reviewDate: now,
+        committed: true,
+        metadata: null,
+        reviewItems: [
+          {
+            id: 'item-screening',
+            scorecardQuestionId: 'question-3',
+            initialAnswer: 'Yes',
+            finalAnswer: 'Yes',
+            managerComment: null,
+            createdAt: now,
+            createdBy: 'other-reviewer',
+            updatedAt: now,
+            updatedBy: 'other-reviewer',
+            reviewItemComments: [],
+          },
+        ],
+        createdAt: now,
+        createdBy: 'creator',
+        updatedAt: now,
+        updatedBy: 'updater',
+        finalScore: 82,
+        initialScore: 80,
+        submission: {
+          id: baseSubmission.id,
+          memberId: '301',
+          challengeId: 'challenge-1',
+        },
+      },
+    ];
+
+    prismaMock.review.findMany.mockResolvedValue(reviewRecords);
+    prismaMock.review.count.mockResolvedValue(reviewRecords.length);
+
+    resourcePrismaMock.resource.findMany.mockResolvedValue([
+      { id: 'resource-self', memberId: '101' },
+      { id: 'resource-other', memberId: '202' },
+    ]);
+
+    memberPrismaMock.member.findMany.mockResolvedValue([
+      {
+        userId: BigInt(101),
+        handle: 'selfHandle',
+        maxRating: { rating: 2400 },
+      },
+      {
+        userId: BigInt(202),
+        handle: 'screeningHandle',
+        maxRating: { rating: 2100 },
+      },
+    ]);
+
+    const response = await service.getReviews(
+      reviewerUser,
+      undefined,
+      'challenge-1',
+    );
+
+    expect(response.data).toHaveLength(2);
+    const screeningReview = response.data.find(
+      (review) => review.id === 'review-screening',
+    );
+
+    expect(screeningReview?.reviewItems).toHaveLength(1);
+    expect(screeningReview?.finalScore).toBe(82);
+    expect(screeningReview?.initialScore).toBe(80);
+    expect(screeningReview?.reviewerHandle).toBe('screeningHandle');
+    expect(screeningReview?.reviewerMaxRating).toBe(2100);
+  });
+
   it('returns empty results for submitters when reviews are not yet accessible', async () => {
     const submitterUser: JwtUser = {
       userId: 'submitter-1',
diff --git a/src/api/review/review.service.ts b/src/api/review/review.service.ts
@@ -3434,7 +3434,9 @@ export class ReviewService {
           !isPrivilegedRequester &&
           reviewerResourceIdSet.size > 0 &&
           !isReviewerForReview &&
-          !this.isCompletedOrCancelledStatus(challengeStatusForReview);
+          !this.isCompletedOrCancelledStatus(challengeStatusForReview) &&
+          normalizedPhaseName !== 'screening' &&
+          normalizedPhaseName !== 'checkpoint screening';
 
         const sanitizeScores =
           shouldMaskReviewDetails ||
@@ -3456,11 +3458,16 @@ export class ReviewService {
           !isOwnSubmission &&
           normalizedPhaseName === 'iterative review' &&
           this.isFirst2FinishChallenge(challengeForReview);
+        const shouldStripOtherReviewerContent =
+          shouldMaskOtherReviewerDetails &&
+          !['screening', 'checkpoint screening'].includes(
+            normalizedPhaseName ?? '',
+          );
         const shouldStripReviewItems =
           shouldMaskReviewDetails ||
           shouldTrimIterativeReviewForOtherSubmitters ||
           shouldLimitNonOwnerVisibility ||
-          shouldMaskOtherReviewerDetails;
+          shouldStripOtherReviewerContent;
 
         if (!isThin) {
           sanitizedReview.reviewItems = shouldStripReviewItems
diff --git a/src/api/submission/submission.service.spec.ts b/src/api/submission/submission.service.spec.ts
@@ -997,6 +997,7 @@ describe('SubmissionService', () => {
               id: 'review-self',
               resourceId: 'resource-self',
               submissionId: 'submission-1',
+              phaseId: 'phase-123',
               finalScore: 95,
               initialScore: 92,
               reviewItems: [
@@ -1017,6 +1018,7 @@ describe('SubmissionService', () => {
               id: 'review-other',
               resourceId: 'resource-other',
               submissionId: 'submission-1',
+              phaseId: 'phase-123',
               finalScore: 80,
               initialScore: 78,
               reviewItems: [
@@ -1100,5 +1102,144 @@ describe('SubmissionService', () => {
       expect(otherReview?.reviewerHandle).toBe('otherHandle');
       expect(otherReview?.reviewerMaxRating).toBe(1800);
     });
+
+    it('preserves screening review items and scores for other reviewers', async () => {
+      const now = new Date('2025-01-06T10:00:00Z');
+      challengeApiServiceMock.getChallengeDetail.mockResolvedValue({
+        id: 'challenge-1',
+        status: ChallengeStatus.ACTIVE,
+        type: 'Challenge',
+        legacy: {},
+        phases: [
+          {
+            id: 'phase-review',
+            phaseId: 'legacy-phase-review',
+            name: 'Review',
+          },
+          {
+            id: 'phase-screening',
+            phaseId: 'legacy-phase-screening',
+            name: 'Screening',
+          },
+        ],
+      });
+      resourceApiServiceListMock.getMemberResourcesRoles.mockResolvedValue([
+        {
+          roleName: 'Reviewer',
+          id: 'resource-self',
+          memberId: '101',
+        },
+      ]);
+
+      const submissions = [
+        {
+          id: 'submission-2',
+          challengeId: 'challenge-1',
+          memberId: 'submitter-1',
+          submittedDate: now,
+          createdAt: now,
+          updatedAt: now,
+          type: SubmissionType.CONTEST_SUBMISSION,
+          status: SubmissionStatus.ACTIVE,
+          review: [
+            {
+              id: 'review-self',
+              resourceId: 'resource-self',
+              submissionId: 'submission-2',
+              phaseId: 'phase-review',
+              finalScore: 90,
+              initialScore: 88,
+              reviewItems: [
+                {
+                  id: 'item-self',
+                  scorecardQuestionId: 'q1',
+                  initialAnswer: 'YES',
+                  finalAnswer: 'YES',
+                  reviewItemComments: [],
+                },
+              ],
+              createdAt: now,
+              createdBy: 'reviewer',
+              updatedAt: now,
+              updatedBy: 'reviewer',
+            },
+            {
+              id: 'review-screening',
+              resourceId: 'resource-other',
+              submissionId: 'submission-2',
+              phaseId: 'phase-screening',
+              finalScore: 75,
+              initialScore: 70,
+              reviewItems: [
+                {
+                  id: 'item-screening',
+                  scorecardQuestionId: 'q2',
+                  initialAnswer: 'NO',
+                  finalAnswer: 'NO',
+                  reviewItemComments: [],
+                },
+              ],
+              createdAt: now,
+              createdBy: 'screening-reviewer',
+              updatedAt: now,
+              updatedBy: 'screening-reviewer',
+            },
+          ],
+          reviewSummation: [],
+          legacyChallengeId: null,
+          prizeId: null,
+        },
+      ];
+
+      prismaMock.submission.findMany.mockResolvedValue(
+        submissions.map((entry) => ({ ...entry })),
+      );
+      prismaMock.submission.count.mockResolvedValue(submissions.length);
+      prismaMock.submission.findFirst.mockResolvedValue({
+        id: 'submission-2',
+      });
+
+      resourcePrismaListMock.resource.findMany.mockResolvedValue([
+        { id: 'resource-self', memberId: '101' },
+        { id: 'resource-other', memberId: '202' },
+      ]);
+
+      memberPrismaMock.member.findMany.mockResolvedValue([
+        {
+          userId: BigInt(101),
+          handle: 'selfHandle',
+          maxRating: { rating: 2500 },
+        },
+        {
+          userId: BigInt(202),
+          handle: 'screeningHandle',
+          maxRating: { rating: 2000 },
+        },
+      ]);
+
+      const result = await listService.listSubmission(
+        {
+          userId: '101',
+          isMachine: false,
+          roles: [UserRole.Reviewer],
+        } as any,
+        { challengeId: 'challenge-1' } as any,
+        { page: 1, perPage: 50 } as any,
+      );
+
+      const submissionResult = result.data.find(
+        (entry) => entry.id === 'submission-2',
+      );
+      const screeningReview = submissionResult?.review?.find(
+        (review) => review.id === 'review-screening',
+      );
+
+      expect(screeningReview).toBeDefined();
+      expect(screeningReview?.initialScore).toBe(70);
+      expect(screeningReview?.finalScore).toBe(75);
+      expect(screeningReview?.reviewItems).toHaveLength(1);
+      expect(screeningReview?.reviewerHandle).toBe('screeningHandle');
+      expect(screeningReview?.reviewerMaxRating).toBe(2000);
+    });
   });
 });
diff --git a/src/api/submission/submission.service.ts b/src/api/submission/submission.service.ts
@@ -2409,14 +2409,32 @@ export class SubmissionService {
               const ownsReview =
                 resourceId.length > 0 &&
                 roleSummary.reviewerResourceIds.includes(resourceId);
+              const resolvedPhaseName = this.getPhaseNameFromId(
+                challenge,
+                (review as any).phaseId ?? null,
+              );
+              const normalizedPhaseName =
+                this.normalizePhaseName(resolvedPhaseName);
+              const isScreeningPhase =
+                normalizedPhaseName === 'screening' ||
+                normalizedPhaseName === 'checkpoint screening';
 
               if (!ownsReview) {
-                review.initialScore = null;
-                review.finalScore = null;
-                if (Array.isArray(review.reviewItems)) {
-                  review.reviewItems = [];
+                if (!isScreeningPhase) {
+                  review.initialScore = null;
+                  review.finalScore = null;
+                  review.reviewItems = Array.isArray(review.reviewItems)
+                    ? []
+                    : [];
                 } else {
-                  review.reviewItems = [];
+                  review.initialScore =
+                    typeof review.initialScore === 'number'
+                      ? review.initialScore
+                      : (review.initialScore ?? null);
+                  review.finalScore =
+                    typeof review.finalScore === 'number'
+                      ? review.finalScore
+                      : (review.finalScore ?? null);
                 }
               }
             }
