Refactoring of large hook, and various QA fixes

Author: jmgasper

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

@@ -7,7 +7,6 @@ import { ActionLoading } from '~/apps/admin/src/lib'
 
 import { ChallengeDetailContext } from '../../contexts'
 import {
-    BackendPhase,
     BackendSubmission,
     ChallengeInfo,
     MappingReviewAppeal,
@@ -29,6 +28,7 @@ import {
 import { ITERATIVE_REVIEW, SUBMITTER } from '../../../config/index.config'
 import { TableNoRecord } from '../TableNoRecord'
 import { hasIsLatestFlag } from '../../utils'
+import { shouldIncludeInReviewPhase } from '../../utils/reviewPhaseGuards'
 
 import TabContentApproval from './TabContentApproval'
 import TabContentCheckpoint from './TabContentCheckpoint'
@@ -47,122 +47,6 @@ const normalizeType = (value?: string): string => (
         : ''
 )
 
-const resolvePhaseNameFromId = (
-    phaseId?: string,
-    phases?: BackendPhase[],
-): string | undefined => {
-    if (!phaseId || !Array.isArray(phases)) {
-        return undefined
-    }
-
-    const normalizedPhaseId = `${phaseId}`.trim()
-        .toLowerCase()
-    if (!normalizedPhaseId) {
-        return undefined
-    }
-
-    const matchingPhase = phases.find(phase => {
-        const candidates = [phase.id, phase.phaseId]
-
-        return candidates.some(candidate => (
-            typeof candidate === 'string'
-            && candidate.trim()
-                .toLowerCase() === normalizedPhaseId
-        ))
-    })
-
-    return matchingPhase?.name
-}
-
-const EXCLUDED_REVIEW_TYPES = [
-    'approval',
-    'checkpoint',
-    'iterative',
-    'postmortem',
-    'screening',
-    'specification',
-] as const
-
-const shouldIncludeInReviewPhase = (
-    submission?: SubmissionInfo,
-    phases?: BackendPhase[],
-): boolean => {
-    if (!submission) {
-        return false
-    }
-
-    const normalizedCandidates = new Set<string>()
-    const registerCandidate = (candidate?: string | null): void => {
-        if (typeof candidate !== 'string') {
-            return
-        }
-
-        const normalized = normalizeType(candidate)
-        if (normalized) {
-            normalizedCandidates.add(normalized)
-        }
-    }
-
-    registerCandidate(submission.reviewTypeId)
-    registerCandidate(submission.review?.phaseName)
-    registerCandidate(submission.review?.reviewType)
-    registerCandidate(resolvePhaseNameFromId(submission.review?.phaseId, phases))
-
-    const metadata = submission.review?.metadata
-    if (metadata && typeof metadata === 'object') {
-        const metadataHints: unknown[] = [
-            (metadata as { type?: unknown }).type,
-            (metadata as { reviewType?: unknown }).reviewType,
-            (metadata as { review_type?: unknown }).review_type,
-            (metadata as { phase?: unknown }).phase,
-            (metadata as { phaseName?: unknown }).phaseName,
-            (metadata as { reviewPhase?: unknown }).reviewPhase,
-        ]
-
-        metadataHints.forEach(hint => {
-            if (typeof hint === 'string') {
-                registerCandidate(hint)
-            }
-        })
-    }
-
-    if (!normalizedCandidates.size) {
-        if (process.env.NODE_ENV !== 'production') {
-            // eslint-disable-next-line no-console
-            console.debug('[ReviewTab] shouldIncludeInReviewPhase', {
-                normalizedCandidates: [],
-                reviewPhaseName: submission.review?.phaseName,
-                reviewType: submission.review?.reviewType,
-                reviewTypeId: submission.reviewTypeId,
-                submissionId: submission.id ?? submission.review?.submissionId,
-                verdict: 'included (no candidates)',
-            })
-        }
-
-        return true
-    }
-
-    const normalizedCandidateList = Array.from(normalizedCandidates)
-    const isExcluded = normalizedCandidateList
-        .some(normalizedType => (
-            EXCLUDED_REVIEW_TYPES.some(fragment => normalizedType.includes(fragment))
-        ))
-
-    if (process.env.NODE_ENV !== 'production') {
-        // eslint-disable-next-line no-console
-        console.debug('[ReviewTab] shouldIncludeInReviewPhase', {
-            normalizedCandidates: normalizedCandidateList,
-            reviewPhaseName: submission.review?.phaseName,
-            reviewType: submission.review?.reviewType,
-            reviewTypeId: submission.reviewTypeId,
-            submissionId: submission.id ?? submission.review?.submissionId,
-            verdict: isExcluded ? 'excluded' : 'included',
-        })
-    }
-
-    return !isExcluded
-}
-
 interface Props {
     selectedTab: string
     isLoadingSubmission: boolean
@@ -244,11 +128,9 @@ const renderSubmissionTab = ({
 }: SubmissionTabParams): JSX.Element => {
     const isSubmissionTab = selectedTabNormalized === 'submission'
     const isTopgearSubmissionTab = selectedTabNormalized === 'topgearsubmission'
-    const shouldRestrictToContestSubmissions = isActiveChallenge
-        && (
-            selectedTabNormalized.startsWith('submission')
-            || isTopgearSubmissionTab
-        )
+    const shouldRestrictToContestSubmissions = selectedTabNormalized
+        .startsWith('submission')
+        || isTopgearSubmissionTab
     const visibleSubmissions = shouldRestrictToContestSubmissions
         ? submissions.filter(
             submission => normalizeType(submission.type) === 'contestsubmission',

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

@@ -15,7 +15,6 @@ import {
     ChallengeDetailContext,
 } from '../../contexts'
 import {
-    BackendPhase,
     BackendResource,
     BackendSubmission,
     ChallengeDetailContextModel,
@@ -38,6 +37,7 @@ import {
     REVIEWER,
     SUBMITTER,
 } from '../../../config/index.config'
+import { shouldIncludeInReviewPhase } from '../../utils/reviewPhaseGuards'
 
 interface Props {
     selectedTab: string
@@ -316,7 +316,7 @@ export const TabContentReview: FC<Props> = (props: Props) => {
     const resolvedReviews = useMemo(
         () => {
             if (providedReviews.length) {
-                return providedReviews.filter(submission => shouldDisplayOnReviewTab(
+                return providedReviews.filter(submission => shouldIncludeInReviewPhase(
                     submission,
                     challengeInfo?.phases,
                 ))
@@ -339,7 +339,7 @@ export const TabContentReview: FC<Props> = (props: Props) => {
                             },
                         }
                         const converted = convertBackendSubmissionToSubmissionInfo(submissionForReviewer)
-                        if (shouldDisplayOnReviewTab(converted, challengeInfo?.phases)) {
+                        if (shouldIncludeInReviewPhase(converted, challengeInfo?.phases)) {
                             fallbackFromBackend.push(converted)
                         }
                     })
@@ -355,13 +355,13 @@ export const TabContentReview: FC<Props> = (props: Props) => {
             }
 
             const fallback = challengeSubmissions.filter(hasReviewerAssignment)
-            const filteredFallback = fallback.filter(submission => shouldDisplayOnReviewTab(
+            const filteredFallback = fallback.filter(submission => shouldIncludeInReviewPhase(
                 submission,
                 challengeInfo?.phases,
             ))
             return filteredFallback.length
                 ? filteredFallback
-                : providedReviews.filter(submission => shouldDisplayOnReviewTab(
+                : providedReviews.filter(submission => shouldIncludeInReviewPhase(
                     submission,
                     challengeInfo?.phases,
                 ))
@@ -404,7 +404,7 @@ export const TabContentReview: FC<Props> = (props: Props) => {
                     return false
                 }
 
-                if (!shouldDisplayOnReviewTab(submission, challengeInfo?.phases)) {
+                if (!shouldIncludeInReviewPhase(submission, challengeInfo?.phases)) {
                     return false
                 }
 
@@ -433,7 +433,7 @@ export const TabContentReview: FC<Props> = (props: Props) => {
                 return fallback
             }
 
-            return providedSubmitterReviews.filter(submission => shouldDisplayOnReviewTab(
+            return providedSubmitterReviews.filter(submission => shouldIncludeInReviewPhase(
                 submission,
                 challengeInfo?.phases,
             ))
@@ -549,60 +549,3 @@ export const TabContentReview: FC<Props> = (props: Props) => {
 }
 
 export default TabContentReview
-function normalizeReviewType(value?: string | null): string {
-    if (!value) {
-        return ''
-    }
-
-    return value
-        .toLowerCase()
-        .replace(/[^a-z]/g, '')
-}
-
-const EXCLUDED_REVIEW_TYPE_FRAGMENTS = [
-    'approval',
-    'checkpoint',
-    'iterative',
-    'postmortem',
-    'screening',
-    'specification',
-] as const
-
-function shouldDisplayOnReviewTab(
-    submission: SubmissionInfo,
-    phases?: BackendPhase[],
-): boolean {
-    const normalized = new Set<string>()
-    const register = (candidate?: string | null): void => {
-        const normalizedCandidate = normalizeReviewType(candidate)
-        if (normalizedCandidate) {
-            normalized.add(normalizedCandidate)
-        }
-    }
-
-    register(submission.reviewTypeId)
-    register(submission.review?.reviewType)
-    register(submission.review?.phaseName)
-
-    const phaseId = submission.review?.phaseId
-    if (phaseId && phases?.length) {
-        const normalizedPhaseId = `${phaseId}`.trim()
-            .toLowerCase()
-        const matchingPhase = phases.find(phase => (
-            (phase.id && phase.id.toLowerCase() === normalizedPhaseId)
-            || (phase.phaseId && phase.phaseId.toLowerCase() === normalizedPhaseId)
-        ))
-        if (matchingPhase?.name) {
-            register(matchingPhase.name)
-        }
-    }
-
-    if (!normalized.size) {
-        return true
-    }
-
-    return !Array.from(normalized)
-        .some(candidate => (
-            EXCLUDED_REVIEW_TYPE_FRAGMENTS.some(fragment => candidate.includes(fragment))
-        ))
-}

src/apps/review/src/lib/components/TableIterativeReview/TableIterativeReview.module.scss

@@ -142,6 +142,28 @@
     color: var(--GrayFontColor);
 }
 
+.resultPill {
+    display: inline-flex;
+    align-items: center;
+    justify-content: center;
+    padding: $sp-1 $sp-2;
+    border-radius: 999px;
+    font-size: 12px;
+    font-weight: 700;
+}
+
+.resultPass {
+    @extend .resultPill;
+    background-color: $green-25;
+    color: $green-140;
+}
+
+.resultFail {
+    @extend .resultPill;
+    background-color: $red-25;
+    color: $red-140;
+}
+
 .submit {
     align-items: center;
     display: flex;