Merge pull request #1278 from topcoder-platform/feat/v6

QA fixes for reviewers seeing screenings even if they aren't a screener

Author: jmgasper

src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentScreening.tsx

@@ -71,17 +71,29 @@ export const TabContentScreening: FC<Props> = (props: Props) => {
     const filteredScreening = useMemo<Screening[]>(
         () => {
             const baseRows = props.screening ?? []
+            // Defensive filtering: exclude any entries that don't belong to Screening phase
+            const phaseValidatedRows = baseRows.filter(row => {
+                if (!row.reviewId) {
+                    return true
+                }
+
+                const normalizedPhaseName = row.phaseName
+                    ?.toLowerCase()
+                    .trim()
+
+                return normalizedPhaseName === 'screening'
+            })
             const canSeeAll = isPrivilegedRole || hasReviewerRole
 
             if (isChallengeCompleted && !canSeeAll && !hasPassedScreeningThreshold) {
                 return []
             }
 
             if (canSeeAll || (isChallengeCompleted && hasPassedScreeningThreshold)) {
-                return baseRows
+                return phaseValidatedRows
             }
 
-            return baseRows.filter(row => {
+            return phaseValidatedRows.filter(row => {
                 if (row.myReviewResourceId
                     && (screenerResourceIds.has(row.myReviewResourceId)
                         || reviewerResourceIds.has(row.myReviewResourceId))) {

src/apps/review/src/lib/components/TableRegistration/TableRegistration.tsx

@@ -58,6 +58,14 @@ export const TableRegistration: FC<Props> = (props: Props) => {
         [myRoles],
     )
 
+    const hasManagerRole = useMemo(
+        () => (myRoles ?? [])
+            .some(role => role
+                ?.trim()
+                .toLowerCase() === 'manager'),
+        [myRoles],
+    )
+
     const isAdmin = useMemo(
         () => loginUserInfo?.roles?.some(
             role => typeof role === 'string'
@@ -66,7 +74,7 @@ export const TableRegistration: FC<Props> = (props: Props) => {
         [loginUserInfo?.roles],
     )
 
-    const shouldDisplayEmail = hasCopilotRole || isAdmin
+    const shouldDisplayEmail = hasCopilotRole || hasManagerRole || isAdmin
 
     const columns = useMemo<TableColumn<BackendResource>[]>(
         () => {

src/apps/review/src/lib/components/TableSubmissionScreening/TableSubmissionScreening.tsx

@@ -44,8 +44,9 @@ import {
 import { ChallengeDetailContext, ReviewAppContext } from '../../contexts'
 import { updateReview } from '../../services'
 import { ConfirmModal } from '../ConfirmModal'
-import { useSubmissionDownloadAccess } from '../../hooks'
+import { useRole, useSubmissionDownloadAccess } from '../../hooks'
 import type { UseSubmissionDownloadAccessResult } from '../../hooks/useSubmissionDownloadAccess'
+import type { useRoleProps } from '../../hooks/useRole'
 
 import styles from './TableSubmissionScreening.module.scss'
 
@@ -424,48 +425,79 @@ const isScreeningReviewInProgress = (entry: Screening): boolean => (
     || isInProgressStatus(entry.myReviewStatus)
 )
 
+/**
+ * Creates columns for displaying screening review data.
+ *
+ * Note: This function assumes that the input data has been validated
+ * to contain only Screening phase reviews. Defensive filtering is
+ * performed in TabContentScreening to ensure phase data isolation.
+ *
+ * @param config - Configuration for scorecard access and score masking
+ * @returns Array of table columns for screening data
+ */
 const createScreeningColumns = ({
     canViewScorecard,
     shouldMaskScore,
 }: ScreeningColumnConfig): TableColumn<Screening>[] => [
     {
         label: 'Screener',
-        propertyName: 'screenerHandle',
-        renderer: (data: Screening) => (data.screener?.memberHandle ? (
-            <a
-                href={getHandleUrl(data.screener)}
-                target='_blank'
-                rel='noreferrer'
-                style={{
-                    color: data.screener?.handleColor,
-                }}
-                onClick={function onClick() {
-                    window.open(
-                        getHandleUrl(data.screener),
-                        '_blank',
-                    )
-                }}
-            >
-                {data.screener?.memberHandle ?? ''}
-            </a>
-        ) : (
-            <span
-                style={{
-                    color: data.screener?.handleColor,
-                }}
-            >
-                {data.screener?.memberHandle ?? ''}
-            </span>
-        )),
+        propertyName: 'screener',
+        renderer: (data: Screening) => {
+            const normalizedPhaseName = data.phaseName
+                ?.toLowerCase()
+                .trim()
+
+            if (normalizedPhaseName && normalizedPhaseName !== 'screening') {
+                return <span />
+            }
+
+            // Display the screener handle from the Screening phase review
+            // (Review phase data is filtered out in TabContentScreening)
+            return data.screener?.memberHandle ? (
+                <a
+                    href={getHandleUrl(data.screener)}
+                    target='_blank'
+                    rel='noreferrer'
+                    style={{
+                        color: data.screener?.handleColor,
+                    }}
+                    onClick={function onClick() {
+                        window.open(
+                            getHandleUrl(data.screener),
+                            '_blank',
+                        )
+                    }}
+                >
+                    {data.screener?.memberHandle ?? ''}
+                </a>
+            ) : (
+                <span
+                    style={{
+                        color: data.screener?.handleColor,
+                    }}
+                >
+                    {data.screener?.memberHandle ?? ''}
+                </span>
+            )
+        },
         type: 'element',
     },
     {
         label: 'Screening Score',
         propertyName: 'score',
         renderer: (data: Screening) => {
+            const normalizedPhaseName = data.phaseName
+                ?.toLowerCase()
+                .trim()
+            // Link to the Screening phase scorecard
+            // (Review phase scorecards are filtered out upstream)
             const maskScore = shouldMaskScore(data)
             const scoreValue = maskScore ? '--' : (data.score ?? '-')
 
+            if (normalizedPhaseName && normalizedPhaseName !== 'screening') {
+                return <span>{scoreValue}</span>
+            }
+
             if (!data.reviewId || maskScore) {
                 return <span>{scoreValue}</span>
             }
@@ -631,6 +663,7 @@ export const TableSubmissionScreening: FC<Props> = (props: Props) => {
         getRestrictionMessageForMember,
         currentMemberId,
     }: UseSubmissionDownloadAccessResult = useSubmissionDownloadAccess()
+    const { hasReviewerRole }: useRoleProps = useRole()
 
     const normalisedRoles = useMemo(
         () => (myRoles ?? []).map(role => role.toLowerCase()),
@@ -971,7 +1004,7 @@ export const TableSubmissionScreening: FC<Props> = (props: Props) => {
                 return false
             }
 
-            if (canViewAllScorecards) {
+            if (canViewAllScorecards || hasReviewerRole) {
                 return true
             }
 
@@ -996,6 +1029,7 @@ export const TableSubmissionScreening: FC<Props> = (props: Props) => {
         },
         [
             canViewAllScorecards,
+            hasReviewerRole,
             currentMemberId,
             myResourceIds,
         ],

src/apps/review/src/lib/hooks/useFetchScreeningReview.ts

@@ -1149,6 +1149,7 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
                     myReviewId: myAssignment?.id,
                     myReviewResourceId: myAssignment?.resourceId,
                     myReviewStatus: myAssignment?.status ?? undefined,
+                    phaseName: matchedReview?.phaseName ?? undefined,
                     result,
                     reviewId: matchedReview?.id,
                     reviewPhaseId: resolveReviewPhaseId(matchedReview),

src/apps/review/src/lib/models/Screening.model.ts

@@ -57,6 +57,11 @@ export interface Screening {
      * Submission type (e.g. CONTEST_SUBMISSION, CHECKPOINT_SUBMISSION).
      */
     type?: string
+    /**
+     * The phase name of the associated review (e.g., 'Screening', 'Review').
+     * Used for defensive filtering to ensure phase data isolation.
+     */
+    phaseName?: string
 }
 
 /**

src/apps/review/src/lib/utils/reviewMatching.ts

@@ -118,12 +118,14 @@ function resolvePhaseOrTypeMatch({
             return true
         }
 
-        debugLog('reviewMatchesPhase.phaseNameFallback', {
+        debugLog('reviewMatchesPhase.phaseNameMismatchRejected', {
             normalizedPhaseName,
             normalizedReviewPhaseName,
             reviewId: review.id,
             reviewPhaseName: truncateForLog(reviewPhaseName ?? ''),
         })
+
+        return false
     }
 
     if (normalizedReviewTypeAlpha && reviewType && normalizedPhaseNameForReviewType) {
@@ -351,14 +353,15 @@ const doesReviewMatchScorecard = (
 }
 
 /**
- * Determines whether a review matches the supplied phase context by evaluating scorecard,
- * phase identifier, phase name, type, and metadata criteria in priority order.
+ * Determines whether a review matches the supplied phase context. Reviews that include an explicit
+ * phase name must match the target phase name exactly; otherwise scorecard, phase identifier, type,
+ * and metadata criteria are evaluated in order.
  *
  * @param review - Review to evaluate.
  * @param scorecardId - Scorecard identifier associated with the target phase.
  * @param phaseIds - Set of phase identifiers collected for the target phase.
  * @param phaseName - Optional phase name for matching when available.
- * @returns True when the review satisfies any of the matching criteria.
+ * @returns True when the review satisfies the matching criteria with phase names taking precedence.
  */
 export function reviewMatchesPhase(
     review: BackendReview | undefined,
@@ -428,6 +431,8 @@ export function reviewMatchesPhase(
             normalizedReviewPhaseName,
             reviewId: review.id,
         })
+
+        return false
     }
 
     const reviewTypeId = getNormalizedLowerCase(review.typeId ?? undefined)

src/apps/review/src/pages/active-review-assignements/ChallengeDetailsPage/ChallengeDetailsPage.tsx

@@ -1045,13 +1045,20 @@ export const ChallengeDetailsPage: FC<Props> = (props: Props) => {
             }),
         [myResources],
     )
+    const hasManagerRole = useMemo(
+        () => (myResources ?? [])
+            .some(resource => (resource.roleName ?? '')
+                .trim()
+                .toLowerCase() === 'manager'),
+        [myResources],
+    )
     const isAdmin = useMemo(
         () => loginUserInfo?.roles?.some(
             role => typeof role === 'string' && role.toLowerCase() === 'administrator',
         ) ?? false,
         [loginUserInfo?.roles],
     )
-    const shouldShowResourcesSection = hasCopilotRole || isAdmin
+    const shouldShowResourcesSection = hasCopilotRole || hasManagerRole || isAdmin
     const canManagePhases = useMemo(
         () => !isPastReviewDetail && (hasCopilotRole || isAdmin),
         [hasCopilotRole, isAdmin, isPastReviewDetail],