Fix a problem where too many reviewers were showing for a submitter and the Reopen Review button not working

Author: jmgasper

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

@@ -29,7 +29,6 @@ import { useRole, useRoleProps, useSubmissionDownloadAccess } from '../../hooks'
 import type { UseSubmissionDownloadAccessResult } from '../../hooks/useSubmissionDownloadAccess'
 import {
     ChallengeDetailContextModel,
-    ChallengeInfo,
     MappingReviewAppeal,
     ReviewAppContextModel,
     SubmissionInfo,
@@ -92,18 +91,14 @@ function createReopenActionButtons(
     aggregatedReviews: AggregatedReviewDetail[] | undefined,
     {
         canManageCompletedReviews,
-        getLatestChallengeInfo,
         isReopening,
         openReopenDialog,
         pendingReopen,
-        setIsReviewPhaseClosedModalOpen,
     }: {
         canManageCompletedReviews: boolean
-        getLatestChallengeInfo: () => Promise<ChallengeInfo | undefined>
         isReopening: boolean
         openReopenDialog: (submission: SubmissionRow, review: AggregatedReviewDetail) => void
         pendingReopen: { review?: AggregatedReviewDetail } | undefined
-        setIsReviewPhaseClosedModalOpen: (isOpen: boolean) => void
     },
 ): JSX.Element[] {
     if (!canManageCompletedReviews) {
@@ -126,14 +121,7 @@ function createReopenActionButtons(
 
         const isTargetReview = pendingReopen?.review?.reviewInfo?.id === reviewInfo.id
 
-        async function handleReopenClick(): Promise<void> {
-            const latestChallengeInfo = await getLatestChallengeInfo()
-
-            if (!isReviewPhase(latestChallengeInfo)) {
-                setIsReviewPhaseClosedModalOpen(true)
-                return
-            }
-
+        function handleReopenClick(): void {
             openReopenDialog(submission, {
                 ...rv,
                 reviewInfo,
@@ -426,11 +414,6 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
         submission?: SubmissionRow
         isOwnReview?: boolean
     } | undefined>(undefined)
-    const [
-        isReviewPhaseClosedModalOpen,
-        setIsReviewPhaseClosedModalOpen,
-    ] = useState(false)
-
     const myResourceIds = useMemo(
         () => new Set(myResources.map(resource => resource.id)),
         [myResources],
@@ -459,25 +442,6 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
         ],
     )
 
-    const getLatestChallengeInfo = useCallback(async (): Promise<ChallengeInfo | undefined> => {
-        const challengeId = challengeInfo?.id
-
-        if (!challengeId) {
-            return challengeInfo
-        }
-
-        try {
-            const updatedChallengeInfo = await mutate(
-                `challengeBaseUrl/challenges/${challengeId}`,
-            ) as ChallengeInfo | undefined
-
-            return updatedChallengeInfo ?? challengeInfo
-        } catch (error) {
-            handleError(error)
-            return challengeInfo
-        }
-    }, [challengeInfo])
-
     const openReopenDialog = useCallback((submission: SubmissionRow, review: AggregatedReviewDetail) => {
         const resourceId = review.reviewInfo?.resourceId
             ?? review.resourceId
@@ -494,11 +458,6 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
         setPendingReopen(undefined)
     }, [])
 
-    const closeReviewPhaseClosedModal = useCallback(() => {
-        setIsReviewPhaseClosedModalOpen(false)
-        window.location.reload()
-    }, [])
-
     const handleConfirmReopen = useCallback(async () => {
         const reviewId = pendingReopen?.review?.reviewInfo?.id
 
@@ -510,7 +469,7 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
         setIsReopening(true)
 
         try {
-            await updateReview(reviewId, { status: 'IN_PROGRESS' })
+            await updateReview(reviewId, { committed: false, status: 'PENDING' })
             toast.success('Scorecard reopened.')
             closeReopenDialog()
 
@@ -700,8 +659,10 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
                 label: 'Review Score',
                 renderer: (data: SubmissionRow) => {
                     const scoreDisplay = data.aggregated?.averageFinalScoreDisplay
-                    const isReviewInProgress = data.review?.status === 'IN_PROGRESS'
-                    if (!scoreDisplay || isReviewInProgress) {
+                    const reviewStatus = (data.review?.status ?? '').toUpperCase()
+                    const isReviewPending = ['IN_PROGRESS', 'PENDING'].includes(reviewStatus)
+
+                    if (!scoreDisplay || isReviewPending) {
                         return (
                             <span className={styles.statusBadgePending}>
                                 Pending Review
@@ -985,11 +946,9 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
                             data.aggregated?.reviews,
                             {
                                 canManageCompletedReviews,
-                                getLatestChallengeInfo,
                                 isReopening,
                                 openReopenDialog,
                                 pendingReopen,
-                                setIsReviewPhaseClosedModalOpen,
                             },
                         )
 
@@ -1511,8 +1470,6 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
         historyByMember,
         shouldShowHistoryActions,
         myResources,
-        getLatestChallengeInfo,
-        setIsReviewPhaseClosedModalOpen,
     ])
 
     /* eslint-enable indent, padding-line-between-statements */
@@ -1623,16 +1580,6 @@ export const TableReviewAppeals: FC<Props> = (props: Props) => {
                         : REOPEN_MESSAGE_OTHER}
                 </div>
             </ConfirmModal>
-            <ConfirmModal
-                title='Review Phase Closed'
-                open={isReviewPhaseClosedModalOpen}
-                onClose={closeReviewPhaseClosedModal}
-                onConfirm={closeReviewPhaseClosedModal}
-                withoutCancel
-                action='OK'
-            >
-                <div>Review is no longer open for this challenge</div>
-            </ConfirmModal>
         </TableWrapper>
     )
 }

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

@@ -604,6 +604,15 @@ function getNormalizedLowerCase(value?: string | null): string | undefined {
     return trimmedValue ? trimmedValue.toLowerCase() : undefined
 }
 
+function isPhaseAllowedForReview(phaseName?: string | null): boolean {
+    const normalized = getNormalizedLowerCase(phaseName)
+    if (!normalized) {
+        return true
+    }
+
+    return normalized === 'review'
+}
+
 function buildMetadataCriteria(detail: MetadataPhaseMatchDetail): string[] {
     if (!detail) {
         return []
@@ -852,6 +861,7 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
 
     const {
         loginUserInfo,
+        resourceRoleReviewer,
     }: ReviewAppContextModel = useContext(ReviewAppContext)
 
     // get challenge info from challenge detail context
@@ -965,10 +975,19 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
             ?.trim()
             .toLowerCase()
 
+        const reviewerRoleId = resourceRoleReviewer?.id
+
         if (challengeReviewers && challengeReviewers.length) {
             const reviewerRoleResources = filter(
                 challengeReviewers,
-                reviewer => normalizeRoleName(reviewer.roleName) === 'reviewer',
+                reviewer => {
+                    const normalizedRoleName = normalizeRoleName(reviewer.roleName)
+                    const matchesRoleName = normalizedRoleName === 'reviewer'
+                    const matchesRoleId = !normalizedRoleName && reviewerRoleId
+                        ? reviewer.roleId === reviewerRoleId
+                        : false
+                    return matchesRoleName || matchesRoleId
+                },
             )
 
             results = (
@@ -984,13 +1003,24 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
         if (!results.length) {
             const reviewerResourceIds = new Set(
                 (resources ?? [])
-                    .filter(resource => normalizeRoleName(resource.roleName) === 'reviewer')
+                    .filter(resource => {
+                        const normalizedRoleName = normalizeRoleName(resource.roleName)
+                        const matchesRoleName = normalizedRoleName === 'reviewer'
+                        const matchesRoleId = !normalizedRoleName && reviewerRoleId
+                            ? resource.roleId === reviewerRoleId
+                            : false
+                        return matchesRoleName || matchesRoleId
+                    })
                     .map(resource => resource.id)
                     .filter((id): id is string => Boolean(id)),
             )
 
             forEach(visibleChallengeSubmissions, challengeSubmission => {
                 forEach(challengeSubmission.review, review => {
+                    if (!isPhaseAllowedForReview(review.phaseName)) {
+                        return
+                    }
+
                     const resourceId = review.resourceId
                     if (!resourceId) {
                         return
@@ -1015,6 +1045,7 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
         actionChallengeRole,
         loginUserInfo,
         resources,
+        resourceRoleReviewer,
     ])
 
     // fetch appeal response
@@ -1039,7 +1070,8 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
         `reviewBaseUrl/reviews/${challengeId}/${reviewerKey}`,
         {
             fetcher: () => fetchChallengeReviews(challengeId ?? ''),
-            isPaused: () => !challengeId || !reviewerIds.length,
+            isPaused: () => !challengeId
+                || (!reviewerIds.length && actionChallengeRole !== SUBMITTER),
         },
     )
 
@@ -1819,13 +1851,46 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
             return challengeReviews ?? []
         }
 
+        const allowedReviewerIds = reviewerIds.length ? new Set(reviewerIds) : undefined
+
         if (challengeReviews && challengeReviews.length) {
-            return challengeReviews
+            const filteredReviews = (challengeReviews ?? []).filter(review => {
+                if (!review) {
+                    return false
+                }
+
+                if (!isPhaseAllowedForReview(review.phaseName)) {
+                    return false
+                }
+
+                if (allowedReviewerIds?.size) {
+                    return Boolean(
+                        review.resourceId && allowedReviewerIds.has(review.resourceId),
+                    )
+                }
+
+                return true
+            })
+
+            if (filteredReviews.length) {
+                return filteredReviews
+            }
         }
 
         const fallbackReviews: BackendReview[] = []
         forEach(visibleChallengeSubmissions, submission => {
             forEach(submission.review, reviewItem => {
+                if (!isPhaseAllowedForReview(reviewItem?.phaseName)) {
+                    return
+                }
+
+                const resourceId = reviewItem?.resourceId
+                if (allowedReviewerIds?.size) {
+                    if (!resourceId || !allowedReviewerIds.has(resourceId)) {
+                        return
+                    }
+                }
+
                 if (reviewItem?.id) {
                     fallbackReviews.push({
                         ...reviewItem,
@@ -1840,11 +1905,16 @@ export function useFetchScreeningReview(): useFetchScreeningReviewProps {
             return fallbackReviews
         }
 
-        return challengeReviews ?? []
+        const filteredByPhase = (challengeReviews ?? []).filter(review => (
+            review ? isPhaseAllowedForReview(review.phaseName) : false
+        ))
+
+        return filteredByPhase
     }, [
         actionChallengeRole,
         challengeReviews,
         visibleChallengeSubmissions,
+        reviewerIds,
     ])
 
     // get review data from challenge submissions