Skip to content

Commit c6b7f67

Browse files
rishabhtcrishabhtc
authored
Merge pull request #1682 from topcoder-platform/pm-1833-validation-error
Validate reviewer errors on saving challenge
2 parents 55723a4 + 2eccf36 commit c6b7f67

File tree

5 files changed

+154
-32
lines changed

5 files changed

+154
-32
lines changed

‎src/actions/challenges.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
updateChallengeSkillsApi,
2424
fetchDefaultReviewers,
2525
fetchScorecards,
26+
fetchScorecardById,
2627
fetchWorkflows
2728
} from '../services/challenges'
2829
import { searchProfilesByUserIds } from '../services/user'
@@ -794,6 +795,34 @@ export function loadScorecards (filters = {}) {
794795
}
795796
}
796797

798+
/**
799+
* Load a specific scorecard by ID
800+
* @param {string} scorecardId the scorecard ID
801+
*/
802+
export function loadScorecardById (scorecardId) {
803+
return async (dispatch) => {
804+
try {
805+
const scorecard = await fetchScorecardById(scorecardId)
806+
dispatch({
807+
type: LOAD_CHALLENGE_METADATA_SUCCESS,
808+
metadataKey: 'scorecardById',
809+
metadataValue: {
810+
...scorecard,
811+
id: scorecardId
812+
}
813+
})
814+
} catch (error) {
815+
console.error('Error loading scorecard by ID:', error)
816+
// Return null on error
817+
dispatch({
818+
type: LOAD_CHALLENGE_METADATA_SUCCESS,
819+
metadataKey: 'scorecardById',
820+
metadataValue: null
821+
})
822+
}
823+
}
824+
}
825+
797826
/**
798827
* Load default reviewers
799828
* @param {Object} filters filters for default reviewers

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,7 @@
186186

187187
.error {
188188
color: $tc-red;
189-
background-color: #ffebee;
190-
padding: 15px;
191-
border-radius: 4px;
192-
border: 1px solid #ffcdd2;
193-
margin-bottom: 20px;
189+
padding: 5px;
194190
}
195191

196192
.validationErrors {
@@ -211,13 +207,6 @@
211207
margin-bottom: 0;
212208
}
213209

214-
.fieldError {
215-
color: #DC3545;
216-
font-size: 12px;
217-
margin-top: 4px;
218-
display: block;
219-
}
220-
221210
// Responsive adjustments
222211
@media (max-width: 768px) {
223212
.formRow {

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

Lines changed: 71 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { connect } from 'react-redux'
44
import cn from 'classnames'
55
import { PrimaryButton, OutlineButton } from '../../Buttons'
66
import { REVIEW_OPPORTUNITY_TYPE_LABELS, REVIEW_OPPORTUNITY_TYPES, VALIDATION_VALUE_TYPE } from '../../../config/constants'
7-
import { loadScorecards, loadDefaultReviewers, loadWorkflows } from '../../../actions/challenges'
7+
import { loadScorecards, loadDefaultReviewers, loadWorkflows, loadScorecardById } from '../../../actions/challenges'
88
import styles from './ChallengeReviewer-Field.module.scss'
99
import { convertDollarToInteger, validateValue } from '../../../util/input-check'
1010

@@ -31,18 +31,28 @@ class ChallengeReviewerField extends Component {
3131
}
3232

3333
componentDidMount () {
34-
this.loadScorecards()
34+
if (this.props.readOnly) {
35+
// In read-only mode, only load specific scorecards for existing reviewers
36+
this.loadSpecificScorecards()
37+
} else {
38+
// In edit mode, load all scorecards for dropdown
39+
this.loadScorecards()
40+
}
3541
this.loadDefaultReviewers()
3642
this.loadWorkflows()
3743
}
3844

3945
componentDidUpdate (prevProps) {
40-
const { challenge } = this.props
46+
const { challenge, readOnly } = this.props
4147
const prevChallenge = prevProps.challenge
4248

4349
if (challenge && prevChallenge &&
4450
(challenge.type !== prevChallenge.type || challenge.track !== prevChallenge.track)) {
45-
this.loadScorecards()
51+
if (readOnly) {
52+
this.loadSpecificScorecards()
53+
} else {
54+
this.loadScorecards()
55+
}
4656
}
4757

4858
if (challenge && prevChallenge &&
@@ -69,6 +79,27 @@ class ChallengeReviewerField extends Component {
6979
loadScorecards(filters)
7080
}
7181

82+
loadSpecificScorecards () {
83+
const { challenge, loadScorecardById } = this.props
84+
const reviewers = challenge.reviewers || []
85+
86+
// Get unique scorecard IDs from reviewers
87+
const scorecardIds = [...new Set(
88+
reviewers
89+
.filter(reviewer => reviewer.scorecardId)
90+
.map(reviewer => reviewer.scorecardId)
91+
)]
92+
93+
if (scorecardIds.length === 0) {
94+
return
95+
}
96+
97+
// Load each scorecard individually
98+
scorecardIds.forEach(scorecardId => {
99+
loadScorecardById(scorecardId)
100+
})
101+
}
102+
72103
loadDefaultReviewers () {
73104
const { challenge, loadDefaultReviewers } = this.props
74105

@@ -240,7 +271,7 @@ class ChallengeReviewerField extends Component {
240271
renderReviewerForm (reviewer, index) {
241272
const { challenge, metadata = {}, readOnly = false } = this.props
242273
const { scorecards = [], workflows = [] } = metadata
243-
const validationErrors = this.validateReviewer(reviewer)
274+
const validationErrors = challenge.submitTriggered ? this.validateReviewer(reviewer) : {}
244275

245276
return (
246277
<div key={`reviewer-${index}`} className={styles.reviewerForm}>
@@ -337,8 +368,10 @@ class ChallengeReviewerField extends Component {
337368
))}
338369
</select>
339370
)}
340-
{validationErrors.aiWorkflowId && (
341-
<div className={styles.fieldError}>{validationErrors.aiWorkflowId}</div>
371+
{!readOnly && challenge.submitTriggered && validationErrors.aiWorkflowId && (
372+
<div className={styles.error}>
373+
{validationErrors.aiWorkflowId}
374+
</div>
342375
)}
343376
</div>
344377
) : (
@@ -347,8 +380,15 @@ class ChallengeReviewerField extends Component {
347380
{readOnly ? (
348381
<span>
349382
{(() => {
383+
const { metadata = {} } = this.props
384+
const specificScorecard = metadata.scorecardById
385+
if (specificScorecard && specificScorecard.id === reviewer.scorecardId) {
386+
return `${specificScorecard.name || 'Unknown'} - ${specificScorecard.type || 'Unknown'} (${specificScorecard.challengeTrack || 'Unknown'}) v${specificScorecard.version || 'Unknown'}`
387+
}
388+
389+
// Fallback to searching in the general scorecards array
350390
const scorecard = scorecards.find(s => s.id === reviewer.scorecardId)
351-
return scorecard ? `${scorecard.name} - ${scorecard.type} (${scorecard.challengeTrack}) v${scorecard.version}` : 'Not selected'
391+
return scorecard ? `${scorecard.name || 'Unknown'} - ${scorecard.type || 'Unknown'} (${scorecard.challengeTrack || 'Unknown'}) v${scorecard.version || 'Unknown'}` : 'Not selected'
352392
})()}
353393
</span>
354394
) : (
@@ -359,13 +399,15 @@ class ChallengeReviewerField extends Component {
359399
<option value=''>Select Scorecard</option>
360400
{scorecards.map(scorecard => (
361401
<option key={scorecard.id} value={scorecard.id}>
362-
{scorecard.name} - {scorecard.type} ({scorecard.challengeTrack}) v{scorecard.version}
402+
{scorecard.name || 'Unknown'} - {scorecard.type || 'Unknown'} ({scorecard.challengeTrack || 'Unknown'}) v{scorecard.version || 'Unknown'}
363403
</option>
364404
))}
365405
</select>
366406
)}
367-
{validationErrors.scorecardId && (
368-
<div className={styles.fieldError}>{validationErrors.scorecardId}</div>
407+
{!readOnly && challenge.submitTriggered && validationErrors.scorecardId && (
408+
<div className={styles.error}>
409+
{validationErrors.scorecardId}
410+
</div>
369411
)}
370412
</div>
371413
)}
@@ -409,8 +451,10 @@ class ChallengeReviewerField extends Component {
409451
))}
410452
</select>
411453
)}
412-
{validationErrors.phaseId && (
413-
<div className={styles.fieldError}>{validationErrors.phaseId}</div>
454+
{!readOnly && challenge.submitTriggered && validationErrors.phaseId && (
455+
<div className={styles.error}>
456+
{validationErrors.phaseId}
457+
</div>
414458
)}
415459
</div>
416460
</div>
@@ -433,8 +477,10 @@ class ChallengeReviewerField extends Component {
433477
}}
434478
/>
435479
)}
436-
{validationErrors.memberReviewerCount && (
437-
<div className={styles.fieldError}>{validationErrors.memberReviewerCount}</div>
480+
{!readOnly && challenge.submitTriggered && validationErrors.memberReviewerCount && (
481+
<div className={styles.error}>
482+
{validationErrors.memberReviewerCount}
483+
</div>
438484
)}
439485
</div>
440486

@@ -453,8 +499,10 @@ class ChallengeReviewerField extends Component {
453499
}}
454500
/>
455501
)}
456-
{validationErrors.basePayment && (
457-
<div className={styles.fieldError}>{validationErrors.basePayment}</div>
502+
{!readOnly && challenge.submitTriggered && validationErrors.basePayment && (
503+
<div className={styles.error}>
504+
{validationErrors.basePayment}
505+
</div>
458506
)}
459507
</div>
460508

@@ -617,13 +665,15 @@ ChallengeReviewerField.propTypes = {
617665
metadata: PropTypes.shape({
618666
scorecards: PropTypes.array,
619667
defaultReviewers: PropTypes.array,
620-
workflows: PropTypes.array
668+
workflows: PropTypes.array,
669+
scorecardById: PropTypes.object
621670
}),
622671
isLoading: PropTypes.bool,
623672
readOnly: PropTypes.bool,
624673
loadScorecards: PropTypes.func.isRequired,
625674
loadDefaultReviewers: PropTypes.func.isRequired,
626-
loadWorkflows: PropTypes.func.isRequired
675+
loadWorkflows: PropTypes.func.isRequired,
676+
loadScorecardById: PropTypes.func.isRequired
627677
}
628678

629679
const mapStateToProps = (state) => ({
@@ -634,7 +684,8 @@ const mapStateToProps = (state) => ({
634684
const mapDispatchToProps = {
635685
loadScorecards,
636686
loadDefaultReviewers,
637-
loadWorkflows
687+
loadWorkflows,
688+
loadScorecardById
638689
}
639690

640691
export default connect(mapStateToProps, mapDispatchToProps)(ChallengeReviewerField)

‎src/components/ChallengeEditor/index.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,46 @@ class ChallengeEditor extends Component {
769769
return true
770770
}
771771

772+
isValidReviewers () {
773+
const { challenge } = this.state
774+
const reviewers = challenge.reviewers || []
775+
776+
if (reviewers.length === 0) {
777+
return true
778+
}
779+
780+
// Validate each reviewer
781+
for (const reviewer of reviewers) {
782+
const isAI = (reviewer.aiWorkflowId && reviewer.aiWorkflowId.trim() !== '') || !reviewer.isMemberReview
783+
784+
if (isAI) {
785+
if (!reviewer.aiWorkflowId || reviewer.aiWorkflowId.trim() === '') {
786+
return false
787+
}
788+
} else {
789+
if (!reviewer.scorecardId) {
790+
return false
791+
}
792+
793+
const memberCount = parseInt(reviewer.memberReviewerCount) || 1
794+
if (memberCount < 1 || !Number.isInteger(memberCount)) {
795+
return false
796+
}
797+
798+
const basePayment = convertDollarToInteger(reviewer.basePayment || '0', '')
799+
if (basePayment < 0) {
800+
return false
801+
}
802+
}
803+
804+
if (!reviewer.phaseId) {
805+
return false
806+
}
807+
}
808+
809+
return true
810+
}
811+
772812
isValidChallenge () {
773813
const { challenge } = this.state
774814
if (this.props.isNew) {
@@ -790,6 +830,10 @@ class ChallengeEditor extends Component {
790830
return false
791831
}
792832

833+
if (!this.isValidReviewers()) {
834+
return false
835+
}
836+
793837
const requiredFields = [
794838
'trackId',
795839
'typeId',

‎src/services/challenges.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,15 @@ export async function fetchScorecards (filters = {}) {
335335
const response = await axiosInstance.get(`${SCORECARDS_API_URL}?${qs.stringify(query, { encode: false })}`)
336336
return _.get(response, 'data', {})
337337
}
338+
/**
339+
* Api request for fetching a specific scorecard by ID
340+
* @param {string} scorecardId the scorecard ID
341+
* @returns {Promise<*>}
342+
*/
343+
export async function fetchScorecardById (scorecardId) {
344+
const response = await axiosInstance.get(`${SCORECARDS_API_URL}/${scorecardId}`)
345+
return _.get(response, 'data', {})
346+
}
338347
/**
339348
* Api request for fetching default reviewers
340349
* @param {Object} filters filters for default reviewers

0 commit comments

Comments
 (0)