diff --git a/src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentScreening.tsx b/src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentScreening.tsx index ccc3018bc..344bd0aa7 100644 --- a/src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentScreening.tsx +++ b/src/apps/review/src/lib/components/ChallengeDetailsContent/TabContentScreening.tsx @@ -71,6 +71,18 @@ export const TabContentScreening: FC = (props: Props) => { const filteredScreening = useMemo( () => { 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) { @@ -78,10 +90,10 @@ export const TabContentScreening: FC = (props: Props) => { } 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))) { diff --git a/src/apps/review/src/lib/components/TableRegistration/TableRegistration.tsx b/src/apps/review/src/lib/components/TableRegistration/TableRegistration.tsx index 8c99cea40..54328d89d 100644 --- a/src/apps/review/src/lib/components/TableRegistration/TableRegistration.tsx +++ b/src/apps/review/src/lib/components/TableRegistration/TableRegistration.tsx @@ -58,6 +58,14 @@ export const TableRegistration: FC = (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) => { [loginUserInfo?.roles], ) - const shouldDisplayEmail = hasCopilotRole || isAdmin + const shouldDisplayEmail = hasCopilotRole || hasManagerRole || isAdmin const columns = useMemo[]>( () => { diff --git a/src/apps/review/src/lib/components/TableSubmissionScreening/TableSubmissionScreening.tsx b/src/apps/review/src/lib/components/TableSubmissionScreening/TableSubmissionScreening.tsx index 659be9df4..20baf2be7 100644 --- a/src/apps/review/src/lib/components/TableSubmissionScreening/TableSubmissionScreening.tsx +++ b/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[] => [ { label: 'Screener', - propertyName: 'screenerHandle', - renderer: (data: Screening) => (data.screener?.memberHandle ? ( - - {data.screener?.memberHandle ?? ''} - - ) : ( - - {data.screener?.memberHandle ?? ''} - - )), + propertyName: 'screener', + renderer: (data: Screening) => { + const normalizedPhaseName = data.phaseName + ?.toLowerCase() + .trim() + + if (normalizedPhaseName && normalizedPhaseName !== 'screening') { + return + } + + // Display the screener handle from the Screening phase review + // (Review phase data is filtered out in TabContentScreening) + return data.screener?.memberHandle ? ( + + {data.screener?.memberHandle ?? ''} + + ) : ( + + {data.screener?.memberHandle ?? ''} + + ) + }, 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 {scoreValue} + } + if (!data.reviewId || maskScore) { return {scoreValue} } @@ -631,6 +663,7 @@ export const TableSubmissionScreening: FC = (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) => { return false } - if (canViewAllScorecards) { + if (canViewAllScorecards || hasReviewerRole) { return true } @@ -996,6 +1029,7 @@ export const TableSubmissionScreening: FC = (props: Props) => { }, [ canViewAllScorecards, + hasReviewerRole, currentMemberId, myResourceIds, ], diff --git a/src/apps/review/src/lib/hooks/useFetchScreeningReview.ts b/src/apps/review/src/lib/hooks/useFetchScreeningReview.ts index 5e947dbf0..21ac26f0f 100644 --- a/src/apps/review/src/lib/hooks/useFetchScreeningReview.ts +++ b/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), diff --git a/src/apps/review/src/lib/models/Screening.model.ts b/src/apps/review/src/lib/models/Screening.model.ts index ca928d6a1..931015adc 100644 --- a/src/apps/review/src/lib/models/Screening.model.ts +++ b/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 } /** diff --git a/src/apps/review/src/lib/utils/reviewMatching.ts b/src/apps/review/src/lib/utils/reviewMatching.ts index f6d9b0d89..21d1322d2 100644 --- a/src/apps/review/src/lib/utils/reviewMatching.ts +++ b/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) diff --git a/src/apps/review/src/pages/active-review-assignements/ChallengeDetailsPage/ChallengeDetailsPage.tsx b/src/apps/review/src/pages/active-review-assignements/ChallengeDetailsPage/ChallengeDetailsPage.tsx index 25e0bc0b1..82ffe6d8a 100644 --- a/src/apps/review/src/pages/active-review-assignements/ChallengeDetailsPage/ChallengeDetailsPage.tsx +++ b/src/apps/review/src/pages/active-review-assignements/ChallengeDetailsPage/ChallengeDetailsPage.tsx @@ -1045,13 +1045,20 @@ export const ChallengeDetailsPage: FC = (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],