From 628b519f118a9ba4753787fd37fa95e7977b4748 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 31 Jul 2026 00:57:21 +1000 Subject: [PATCH] PM-5775: show checkpoint winner indicator What was broken Checkpoint Review showed scores without identifying members who had won a checkpoint prize. Root cause The review app did not model or render the Challenge API's separate checkpointWinners data. What was changed - Preserve checkpoint winners separately from final-placement winners. - Show an accessible star beside checkpoint scores for winning members with click, hover, and keyboard tooltip details. - Document the member-level winner matching used by the table. Any added/updated tests - Added checkpoint table coverage for winner and non-winner rows, repeated submissions, score links, and tooltip behavior. - Updated challenge model coverage for dedicated and legacy checkpoint winner data. --- src/apps/review/README.md | 2 + .../TableCheckpointSubmissions.module.scss | 29 ++ .../TableCheckpointSubmissions.spec.tsx | 248 ++++++++++++++++++ .../TableCheckpointSubmissions.tsx | 67 +++-- .../models/BackendChallengeInfo.model.spec.ts | 51 ++++ .../lib/models/BackendChallengeInfo.model.ts | 10 +- .../src/lib/models/ChallengeInfo.model.ts | 1 + 7 files changed, 392 insertions(+), 16 deletions(-) create mode 100644 src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.spec.tsx diff --git a/src/apps/review/README.md b/src/apps/review/README.md index 2258c46c6..6ad3ecd76 100644 --- a/src/apps/review/README.md +++ b/src/apps/review/README.md @@ -33,3 +33,5 @@ sudo yarn start omitted safely. - Canonical `PLACEMENT` winner types are shown. Untyped and contest-submission winner types remain supported for legacy challenge records, while checkpoint winner types are excluded. +- Checkpoint winners remain separate from final placements and are identified by member ID in the + Checkpoint Review table. diff --git a/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.module.scss b/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.module.scss index 116e28b27..42714edc6 100644 --- a/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.module.scss +++ b/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.module.scss @@ -114,6 +114,35 @@ } } +.scoreCell { + align-items: center; + display: inline-flex; + gap: $sp-2; +} + +.checkpointWinnerButton { + align-items: center; + background: none; + border: 0; + color: $gold-3; + cursor: pointer; + display: inline-flex; + height: 24px; + justify-content: center; + padding: 0; + width: 24px; + + &:focus { + outline: 2px solid currentColor; + outline-offset: 2px; + } + + svg { + height: 18px; + width: 18px; + } +} + .pendingScore { color: var(--GrayFontColor); diff --git a/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.spec.tsx b/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.spec.tsx new file mode 100644 index 000000000..456821828 --- /dev/null +++ b/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.spec.tsx @@ -0,0 +1,248 @@ +/* eslint-disable import/no-extraneous-dependencies, ordered-imports/ordered-imports */ +import type { PropsWithChildren, ReactNode } from 'react' +import { render, screen } from '@testing-library/react' + +import { + ChallengeDetailContext, + ReviewAppContext, +} from '../../contexts' +import type { + ChallengeDetailContextModel, + ChallengeInfo, + ReviewAppContextModel, + Screening, +} from '../../models' + +import { TableCheckpointSubmissions } from './TableCheckpointSubmissions' + +jest.mock('react-router-dom', () => ({ + Link: (props: PropsWithChildren<{ className?: string, to: string }>) => ( + {props.children} + ), +})) + +jest.mock('react-toastify', () => ({ + toast: { + success: jest.fn(), + }, +})) + +jest.mock('~/libs/core', () => ({ + UserRole: { + administrator: 'administrator', + }, +}), { virtual: true }) + +jest.mock('~/libs/shared', () => ({ + copyTextToClipboard: () => Promise.resolve(), + useWindowSize: () => ({ + height: 800, + width: 1200, + }), +}), { virtual: true }) + +jest.mock('~/apps/admin/src/lib/components/common/TableMobile', () => ({ + TableMobile: () =>
Mobile table
, +}), { virtual: true }) + +jest.mock('~/apps/admin/src/lib/utils', () => ({ + handleError: jest.fn(), +}), { virtual: true }) + +jest.mock('~/libs/ui', () => ({ + IconOutline: { + CheckIcon: () => , + DocumentDuplicateIcon: () => , + }, + IconSolid: { + StarIcon: () => , + }, + Table: (props: { + columns: Array<{ + label?: ReactNode + renderer?: (row: Screening, rows: Screening[]) => JSX.Element + }> + data: Screening[] + }) => { + const scoreColumn = props.columns.find(column => column.label === 'Review Score') + + return ( +
+ {props.data.map(row => ( +
+ {scoreColumn?.renderer?.(row, props.data)} +
+ ))} +
+ ) + }, + Tooltip: (props: PropsWithChildren<{ + content?: ReactNode + triggerOn?: string + }>) => ( + + {props.children} + {props.content} + + ), +}), { virtual: true }) + +jest.mock('../../contexts', () => { + const React: typeof import('react') = jest.requireActual('react') + + return { + ChallengeDetailContext: React.createContext({}), + ReviewAppContext: React.createContext({}), + } +}) + +jest.mock('../../hooks', () => ({ + useRolePermissions: () => ({ + canViewAllSubmissions: true, + }), + useSubmissionDownloadAccess: () => ({ + getRestrictionMessageForMember: () => undefined, + isSubmissionDownloadRestrictedForMember: () => false, + restrictionMessage: undefined, + }), +})) + +jest.mock('../../services', () => ({ + updateReview: jest.fn(), +})) + +jest.mock('../../utils', () => ({ + getHandleUrl: () => 'https://profiles.example.com', + isReviewPhaseCurrentlyOpen: () => false, + refreshChallengeReviewData: jest.fn(), + REOPEN_MESSAGE_OTHER: 'Reopen another review?', + REOPEN_MESSAGE_SELF: 'Reopen your review?', +})) + +jest.mock('../CollapsibleAiReviewsRow', () => ({ + CollapsibleAiReviewsRow: () =>
AI reviews
, +})) + +jest.mock('../ConfirmModal', () => ({ + ConfirmModal: () => undefined, +})) + +jest.mock('../TableWrapper', () => ({ + TableWrapper: (props: PropsWithChildren<{ className?: string }>) => ( +
{props.children}
+ ), +})) + +const winnerRow = { + challengeId: 'challenge-id', + createdAt: '2026-07-29T04:17:00.000Z', + memberId: '5678', + result: 'PASS', + score: '100.00', + submissionId: 'winner-submission', +} as Screening + +const nonWinnerRow = { + ...winnerRow, + memberId: '9999', + reviewId: 'non-winner-review', + score: '88.89', + submissionId: 'non-winner-submission', +} as Screening + +const secondWinnerRow = { + ...winnerRow, + reviewId: 'second-winner-review', + score: '77.78', + submissionId: 'second-winner-submission', +} as Screening + +const challengeInfo = { + checkpointWinners: [{ + handle: 'checkpointWinner', + placement: 1, + userId: 5678, + }], + currentPhase: 'Checkpoint Review', + currentPhaseEndDate: '2026-07-29T05:00:00.000Z', + id: 'challenge-id', + metadata: [], + name: 'Checkpoint Challenge', + phases: [], + status: 'Completed', + submissions: [], + track: { + id: 'track-id', + name: 'Design', + }, + type: { + id: 'type-id', + name: 'Challenge', + }, + typeId: 'type-id', +} as ChallengeInfo + +const challengeContext = { + challengeInfo, + myResources: [], + myRoles: [], +} as unknown as ChallengeDetailContextModel + +const reviewAppContext = { + cancelLoadChallengeRelativeInfos: jest.fn(), + challengeRelativeInfosMapping: {}, + loadChallengeRelativeInfos: jest.fn(), + loginUserInfo: { + roles: [], + userId: 5678, + }, +} as ReviewAppContextModel + +/** + * Renders the desktop checkpoint review table with two winner rows and one non-winner. + * + * @returns The Testing Library render result for the checkpoint table. + * @throws This test helper does not throw. + */ +function renderCheckpointTable(): ReturnType { + return render( + + + + + , + ) +} + +describe('TableCheckpointSubmissions checkpoint winner indicator', () => { + it('marks only rows whose member id matches a checkpoint winner', () => { + renderCheckpointTable() + + expect(screen.getAllByRole('button', { name: 'Checkpoint winner details' })) + .toHaveLength(2) + expect(screen.getAllByTestId('checkpoint-winner-star')) + .toHaveLength(2) + screen.getAllByTestId('checkpoint-winner-tooltip') + .forEach(tooltip => { + expect(tooltip.getAttribute('data-trigger-on')) + .toBe('click-hover') + }) + expect(screen.getAllByText( + 'Checkpoint winner. This member is eligible for the checkpoint prize associated with this challenge.', + )) + .toHaveLength(2) + expect(screen.getByText('100.00')) + .toBeTruthy() + expect(screen.getByRole('link', { name: '88.89' }) + .getAttribute('href')) + .toBe('./../reviews/non-winner-submission?reviewId=non-winner-review') + }) +}) diff --git a/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.tsx b/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.tsx index 88ee54e87..0882351a1 100644 --- a/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.tsx +++ b/src/apps/review/src/lib/components/TableCheckpointSubmissions/TableCheckpointSubmissions.tsx @@ -11,7 +11,13 @@ import { TableMobile } from '~/apps/admin/src/lib/components/common/TableMobile' import { IsRemovingType } from '~/apps/admin/src/lib/models' import { MobileTableColumn } from '~/apps/admin/src/lib/models/MobileTableColumn.model' import { copyTextToClipboard, useWindowSize, WindowSize } from '~/libs/shared' -import { IconOutline, Table, TableColumn, Tooltip } from '~/libs/ui' +import { + IconOutline, + IconSolid, + Table, + TableColumn, + Tooltip, +} from '~/libs/ui' import { UserRole } from '~/libs/core' import { handleError } from '~/apps/admin/src/lib/utils' @@ -109,6 +115,13 @@ export const TableCheckpointSubmissions: FC = (props: Props) => { const canReopenGlobally = isAdminUser || hasCopilotRole const challengeId = challengeInfo?.id + const checkpointWinnerMemberIds = useMemo( + () => new Set( + (challengeInfo?.checkpointWinners ?? []) + .map(winner => `${winner.userId}`), + ), + [challengeInfo?.checkpointWinners], + ) const [pendingReopen, setPendingReopen] = useState<{ reviewId: string @@ -597,23 +610,46 @@ export const TableCheckpointSubmissions: FC = (props: Props) => { renderer: (data: Screening) => { const reviewId = data.reviewId const scoreLabel = data.score ?? 'Pending' - - if (!reviewId) { - return {scoreLabel} - } + const isCheckpointWinner = checkpointWinnerMemberIds.has(`${data.memberId}`) return ( - + {reviewId ? ( + + {scoreLabel} + + ) : ( + {scoreLabel} )} - > - {scoreLabel} - + {isCheckpointWinner && ( + + Checkpoint winner. This member is eligible for the checkpoint prize + associated with this challenge. + + )} + triggerOn='click-hover' + > + + + )} + ) }, type: 'element', @@ -765,6 +801,7 @@ export const TableCheckpointSubmissions: FC = (props: Props) => { isSubmissionDownloadRestrictedForMember, getRestrictionMessageForMember, canReopenGlobally, + checkpointWinnerMemberIds, myResourceIds, openReopenDialog, isReopening, diff --git a/src/apps/review/src/lib/models/BackendChallengeInfo.model.spec.ts b/src/apps/review/src/lib/models/BackendChallengeInfo.model.spec.ts index 7328b71bf..98c384fc6 100644 --- a/src/apps/review/src/lib/models/BackendChallengeInfo.model.spec.ts +++ b/src/apps/review/src/lib/models/BackendChallengeInfo.model.spec.ts @@ -11,7 +11,9 @@ jest.mock('~/libs/core', () => ({ const buildChallengeInfo = ( winners: BackendChallengeInfo['winners'], + checkpointWinners?: BackendChallengeInfo['checkpointWinners'], ): BackendChallengeInfo => ({ + checkpointWinners, created: '2026-01-01T00:00:00.000Z', createdBy: 'tester', currentPhaseNames: [], @@ -141,5 +143,54 @@ describe('convertBackendChallengeInfo winners mapping', () => { expect(result?.winners) .toEqual([]) + expect(result?.checkpointWinners) + .toEqual([ + { + handle: 'canonicalCheckpointHandle', + placement: 1, + type: 'CHECKPOINT', + userId: 8888, + }, + { + handle: 'checkpointHandle', + placement: 1, + type: 'Checkpoint Submission', + userId: 9999, + }, + ]) + }) + + it('preserves checkpoint winners separately from final placements', () => { + const result = convertBackendChallengeInfo(buildChallengeInfo( + [{ + handle: 'placementWinner', + placement: 1, + userId: 1234, + }], + [{ + handle: 'checkpointWinner', + placement: 1, + userId: 5678, + }], + )) + + expect(result?.winners) + .toEqual([ + { + handle: 'placementWinner', + maxRating: undefined, + placement: 1, + type: undefined, + userId: 1234, + }, + ]) + expect(result?.checkpointWinners) + .toEqual([ + { + handle: 'checkpointWinner', + placement: 1, + userId: 5678, + }, + ]) }) }) diff --git a/src/apps/review/src/lib/models/BackendChallengeInfo.model.ts b/src/apps/review/src/lib/models/BackendChallengeInfo.model.ts index b342db623..1b076bf94 100644 --- a/src/apps/review/src/lib/models/BackendChallengeInfo.model.ts +++ b/src/apps/review/src/lib/models/BackendChallengeInfo.model.ts @@ -1,7 +1,7 @@ import moment from 'moment' import { formatDurationDate } from '../utils' -import { isContestSubmissionType } from '../constants' +import { isCheckpointSubmissionType, isContestSubmissionType } from '../constants' import { TABLE_DATE_FORMAT } from '../../config/index.config' import { BackendMetadata } from './BackendMetadata.model' @@ -67,6 +67,7 @@ export interface BackendChallengeInfo { numOfRegistrants: number currentPhase?: BackendPhase winners?: BackendChallengeWinner[] | null + checkpointWinners?: BackendChallengeWinner[] | null } function normalizeType( @@ -176,6 +177,12 @@ export function convertBackendChallengeInfo( : undefined const winners: ChallengeWinner[] | undefined = mapWinners(data.winners) + const checkpointWinners: ChallengeWinner[] | undefined = data.checkpointWinners + ?? data.winners?.filter(winner => ( + winner.type?.trim() + .toUpperCase() === 'CHECKPOINT' + || isCheckpointSubmissionType(winner.type) + )) // normalize type/track to objects const normalizedType: ChallengeType = normalizeType(data.type, data.typeId) @@ -183,6 +190,7 @@ export function convertBackendChallengeInfo( return { ...data, + checkpointWinners, currentPhase, currentPhaseEndDate, currentPhaseEndDateString, diff --git a/src/apps/review/src/lib/models/ChallengeInfo.model.ts b/src/apps/review/src/lib/models/ChallengeInfo.model.ts index b2b417230..4d2741a58 100644 --- a/src/apps/review/src/lib/models/ChallengeInfo.model.ts +++ b/src/apps/review/src/lib/models/ChallengeInfo.model.ts @@ -60,6 +60,7 @@ export interface ChallengeInfo { status?: string phases: BackendPhase[] winners?: ChallengeWinner[] + checkpointWinners?: ChallengeWinner[] // Optional: prize sets from backend (placement, copilot, etc.) // Present on the backend response and spread into the converted model. // We include it here so components can read prize configuration safely.