Skip to content

Commit f4975c3

Browse files
authored
Merge pull request #1710 from topcoder-platform/pm-2372_1
fix: user should be able to add only one manual reviewer config per phase
2 parents 846591f + 2218242 commit f4975c3

File tree

2 files changed

+86
-2
lines changed

2 files changed

+86
-2
lines changed

src/components/ChallengeEditor/ChallengeReviewer-Field/ChallengeReviewer-Field.module.scss

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@
3939
flex-direction: column;
4040
width: 600px;
4141
}
42+
43+
.fieldError {
44+
margin-top: 12px;
45+
}
4246
}
4347
}
4448

src/components/ChallengeEditor/ChallengeReviewer-Field/index.js

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,41 @@ class ChallengeReviewerField extends Component {
444444

445445
const isAIReviewer = this.isAIReviewer(defaultTrackReviewer)
446446

447+
// Prevent adding a second manual (member) reviewer for the same phase.
448+
// If the default phase already has a manual reviewer, attempt to find another
449+
// suitable review phase that does not yet have a manual reviewer and use it.
450+
if (!isAIReviewer && defaultPhaseId) {
451+
const existsManualForPhase = currentReviewers.some(r => (r.isMemberReview !== false) && (r.phaseId === defaultPhaseId))
452+
if (existsManualForPhase) {
453+
const possibleAlternatePhase = (challenge.phases || []).find(p => {
454+
const rawName = p.name ? p.name : ''
455+
const phaseName = rawName.toLowerCase()
456+
const phaseWithoutHyphens = phaseName.replace(/[-\s]/g, '')
457+
const acceptedPhases = ['review', 'screening', 'checkpointscreening', 'approval', 'postmortem']
458+
const isSubmissionPhase = phaseName.includes('submission')
459+
const acceptable = acceptedPhases.includes(phaseWithoutHyphens) && !isSubmissionPhase
460+
461+
if (!acceptable) return false
462+
463+
const phaseId = p.phaseId || p.id
464+
const used = currentReviewers.some(r => (r.isMemberReview !== false) && (r.phaseId === phaseId))
465+
return !used
466+
})
467+
468+
if (possibleAlternatePhase) {
469+
defaultPhaseId = possibleAlternatePhase.phaseId || possibleAlternatePhase.id
470+
if (this.state.error) this.setState({ error: null })
471+
} else {
472+
const phase = (challenge.phases || []).find(p => (p.id === defaultPhaseId) || (p.phaseId === defaultPhaseId))
473+
const phaseName = phase ? (phase.name || defaultPhaseId) : defaultPhaseId
474+
this.setState({
475+
error: `A manual reviewer configuration already exists for phase '${phaseName}'`
476+
})
477+
return
478+
}
479+
}
480+
}
481+
447482
// For AI reviewers, get scorecardId from the workflow if available
448483
let scorecardId = ''
449484
if (isAIReviewer) {
@@ -482,6 +517,11 @@ class ChallengeReviewerField extends Component {
482517
newReviewer.memberReviewerCount = (defaultReviewer && defaultReviewer.memberReviewerCount) || 1
483518
}
484519

520+
// Clear any prior transient error when add succeeds
521+
if (this.state.error) {
522+
this.setState({ error: null })
523+
}
524+
485525
const updatedReviewers = currentReviewers.concat([newReviewer])
486526
onUpdateReviewers({ field: 'reviewers', value: updatedReviewers })
487527
}
@@ -513,6 +553,21 @@ class ChallengeReviewerField extends Component {
513553

514554
// Special handling for phase and count changes
515555
if (field === 'phaseId') {
556+
// Before changing phase, ensure we're not creating a duplicate manual reviewer for the target phase
557+
const targetPhaseId = value
558+
const isCurrentMember = (updatedReviewers[index] && (updatedReviewers[index].isMemberReview !== false))
559+
if (isCurrentMember) {
560+
const conflict = (currentReviewers || []).some((r, i) => i !== index && (r.isMemberReview !== false) && (r.phaseId === targetPhaseId))
561+
if (conflict) {
562+
const phase = (challenge.phases || []).find(p => (p.id === targetPhaseId) || (p.phaseId === targetPhaseId))
563+
const phaseName = phase ? (phase.name || targetPhaseId) : targetPhaseId
564+
this.setState({
565+
error: `Cannot move manual reviewer to phase '${phaseName}' because a manual reviewer configuration already exists for that phase.`
566+
})
567+
return
568+
}
569+
}
570+
516571
this.handlePhaseChangeWithReassign(index, value)
517572

518573
// update payment based on default reviewer
@@ -632,6 +687,21 @@ class ChallengeReviewerField extends Component {
632687
const currentReviewers = challenge.reviewers || []
633688
const updatedReviewers = currentReviewers.slice()
634689

690+
// Block switching an AI reviewer to a member reviewer if another manual reviewer exists for same phase
691+
if (!isAI) {
692+
const existingReviewer = currentReviewers[index] || {}
693+
const phaseId = existingReviewer.phaseId
694+
const conflict = currentReviewers.some((r, i) => i !== index && (r.isMemberReview !== false) && (r.phaseId === phaseId))
695+
if (conflict) {
696+
const phase = (challenge.phases || []).find(p => (p.id === phaseId) || (p.phaseId === phaseId))
697+
const phaseName = phase ? (phase.name || phaseId) : phaseId
698+
this.setState({
699+
error: `Cannot switch to Member Reviewer: a manual reviewer configuration already exists for phase '${phaseName}'. Increase "Number of Reviewers" on the existing configuration instead.`
700+
})
701+
return
702+
}
703+
}
704+
635705
// Update reviewer type by setting/clearing aiWorkflowId
636706
const currentReviewer = updatedReviewers[index]
637707

@@ -674,6 +744,11 @@ class ChallengeReviewerField extends Component {
674744
this.handleToggleShouldOpen(index, true)
675745
}
676746

747+
// Clear any transient error when successful change is applied
748+
if (this.state.error) {
749+
this.setState({ error: null })
750+
}
751+
677752
onUpdateReviewers({ field: 'reviewers', value: updatedReviewers })
678753
}}
679754
>
@@ -772,10 +847,10 @@ class ChallengeReviewerField extends Component {
772847
const isPostMortemPhase = norm === 'postmortem'
773848
const isCurrentlySelected = reviewer.phaseId && ((phase.id === reviewer.phaseId) || (phase.phaseId === reviewer.phaseId)) && !isSubmissionPhase
774849

775-
// Collect phases already assigned to other reviewers (excluding current reviewer)
850+
// Collect phases already assigned to other manual (member) reviewers (excluding current reviewer)
776851
const assignedPhaseIds = new Set(
777852
(challenge.reviewers || [])
778-
.filter((r, i) => i !== index)
853+
.filter((r, i) => i !== index && (r.isMemberReview !== false))
779854
.map(r => r.phaseId)
780855
.filter(id => id !== undefined && id !== null)
781856
)
@@ -1051,6 +1126,11 @@ class ChallengeReviewerField extends Component {
10511126
/>
10521127
</div>
10531128
)}
1129+
{error && !isLoading && (
1130+
<div className={cn(styles.fieldError, styles.error)}>
1131+
{error}
1132+
</div>
1133+
)}
10541134
</div>
10551135
</div>
10561136
</>

0 commit comments

Comments
 (0)