Skip to content

Commit 640f2da

Browse files
authored
Merge pull request #1711 from topcoder-platform/pm-2372_2
fix(PM-2372): show the duplicate reviewer config error on saving
2 parents f4975c3 + a72fcab commit 640f2da

File tree

2 files changed

+36
-35
lines changed

2 files changed

+36
-35
lines changed

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

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -444,41 +444,6 @@ 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-
482447
// For AI reviewers, get scorecardId from the workflow if available
483448
let scorecardId = ''
484449
if (isAIReviewer) {

src/components/ChallengeEditor/index.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,11 +897,47 @@ class ChallengeEditor extends Component {
897897
return !(isRequiredMissing || _.isEmpty(this.state.currentTemplate))
898898
}
899899

900+
// Return array of phase names that have more than one manual (member) reviewer configured.
901+
// If none, returns empty array.
902+
getDuplicateManualReviewerPhases () {
903+
const { challenge } = this.state
904+
const reviewers = (challenge && challenge.reviewers) || []
905+
const phases = (challenge && challenge.phases) || []
906+
907+
const counts = {}
908+
reviewers.forEach(r => {
909+
if (r && (r.isMemberReview !== false) && r.phaseId) {
910+
const pid = String(r.phaseId)
911+
counts[pid] = (counts[pid] || 0) + 1
912+
}
913+
})
914+
915+
const duplicatedPhaseIds = Object.keys(counts).filter(pid => counts[pid] > 1)
916+
if (duplicatedPhaseIds.length === 0) return []
917+
918+
return duplicatedPhaseIds.map(pid => {
919+
const p = phases.find(ph => String(ph.phaseId || ph.id) === pid)
920+
return p ? (p.name || pid) : pid
921+
})
922+
}
923+
900924
validateChallenge () {
901925
if (this.isValidChallenge()) {
926+
// Additional validation: block saving draft if there are duplicate manual reviewer configs per phase
927+
const duplicates = this.getDuplicateManualReviewerPhases()
928+
if (duplicates && duplicates.length > 0) {
929+
const message = `Duplicate manual reviewer configuration found for phase(s): ${duplicates.join(', ')}. Only one manual reviewer configuration is allowed per phase.`
930+
this.setState({ hasValidationErrors: true, error: message })
931+
return false
932+
}
933+
934+
if (this.state.error) {
935+
this.setState({ error: null })
936+
}
902937
this.setState({ hasValidationErrors: false })
903938
return true
904939
}
940+
905941
this.setState(prevState => ({
906942
...prevState,
907943
challenge: {

0 commit comments

Comments
 (0)