Skip to content

Commit 2386d72

Browse files
committed
Make sure we don't show screenign on review tabs
1 parent 7a8f7dc commit 2386d72

File tree

5 files changed

+352
-108
lines changed

5 files changed

+352
-108
lines changed

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

Lines changed: 104 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { ActionLoading } from '~/apps/admin/src/lib'
77

88
import { ChallengeDetailContext } from '../../contexts'
99
import {
10+
BackendPhase,
1011
BackendSubmission,
1112
ChallengeInfo,
1213
MappingReviewAppeal,
@@ -46,6 +47,33 @@ const normalizeType = (value?: string): string => (
4647
: ''
4748
)
4849

50+
const resolvePhaseNameFromId = (
51+
phaseId?: string,
52+
phases?: BackendPhase[],
53+
): string | undefined => {
54+
if (!phaseId || !Array.isArray(phases)) {
55+
return undefined
56+
}
57+
58+
const normalizedPhaseId = `${phaseId}`.trim()
59+
.toLowerCase()
60+
if (!normalizedPhaseId) {
61+
return undefined
62+
}
63+
64+
const matchingPhase = phases.find(phase => {
65+
const candidates = [phase.id, phase.phaseId]
66+
67+
return candidates.some(candidate => (
68+
typeof candidate === 'string'
69+
&& candidate.trim()
70+
.toLowerCase() === normalizedPhaseId
71+
))
72+
})
73+
74+
return matchingPhase?.name
75+
}
76+
4977
const EXCLUDED_REVIEW_TYPES = [
5078
'approval',
5179
'checkpoint',
@@ -55,17 +83,84 @@ const EXCLUDED_REVIEW_TYPES = [
5583
'specification',
5684
] as const
5785

58-
const shouldIncludeInReviewPhase = (submission?: SubmissionInfo): boolean => {
86+
const shouldIncludeInReviewPhase = (
87+
submission?: SubmissionInfo,
88+
phases?: BackendPhase[],
89+
): boolean => {
5990
if (!submission) {
6091
return false
6192
}
6293

63-
const normalizedType = normalizeType(submission.reviewTypeId)
64-
if (!normalizedType) {
94+
const normalizedCandidates = new Set<string>()
95+
const registerCandidate = (candidate?: string | null): void => {
96+
if (typeof candidate !== 'string') {
97+
return
98+
}
99+
100+
const normalized = normalizeType(candidate)
101+
if (normalized) {
102+
normalizedCandidates.add(normalized)
103+
}
104+
}
105+
106+
registerCandidate(submission.reviewTypeId)
107+
registerCandidate(submission.review?.phaseName)
108+
registerCandidate(submission.review?.reviewType)
109+
registerCandidate(resolvePhaseNameFromId(submission.review?.phaseId, phases))
110+
111+
const metadata = submission.review?.metadata
112+
if (metadata && typeof metadata === 'object') {
113+
const metadataHints: unknown[] = [
114+
(metadata as { type?: unknown }).type,
115+
(metadata as { reviewType?: unknown }).reviewType,
116+
(metadata as { review_type?: unknown }).review_type,
117+
(metadata as { phase?: unknown }).phase,
118+
(metadata as { phaseName?: unknown }).phaseName,
119+
(metadata as { reviewPhase?: unknown }).reviewPhase,
120+
]
121+
122+
metadataHints.forEach(hint => {
123+
if (typeof hint === 'string') {
124+
registerCandidate(hint)
125+
}
126+
})
127+
}
128+
129+
if (!normalizedCandidates.size) {
130+
if (process.env.NODE_ENV !== 'production') {
131+
// eslint-disable-next-line no-console
132+
console.debug('[ReviewTab] shouldIncludeInReviewPhase', {
133+
normalizedCandidates: [],
134+
reviewPhaseName: submission.review?.phaseName,
135+
reviewType: submission.review?.reviewType,
136+
reviewTypeId: submission.reviewTypeId,
137+
submissionId: submission.id ?? submission.review?.submissionId,
138+
verdict: 'included (no candidates)',
139+
})
140+
}
141+
65142
return true
66143
}
67144

68-
return !EXCLUDED_REVIEW_TYPES.some(fragment => normalizedType.includes(fragment))
145+
const normalizedCandidateList = Array.from(normalizedCandidates)
146+
const isExcluded = normalizedCandidateList
147+
.some(normalizedType => (
148+
EXCLUDED_REVIEW_TYPES.some(fragment => normalizedType.includes(fragment))
149+
))
150+
151+
if (process.env.NODE_ENV !== 'production') {
152+
// eslint-disable-next-line no-console
153+
console.debug('[ReviewTab] shouldIncludeInReviewPhase', {
154+
normalizedCandidates: normalizedCandidateList,
155+
reviewPhaseName: submission.review?.phaseName,
156+
reviewType: submission.review?.reviewType,
157+
reviewTypeId: submission.reviewTypeId,
158+
submissionId: submission.id ?? submission.review?.submissionId,
159+
verdict: isExcluded ? 'excluded' : 'included',
160+
})
161+
}
162+
163+
return !isExcluded
69164
}
70165

71166
interface Props {
@@ -248,8 +343,11 @@ export const ChallengeDetailsContent: FC<Props> = (props: Props) => {
248343
[challengeInfo?.phases],
249344
)
250345
const passesReviewTabGuards: (submission: SubmissionInfo) => boolean = useMemo(
251-
() => (submission: SubmissionInfo): boolean => shouldIncludeInReviewPhase(submission),
252-
[],
346+
() => (submission: SubmissionInfo): boolean => shouldIncludeInReviewPhase(
347+
submission,
348+
challengeInfo?.phases,
349+
),
350+
[challengeInfo?.phases],
253351
)
254352
const {
255353
reviews: reviewTabReviews,

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

Lines changed: 90 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
ChallengeDetailContext,
1616
} from '../../contexts'
1717
import {
18+
BackendPhase,
1819
BackendResource,
1920
BackendSubmission,
2021
ChallengeDetailContextModel,
@@ -315,7 +316,10 @@ export const TabContentReview: FC<Props> = (props: Props) => {
315316
const resolvedReviews = useMemo(
316317
() => {
317318
if (providedReviews.length) {
318-
return providedReviews
319+
return providedReviews.filter(submission => shouldDisplayOnReviewTab(
320+
submission,
321+
challengeInfo?.phases,
322+
))
319323
}
320324

321325
const fallbackFromBackend: SubmissionInfo[] = []
@@ -334,9 +338,10 @@ export const TabContentReview: FC<Props> = (props: Props) => {
334338
[review.resourceId]: review,
335339
},
336340
}
337-
fallbackFromBackend.push(
338-
convertBackendSubmissionToSubmissionInfo(submissionForReviewer),
339-
)
341+
const converted = convertBackendSubmissionToSubmissionInfo(submissionForReviewer)
342+
if (shouldDisplayOnReviewTab(converted, challengeInfo?.phases)) {
343+
fallbackFromBackend.push(converted)
344+
}
340345
})
341346
})
342347
}
@@ -350,11 +355,21 @@ export const TabContentReview: FC<Props> = (props: Props) => {
350355
}
351356

352357
const fallback = challengeSubmissions.filter(hasReviewerAssignment)
353-
return fallback.length ? fallback : providedReviews
358+
const filteredFallback = fallback.filter(submission => shouldDisplayOnReviewTab(
359+
submission,
360+
challengeInfo?.phases,
361+
))
362+
return filteredFallback.length
363+
? filteredFallback
364+
: providedReviews.filter(submission => shouldDisplayOnReviewTab(
365+
submission,
366+
challengeInfo?.phases,
367+
))
354368
},
355369
[
356370
backendChallengeSubmissions,
357371
challengeSubmissions,
372+
challengeInfo?.phases,
358373
hasReviewerAssignment,
359374
myReviewerResourceIds,
360375
providedReviews,
@@ -389,6 +404,10 @@ export const TabContentReview: FC<Props> = (props: Props) => {
389404
return false
390405
}
391406

407+
if (!shouldDisplayOnReviewTab(submission, challengeInfo?.phases)) {
408+
return false
409+
}
410+
392411
const submissionMemberId = submission.memberId
393412
? `${submission.memberId}`.trim()
394413
: ''
@@ -410,11 +429,19 @@ export const TabContentReview: FC<Props> = (props: Props) => {
410429
})
411430
.map(enhanceSubmissionForSubmitter)
412431

413-
return fallback.length ? fallback : providedSubmitterReviews
432+
if (fallback.length) {
433+
return fallback
434+
}
435+
436+
return providedSubmitterReviews.filter(submission => shouldDisplayOnReviewTab(
437+
submission,
438+
challengeInfo?.phases,
439+
))
414440
},
415441
[
416442
challengeSubmissions,
417443
enhanceSubmissionForSubmitter,
444+
challengeInfo?.phases,
418445
myOwnedMemberIds,
419446
myOwnedSubmissionIds,
420447
providedReviews,
@@ -522,3 +549,60 @@ export const TabContentReview: FC<Props> = (props: Props) => {
522549
}
523550

524551
export default TabContentReview
552+
function normalizeReviewType(value?: string | null): string {
553+
if (!value) {
554+
return ''
555+
}
556+
557+
return value
558+
.toLowerCase()
559+
.replace(/[^a-z]/g, '')
560+
}
561+
562+
const EXCLUDED_REVIEW_TYPE_FRAGMENTS = [
563+
'approval',
564+
'checkpoint',
565+
'iterative',
566+
'postmortem',
567+
'screening',
568+
'specification',
569+
] as const
570+
571+
function shouldDisplayOnReviewTab(
572+
submission: SubmissionInfo,
573+
phases?: BackendPhase[],
574+
): boolean {
575+
const normalized = new Set<string>()
576+
const register = (candidate?: string | null): void => {
577+
const normalizedCandidate = normalizeReviewType(candidate)
578+
if (normalizedCandidate) {
579+
normalized.add(normalizedCandidate)
580+
}
581+
}
582+
583+
register(submission.reviewTypeId)
584+
register(submission.review?.reviewType)
585+
register(submission.review?.phaseName)
586+
587+
const phaseId = submission.review?.phaseId
588+
if (phaseId && phases?.length) {
589+
const normalizedPhaseId = `${phaseId}`.trim()
590+
.toLowerCase()
591+
const matchingPhase = phases.find(phase => (
592+
(phase.id && phase.id.toLowerCase() === normalizedPhaseId)
593+
|| (phase.phaseId && phase.phaseId.toLowerCase() === normalizedPhaseId)
594+
))
595+
if (matchingPhase?.name) {
596+
register(matchingPhase.name)
597+
}
598+
}
599+
600+
if (!normalized.size) {
601+
return true
602+
}
603+
604+
return !Array.from(normalized)
605+
.some(candidate => (
606+
EXCLUDED_REVIEW_TYPE_FRAGMENTS.some(fragment => candidate.includes(fragment))
607+
))
608+
}

0 commit comments

Comments
 (0)