Make sure all reviews render correctly on the 'Review' tab

Author: jmgasper

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

@@ -247,71 +247,9 @@ export const ChallengeDetailsContent: FC<Props> = (props: Props) => {
         ),
         [challengeInfo?.phases],
     )
-    const disallowedReviewSets = useMemo<{
-        disallowedReviewIds: Set<string>
-        disallowedReviewPhaseIds: Set<string>
-    }>(
-        () => {
-            const reviewIds = new Set<string>()
-            const phaseIds = new Set<string>()
-            const addReviewId = (id?: string): void => {
-                if (id) {
-                    reviewIds.add(id)
-                }
-            }
-
-            const addPhaseId = (id?: string): void => {
-                if (id) {
-                    phaseIds.add(id)
-                }
-            }
-
-            props.screening.forEach(entry => {
-                addReviewId(entry.reviewId)
-                addPhaseId(entry.reviewPhaseId)
-            })
-            props.checkpoint.forEach(entry => {
-                addReviewId(entry.reviewId)
-                addPhaseId(entry.reviewPhaseId)
-            })
-            props.checkpointReview.forEach(entry => {
-                addReviewId(entry.reviewId)
-                addPhaseId(entry.reviewPhaseId)
-            })
-
-            return {
-                disallowedReviewIds: reviewIds,
-                disallowedReviewPhaseIds: phaseIds,
-            }
-        },
-        [props.screening, props.checkpoint, props.checkpointReview],
-    )
-    const {
-        disallowedReviewIds,
-        disallowedReviewPhaseIds,
-    }: {
-        disallowedReviewIds: Set<string>
-        disallowedReviewPhaseIds: Set<string>
-    } = disallowedReviewSets
     const passesReviewTabGuards: (submission: SubmissionInfo) => boolean = useMemo(
-        () => (submission: SubmissionInfo): boolean => {
-            if (!shouldIncludeInReviewPhase(submission)) {
-                return false
-            }
-
-            const reviewId = submission.review?.id
-            if (reviewId && disallowedReviewIds.has(reviewId)) {
-                return false
-            }
-
-            const reviewPhaseId = submission.review?.phaseId
-            if (reviewPhaseId && disallowedReviewPhaseIds.has(reviewPhaseId)) {
-                return false
-            }
-
-            return true
-        },
-        [disallowedReviewIds, disallowedReviewPhaseIds],
+        () => (submission: SubmissionInfo): boolean => shouldIncludeInReviewPhase(submission),
+        [],
     )
     const {
         reviews: reviewTabReviews,

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

@@ -14,7 +14,6 @@ import classNames from 'classnames'
 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 { getRatingColor } from '~/libs/core'
 import { handleError, useWindowSize, WindowSize } from '~/libs/shared'
 import { IconOutline, Table, TableColumn } from '~/libs/ui'
 
@@ -225,24 +224,6 @@ export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
         [datas, latestSubmissions, restrictToLatest],
     )
 
-    if (process.env.NODE_ENV !== 'production') {
-        try {
-            console.debug('[TableReview] submissionsForAggregation', submissionsForAggregation.map(submission => ({
-                id: submission.id,
-                reviewResourceId: submission.review?.resourceId,
-                reviewHandle: submission.review?.reviewerHandle,
-                reviewId: submission.review?.id,
-                reviews: submission.reviews?.map(review => ({
-                    resourceId: review.resourceId,
-                    reviewerHandle: review.reviewerHandle,
-                    score: review.score,
-                })),
-            })))
-        } catch {
-            // ignore logging issues
-        }
-    }
-
     const aggregatedSubmissionRows = useMemo<AggregatedSubmissionReviews[]>(() => (
         aggregateSubmissionReviews({
             mappingReviewAppeal,
@@ -299,65 +280,10 @@ export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
         renderLabel?: () => JSX.Element
     }
     const reviewerColumnMetadata = useMemo<ReviewerColumnMetadata[]>(() => (
-        Array.from({ length: maxReviewCount }, (unused, index) => {
-            const reviewerDetails = aggregatedSubmissionRows
-                .map(aggregated => aggregated.reviews?.[index])
-                .filter((detail): detail is AggregatedReviewDetail => Boolean(detail))
-
-            const resolvedHandle = reviewerDetails
-                .map(detail => detail.reviewerHandle?.trim()
-                    || detail.reviewInfo?.reviewerHandle?.trim()
-                    || undefined)
-                .find((handle): handle is string => Boolean(handle))
-
-            if (!resolvedHandle) {
-                return {
-                    label: `Reviewer ${index + 1}`,
-                }
-            }
-
-            const resolvedRating = reviewerDetails
-                .map(detail => detail.reviewerMaxRating
-                    ?? detail.reviewInfo?.reviewerMaxRating
-                    ?? undefined)
-                .find((rating): rating is number => (
-                    typeof rating === 'number'
-                    && Number.isFinite(rating)
-                ))
-
-            const resolvedColor = reviewerDetails
-                .map(detail => detail.reviewerHandleColor
-                    ?? detail.reviewInfo?.reviewerHandleColor
-                    ?? undefined)
-                .find((color): color is string => Boolean(color))
-
-            const ratingDisplay = resolvedRating !== undefined
-                ? Math.round(resolvedRating)
-                : undefined
-            const baseLabel = ratingDisplay !== undefined
-                ? `${resolvedHandle} (${ratingDisplay})`
-                : resolvedHandle
-
-            const renderLabel = (): JSX.Element => {
-                const color = resolvedColor
-                    ?? (resolvedRating !== undefined
-                        ? getRatingColor(resolvedRating)
-                        : undefined)
-                    ?? '#2a2a2a'
-
-                return (
-                    <span style={{ color }}>
-                        {baseLabel}
-                    </span>
-                )
-            }
-
-            return {
-                label: baseLabel,
-                renderLabel,
-            }
-        })
-    ), [aggregatedSubmissionRows, maxReviewCount])
+        Array.from({ length: maxReviewCount }, (_unused, index) => ({
+            label: `Reviewer ${index + 1}`,
+        }))
+    ), [maxReviewCount])
 
     const [isReopening, setIsReopening] = useState(false)
     const [pendingReopen, setPendingReopen] = useState<PendingReopenState | undefined>(undefined)

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

@@ -25,6 +25,7 @@ import {
     SubmissionInfo,
 } from '../models'
 import { fetchChallengeReviews } from '../services'
+import { registerChallengeReviewKey } from '../utils/reviewCacheRegistry'
 
 import { useFetchAppealQueue, useFetchAppealQueueProps } from './useFetchAppealQueue'
 import { useFetchChallengeSubmissions, useFetchChallengeSubmissionsProps } from './useFetchChallengeSubmissions'
@@ -1087,15 +1088,7 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
             })
         })
 
-        const resolvedReviewerIds = Array.from(fallbackResults)
-
-        debugLog('reviewerIds', {
-            challengeId,
-            reviewerCount: resolvedReviewerIds.length,
-            reviewerIds: resolvedReviewerIds,
-        })
-
-        return resolvedReviewerIds
+        return Array.from(fallbackResults)
 
     }, [
         challengeReviewers,
@@ -1135,6 +1128,14 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
         },
     )
 
+    useEffect(() => {
+        if (!challengeId) {
+            return
+        }
+
+        registerChallengeReviewKey(challengeId, `reviewBaseUrl/reviews/${challengeId}/${reviewerKey}`)
+    }, [challengeId, reviewerKey])
+
     const challengeReviews = useMemo(
         () => challengeReviewsData,
         [challengeReviewsData],
@@ -1326,23 +1327,6 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
         [challengeReviews, reviewerIds],
     )
 
-    const reviewAssignmentSummary = useMemo(
-        () => Object.entries(reviewAssignmentsBySubmission)
-            .map(
-                ([submissionId, reviewerMapping]) => ({
-                    reviewerIds: Object.keys(reviewerMapping),
-                    submissionId,
-                }),
-            ),
-        [reviewAssignmentsBySubmission],
-    )
-
-    debugLog('reviewAssignments', {
-        assignments: reviewAssignmentSummary,
-        challengeId,
-        submissionCount: reviewAssignmentSummary.length,
-    })
-
     // get screening data from challenge submissions
     const screening = useMemo(
         () => {
@@ -2185,19 +2169,6 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
                     reviewForResource = emptyReview
                 }
 
-                debugLog('reviewEntry', {
-                    challengeId,
-                    finalScore: reviewForResource?.finalScore,
-                    origin: assignmentReview
-                        ? (matchingReview ? 'assignment+existing' : 'assignment')
-                        : (matchingReview ? 'existing' : 'empty'),
-                    reviewerHandle: reviewForResource?.reviewerHandle,
-                    reviewerId,
-                    reviewId: reviewForResource?.id,
-                    status: reviewForResource?.status,
-                    submissionId: challengeSubmission.id,
-                })
-
                 validReviews.push({
                     ...challengeSubmission,
                     review: [reviewForResource],
@@ -2211,15 +2182,6 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
         return validReviews.map(item => {
             const result = convertBackendSubmissionToSubmissionInfo(item)
 
-            debugLog('reviewSubmissionInfo', {
-                challengeId,
-                submissionId: result.id,
-                reviewerId: result.review?.resourceId,
-                reviewerHandle: result.review?.reviewerHandle,
-                reviewId: result.review?.id,
-                finalScore: result.review?.finalScore,
-            })
-
             return {
                 ...result,
                 userInfo: resourceMemberIdMapping[result.memberId],

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

@@ -139,19 +139,6 @@ export function aggregateSubmissionReviews({
             ? find(submission.reviews, reviewResult => reviewResult.resourceId === resourceId)
             : undefined
         if (!reviewId) {
-            if (process.env.NODE_ENV !== 'production') {
-                try {
-                    console.debug('[aggregateSubmissionReviews] skipped review without id', {
-                        resourceId,
-                        reviewInfo,
-                        reviewResult: matchingReviewResult,
-                        submissionId: submission.id,
-                    })
-                } catch {
-                    // ignore logging failures
-                }
-            }
-
             return
         }
 
@@ -400,24 +387,6 @@ export function aggregateSubmissionReviews({
         })
     })
 
-    if (process.env.NODE_ENV !== 'production') {
-        try {
-            console.debug('[aggregateSubmissionReviews] summaries', aggregatedRows.map(row => ({
-                reviews: row.reviews.map(review => ({
-                    finalScore: review.finalScore
-                        ?? review.reviewInfo?.finalScore,
-                    resourceId: review.resourceId,
-                    reviewerHandle: review.reviewerHandle
-                        ?? review.reviewInfo?.reviewerHandle,
-                    reviewId: review.reviewId ?? review.reviewInfo?.id,
-                })),
-                submissionId: row.id,
-            })))
-        } catch {
-            // ignore logging errors
-        }
-    }
-
     return aggregatedRows
 }
 /* eslint-enable complexity */

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

@@ -0,0 +1,43 @@
+/**
+ * Registry tracking SWR cache keys that contain challenge-review data.
+ *
+ * The registry lets utility functions invalidate every relevant cache entry
+ * after mutations (for example, when reopening a review) without needing
+ * component-specific context.
+ */
+
+const challengeReviewKeyRegistry = new Map<string, Set<string>>()
+
+/**
+ * Record a cache key that should be revalidated when review data changes.
+ */
+export const registerChallengeReviewKey = (challengeId: string | undefined, cacheKey: string | undefined): void => {
+    if (!challengeId || !cacheKey) {
+        return
+    }
+
+    const normalizedKey = cacheKey.trim()
+    if (!normalizedKey.length) {
+        return
+    }
+
+    const registryEntry = challengeReviewKeyRegistry.get(challengeId)
+    if (registryEntry) {
+        registryEntry.add(normalizedKey)
+        return
+    }
+
+    challengeReviewKeyRegistry.set(challengeId, new Set<string>([normalizedKey]))
+}
+
+/**
+ * Retrieve all registered cache keys for the provided challenge.
+ */
+export const getChallengeReviewKeys = (challengeId: string | undefined): string[] => {
+    if (!challengeId) {
+        return []
+    }
+
+    const entry = challengeReviewKeyRegistry.get(challengeId)
+    return entry ? Array.from(entry) : []
+}