Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/apps/work/src/lib/schemas/challenge-editor.schema.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,29 +332,28 @@ describe('challenge-editor schema reviewer slot assignment validation', () => {
.toBeTruthy()
})

it('still requires an assigned checkpoint screener', async () => {
it('accepts an unassigned checkpoint screener for the Checkpoint Screening phase', async () => {
await expect(
challengeAdvancedOptionsSchema.validate({
...baseFormData,
phases: [{
id: 'checkpoint-screening-phase-instance-id',
name: 'Checkpoint Screening',
phaseId: 'checkpoint-screening-phase-id',
}],
reviewers: [
{
isMemberReview: true,
memberReviewerCount: 1,
phaseId: 'checkpoint-screening-phase-id',
phaseId: 'checkpoint-screening-phase-instance-id',
scorecardId: 'checkpoint-screening-scorecard-id',
shouldOpenOpportunity: false,
},
],
}),
)
.rejects
.toMatchObject({
path: 'reviewers[0].memberId',
})
.resolves
.toBeTruthy()
})

it('accepts required reviewer slot assignments when opportunity is closed', async () => {
Expand Down
17 changes: 11 additions & 6 deletions src/apps/work/src/lib/utils/reviewer.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type {
*
* @param value reviewer or phase value to normalize.
* @returns trimmed text for string values, otherwise an empty string.
* @remarks Used internally while resolving whether a reviewer belongs to standard Screening.
* @remarks Used internally while resolving whether a reviewer belongs to a screening phase.
* @throws Does not throw.
*/
function normalizeReviewerValue(value: unknown): string {
Expand All @@ -22,9 +22,9 @@ function normalizeReviewerValue(value: unknown): string {
*
* @param reviewer reviewer configuration whose phase should be inspected.
* @param phases challenge phases used to resolve the reviewer's phase name.
* @returns `true` only for a human reviewer configured on the standard Screening phase.
* @remarks Form validation and reviewer fields use this exception; Checkpoint Screening and every
* other reviewer phase still require assignments up front.
* @returns `true` for a human reviewer configured on Screening or Checkpoint Screening.
* @remarks Form validation and reviewer fields use this exception; every other reviewer phase
* still requires assignments up front.
* @throws Does not throw.
*/
export function isScreenerAssignmentOptional(
Expand All @@ -46,8 +46,13 @@ export function isScreenerAssignmentOptional(
const matchesPhase = reviewerPhaseId === phaseTemplateId
|| reviewerPhaseId === phaseInstanceId

const normalizedPhaseName = normalizeReviewerValue(phase.name)
.toLowerCase()

return matchesPhase
&& normalizeReviewerValue(phase.name)
.toLowerCase() === 'screening'
&& (
normalizedPhaseName === 'screening'
|| normalizedPhaseName === 'checkpoint screening'
)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ The form uses `challengeBasicInfoSchema` from `src/apps/work/src/lib/schemas/cha
- `tags`: optional string array.
- `skills`: required unless billing account is listed in `SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS`.
- `reviewer`: optional for task challenges.
- `reviewers`: when using `Save as Draft` from `NEW` status, non-task/non-marathon challenges must include reviewer coverage for configured review phases. If required phases are configured, each phase must have at least one member reviewer with a scorecard. The standard Screening configuration and scorecard remain required, but its Screener member assignment may be left empty until after launch; other closed manual reviewer assignments remain required.
- `reviewers`: when using `Save as Draft` from `NEW` status, non-task/non-marathon challenges must include reviewer coverage for configured review phases. If required phases are configured, each phase must have at least one member reviewer with a scorecard. The Screening and Checkpoint Screening configurations and scorecards remain required, but their Screener member assignments may be left empty until after launch; other closed manual reviewer assignments remain required.
- `AI review configuration`: templates and manual configs autosave separately once valid, switching a template-backed config to manual mode keeps its copied settings but clears the template link on save, and the AI tab becomes read-only after the challenge has submissions.

## Autosave Behavior
Expand Down Expand Up @@ -80,7 +80,7 @@ The form uses `challengeBasicInfoSchema` from `src/apps/work/src/lib/schemas/cha
from `is_test_challenge`, and explicitly persists metadata value `true` or `false`. Test
challenges do not generate payments, and authorized modifiers can delete them after they reach
a completed or cancelled status.
- `ReviewersField`: hidden for `Task` and `Marathon Match` challenges because manual reviewer assignment is handled elsewhere. Copilot-only Design Challenge editing exposes only the shared Screener selector; selecting, replacing, or clearing it updates both the `Screener` and any `Checkpoint Screener` challenge resources. On the full human-review tab, each manual reviewer card keeps the legacy review-type dropdown, backfills missing legacy review-type values from the matching default reviewer or iterative-review phase fallback, and each manual reviewer phase selector hides registration/submission phases and any phase already assigned on another manual reviewer card while preserving the card's current selection. When default reviewer metadata is missing, stale, or already covered by existing rows, `Add reviewer` starts from the next unassigned selectable reviewer phase, preferring review phases before approval or screening phases, so single-round Design schedules add the Approver row instead of a registration/submission or duplicate reviewer row. Manual reviewer counts are capped before rendering member assignment controls so closed public opportunities cannot create an unbounded number of member selectors. Design challenge manual reviewers always keep the public review opportunity checkbox disabled and unchecked. Standard Screening member selectors remain available but are optional so a copilot can assign the Screener after launch.
- `ReviewersField`: hidden for `Task` and `Marathon Match` challenges because manual reviewer assignment is handled elsewhere. On the human-review tab, each manual reviewer card keeps the legacy review-type dropdown, backfills missing legacy review-type values from the matching default reviewer or iterative-review phase fallback, and each manual reviewer phase selector hides registration/submission phases and any phase already assigned on another manual reviewer card while preserving the card's current selection. When default reviewer metadata is missing, stale, or already covered by existing rows, `Add reviewer` starts from the next unassigned selectable reviewer phase, preferring review phases before approval or screening phases, so single-round Design schedules add the Approver row instead of a registration/submission or duplicate reviewer row. Manual reviewer counts are capped before rendering member assignment controls so closed public opportunities cannot create an unbounded number of member selectors. Design challenge manual reviewers always keep the public review opportunity checkbox disabled and unchecked. Screening and Checkpoint Screening member selectors remain available but are optional so a copilot can assign the Screener or Checkpoint Screener after launch.
- `Submission Settings`: shown for Design `Challenge` and Design `First2Finish` types, and contains the final-deliverables, stock-art, and submission-limit compatibility fields.
- `RegisteredMemberDownloadField`: shown in Advanced Options for every created challenge type. The radio group persists `allowAllRegistrantsToDownloadWinningSubmissions` as the exact string `true` for all challenge registrants or `false` for passing submitters only. New Development challenges default to passing submitters; other new challenges, including Design, default to all registrants. Existing challenges without the metadata retain passing-submitter-only access.
- `FinalDeliverablesField`: design-challenge file-type editor that persists the legacy `fileTypes` metadata payload used on challenge draft pages.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ describe('ChallengeEditorForm', () => {
...validDraftChallenge,
status: 'NEW',
} as Challenge
const designChallengeWithDeferredScreener = {
const designChallengeWithDeferredScreeners = {
...validDraftChallenge,
approvalStatus: 'APPROVED',
phases: [
Expand All @@ -871,6 +871,11 @@ describe('ChallengeEditorForm', () => {
name: 'Screening',
phaseId: 'screening-phase-id',
},
{
duration: 60,
name: 'Checkpoint Screening',
phaseId: 'checkpoint-screening-phase-id',
},
{
duration: 60,
name: 'Review',
Expand All @@ -886,6 +891,14 @@ describe('ChallengeEditorForm', () => {
scorecardId: 'screening-scorecard-id',
shouldOpenOpportunity: false,
},
{
additionalMemberIds: [],
isMemberReview: true,
memberReviewerCount: 1,
phaseId: 'checkpoint-screening-phase-id',
scorecardId: 'checkpoint-screening-scorecard-id',
shouldOpenOpportunity: false,
},
{
additionalMemberIds: [],
isMemberReview: true,
Expand Down Expand Up @@ -1916,7 +1929,7 @@ describe('ChallengeEditorForm', () => {
.not.toHaveBeenCalledWith('Challenge launch is blocked until budget approval is Approved.')
})

it('launches a design draft before a screener member is assigned', async () => {
it('launches a design draft before screening members are assigned', async () => {
let launchAction: (() => Promise<void>) | undefined

mockedUseFetchChallengeTracks.mockReturnValue({
Expand Down Expand Up @@ -1944,14 +1957,14 @@ describe('ChallengeEditorForm', () => {
isLoading: false,
})
mockedPatchChallenge.mockResolvedValue({
...designChallengeWithDeferredScreener,
...designChallengeWithDeferredScreeners,
status: 'ACTIVE',
})

render(
<MemoryRouter>
<ChallengeEditorForm
challenge={designChallengeWithDeferredScreener}
challenge={designChallengeWithDeferredScreeners}
isReadOnly
onRegisterLaunchAction={action => {
launchAction = action
Expand All @@ -1977,6 +1990,10 @@ describe('ChallengeEditorForm', () => {
phaseId: 'screening-phase-id',
scorecardId: 'screening-scorecard-id',
}),
expect.objectContaining({
phaseId: 'checkpoint-screening-phase-id',
scorecardId: 'checkpoint-screening-scorecard-id',
}),
]),
status: 'ACTIVE',
}))
Expand Down Expand Up @@ -4332,7 +4349,7 @@ describe('ChallengeEditorForm', () => {
})
})

it('saves a new design draft before a screener member is assigned', async () => {
it('saves a new design draft before screening members are assigned', async () => {
const user = userEvent.setup()

mockedUseFetchChallengeTracks.mockReturnValue({
Expand All @@ -4352,15 +4369,15 @@ describe('ChallengeEditorForm', () => {
isLoading: false,
})
mockedPatchChallenge.mockResolvedValue({
...designChallengeWithDeferredScreener,
...designChallengeWithDeferredScreeners,
status: 'DRAFT',
})

render(
<MemoryRouter initialEntries={['/projects/100578/challenges/new']}>
<ChallengeEditorForm
challenge={{
...designChallengeWithDeferredScreener,
...designChallengeWithDeferredScreeners,
status: 'NEW',
}}
projectId='100578'
Expand All @@ -4379,6 +4396,10 @@ describe('ChallengeEditorForm', () => {
phaseId: 'screening-phase-id',
scorecardId: 'screening-scorecard-id',
}),
expect.objectContaining({
phaseId: 'checkpoint-screening-phase-id',
scorecardId: 'checkpoint-screening-scorecard-id',
}),
]),
status: 'DRAFT',
}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ describe('HumanReviewTab', () => {
})
})

it('keeps a Reviewer resource on Review when Screening has no assigned Screener', async () => {
it('keeps a Reviewer resource on Review when screening phases have no assigned screeners', async () => {
mockedUseFetchResourceRoles.mockReturnValue({
resourceRoles: [
{
Expand All @@ -673,6 +673,10 @@ describe('HumanReviewTab', () => {
id: 'role-reviewer',
name: 'Reviewer',
},
{
id: 'role-checkpoint-screener',
name: 'Checkpoint Screener',
},
],
})
mockedUseFetchResources.mockReturnValue({
Expand All @@ -696,6 +700,11 @@ describe('HumanReviewTab', () => {
name: 'Screening',
phaseId: 'phase-screening',
},
{
id: 'checkpoint-screening-instance',
name: 'Checkpoint Screening',
phaseId: 'phase-checkpoint-screening',
},
{
id: 'review-instance',
name: 'Review',
Expand All @@ -710,6 +719,13 @@ describe('HumanReviewTab', () => {
phaseId: 'phase-screening',
shouldOpenOpportunity: false,
},
{
additionalMemberIds: [],
isMemberReview: true,
memberReviewerCount: 1,
phaseId: 'phase-checkpoint-screening',
shouldOpenOpportunity: false,
},
{
additionalMemberIds: [],
isMemberReview: true,
Expand All @@ -727,6 +743,9 @@ describe('HumanReviewTab', () => {
.getAttribute('data-value'))
.toBe('')
expect(screen.getByTestId('reviewers.1.memberId')
.getAttribute('data-value'))
.toBe('')
expect(screen.getByTestId('reviewers.2.memberId')
.getAttribute('data-value'))
.toBe('member-reviewer')
})
Expand Down Expand Up @@ -973,7 +992,7 @@ describe('HumanReviewTab', () => {
.not.toBeNull()
})

it('marks only the standard Screening member assignment optional', () => {
it('marks Screening and Checkpoint Screening member assignments optional', () => {
mockedUseFetchChallengeTracks.mockReturnValue({
tracks: [
{
Expand All @@ -992,6 +1011,10 @@ describe('HumanReviewTab', () => {
name: 'Screening',
phaseId: 'screening-phase-id',
},
{
name: 'Checkpoint Screening',
phaseId: 'checkpoint-screening-phase-id',
},
{
name: 'Review',
phaseId: 'review-phase-id',
Expand All @@ -1005,6 +1028,13 @@ describe('HumanReviewTab', () => {
scorecardId: 'screening-scorecard-id',
shouldOpenOpportunity: false,
},
{
isMemberReview: true,
memberReviewerCount: 1,
phaseId: 'checkpoint-screening-phase-id',
scorecardId: 'checkpoint-screening-scorecard-id',
shouldOpenOpportunity: false,
},
{
isMemberReview: true,
memberReviewerCount: 1,
Expand All @@ -1020,6 +1050,8 @@ describe('HumanReviewTab', () => {
expect(screen.getByTestId('reviewers.0.memberId'))
.toHaveProperty('dataset.required', 'false')
expect(screen.getByTestId('reviewers.1.memberId'))
.toHaveProperty('dataset.required', 'false')
expect(screen.getByTestId('reviewers.2.memberId'))
.toHaveProperty('dataset.required', 'true')
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from './reviewerAssignments.utils'

describe('buildAssignedResourcesByReviewer', () => {
it('reserves generic reviewer resources for Review rows before Screening fallbacks', () => {
it('reserves generic reviewer resources for Review rows before screening phase fallbacks', () => {
const resourceRoles: ResourceRole[] = [
{
id: 'role-screener',
Expand All @@ -19,6 +19,10 @@ describe('buildAssignedResourcesByReviewer', () => {
id: 'role-reviewer',
name: 'Reviewer',
},
{
id: 'role-checkpoint-screener',
name: 'Checkpoint Screener',
},
]
const resources: Resource[] = [
{
Expand All @@ -33,6 +37,11 @@ describe('buildAssignedResourcesByReviewer', () => {
phaseId: 'phase-screening',
roleId: 'role-reviewer',
},
{
memberReviewerCount: 1,
phaseId: 'phase-checkpoint-screening',
roleId: 'role-reviewer',
},
{
memberReviewerCount: 1,
phaseId: 'phase-review',
Expand All @@ -45,6 +54,10 @@ describe('buildAssignedResourcesByReviewer', () => {
'phase-screening',
'Screening',
],
[
'phase-checkpoint-screening',
'Checkpoint Screening',
],
[
'phase-review',
'Review',
Expand All @@ -58,6 +71,7 @@ describe('buildAssignedResourcesByReviewer', () => {
expect(assignedResourcesByReviewer.map(assignedResources => assignedResources
.map(resource => resource.memberHandle)))
.toEqual([
[],
[],
['reviewer-one'],
])
Expand Down
Loading