Skip to content

Commit 7a8f7dc

Browse files
committed
Don't show pass / fail until all reviews are complete
1 parent 88ff920 commit 7a8f7dc

File tree

2 files changed

+175
-1
lines changed

2 files changed

+175
-1
lines changed
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import { resolveSubmissionReviewResult } from './reviewResult'
2+
import type { AggregatedReviewDetail, SubmissionRow } from './types'
3+
4+
const DEFAULT_PASSING_SCORE = 80
5+
6+
const buildSubmission = (
7+
reviews: AggregatedReviewDetail[],
8+
averageFinalScore?: number,
9+
): SubmissionRow => {
10+
const submissionInfo = {
11+
id: 'submission-1',
12+
memberId: 'member-1',
13+
}
14+
15+
return {
16+
...submissionInfo,
17+
aggregated: {
18+
averageFinalScore,
19+
id: submissionInfo.id,
20+
reviews: reviews.map(review => ({ ...review })),
21+
submission: submissionInfo,
22+
},
23+
}
24+
}
25+
26+
describe('resolveSubmissionReviewResult', () => {
27+
it('returns undefined when any reviewer is still pending in a multi-review setup', () => {
28+
const submission = buildSubmission([
29+
{
30+
finalScore: 92,
31+
resourceId: 'res-1',
32+
status: 'COMPLETED',
33+
},
34+
{
35+
resourceId: 'res-2',
36+
},
37+
], 92)
38+
39+
const result = resolveSubmissionReviewResult(submission, {
40+
defaultMinimumPassingScore: DEFAULT_PASSING_SCORE,
41+
})
42+
43+
expect(result)
44+
.toBeUndefined()
45+
})
46+
47+
it('returns PASS once all reviewers have completed their assessments', () => {
48+
const submission = buildSubmission([
49+
{
50+
finalScore: 88,
51+
resourceId: 'res-1',
52+
status: 'COMPLETED',
53+
},
54+
{
55+
finalScore: 84,
56+
resourceId: 'res-2',
57+
status: 'SUBMITTED',
58+
},
59+
], 86)
60+
61+
const result = resolveSubmissionReviewResult(submission, {
62+
defaultMinimumPassingScore: DEFAULT_PASSING_SCORE,
63+
})
64+
65+
expect(result)
66+
.toBe('PASS')
67+
})
68+
69+
it('considers reviews complete when final scores exist even without explicit statuses', () => {
70+
const submission = buildSubmission([
71+
{
72+
finalScore: 90,
73+
resourceId: 'res-1',
74+
},
75+
{
76+
finalScore: 82,
77+
resourceId: 'res-2',
78+
},
79+
], 86)
80+
81+
const result = resolveSubmissionReviewResult(submission, {
82+
defaultMinimumPassingScore: DEFAULT_PASSING_SCORE,
83+
})
84+
85+
expect(result)
86+
.toBe('PASS')
87+
})
88+
89+
it('does not defer outcomes for single-review assignments', () => {
90+
const submission = buildSubmission([
91+
{
92+
finalScore: 91,
93+
resourceId: 'res-1',
94+
status: 'PENDING',
95+
},
96+
], 91)
97+
98+
const result = resolveSubmissionReviewResult(submission, {
99+
defaultMinimumPassingScore: DEFAULT_PASSING_SCORE,
100+
})
101+
102+
expect(result)
103+
.toBe('PASS')
104+
})
105+
})

src/apps/review/src/lib/components/common/reviewResult.ts

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { SubmissionRow } from './types'
1+
import type { AggregatedReviewDetail, SubmissionRow } from './types'
22

33
export type ReviewOutcome = 'PASS' | 'FAIL'
44

@@ -38,6 +38,71 @@ const OUTCOME_KEYS = [
3838

3939
type MetadataObject = Record<string, unknown>
4040

41+
const COMPLETED_REVIEW_STATUSES = new Set(['COMPLETED', 'SUBMITTED'])
42+
43+
const normalizeStatusString = (value: unknown): string | undefined => {
44+
if (typeof value !== 'string') {
45+
return undefined
46+
}
47+
48+
const normalized = value.trim()
49+
.toUpperCase()
50+
return normalized.length ? normalized : undefined
51+
}
52+
53+
const isAggregatedReviewDetailCompleted = (detail: AggregatedReviewDetail): boolean => {
54+
const statusCandidates = [
55+
detail.status,
56+
detail.reviewInfo?.status,
57+
]
58+
59+
for (const candidate of statusCandidates) {
60+
const normalized = normalizeStatusString(candidate)
61+
if (normalized) {
62+
return COMPLETED_REVIEW_STATUSES.has(normalized)
63+
}
64+
}
65+
66+
const committedCandidates = [
67+
detail.reviewInfo?.committed,
68+
]
69+
70+
if (committedCandidates.some(value => value === false)) {
71+
return false
72+
}
73+
74+
if (committedCandidates.some(value => value === true)) {
75+
return true
76+
}
77+
78+
const finalScoreCandidates = [
79+
detail.finalScore,
80+
detail.reviewInfo?.finalScore,
81+
]
82+
83+
return finalScoreCandidates.some(value => typeof value === 'number' && Number.isFinite(value))
84+
}
85+
86+
const shouldDeferAggregatedOutcome = (submission: SubmissionRow): boolean => {
87+
const aggregatedReviews = submission.aggregated?.reviews
88+
if (!aggregatedReviews || aggregatedReviews.length === 0) {
89+
return false
90+
}
91+
92+
const assignedReviews = aggregatedReviews.filter(review => (
93+
Boolean(review.resourceId)
94+
|| Boolean(review.reviewInfo?.resourceId)
95+
|| Boolean(review.reviewId)
96+
|| Boolean(review.reviewerHandle ?? review.reviewInfo?.reviewerHandle)
97+
))
98+
99+
if (assignedReviews.length <= 1) {
100+
return false
101+
}
102+
103+
return !assignedReviews.every(isAggregatedReviewDetailCompleted)
104+
}
105+
41106
const isRecord = (value: unknown): value is MetadataObject => (
42107
typeof value === 'object'
43108
&& value !== null
@@ -317,6 +382,10 @@ export function resolveSubmissionReviewResult(
317382
defaultMinimumPassingScore,
318383
}: ResolveSubmissionReviewResultOptions = options
319384

385+
if (shouldDeferAggregatedOutcome(submission)) {
386+
return undefined
387+
}
388+
320389
const metadataCandidates = collectMetadataCandidates(submission)
321390

322391
const metadataOutcome = metadataCandidates

0 commit comments

Comments
 (0)