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
29 changes: 29 additions & 0 deletions src/actions/challenges.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
updateChallengeSkillsApi,
fetchDefaultReviewers,
fetchScorecards,
fetchScorecardById,
fetchWorkflows
} from '../services/challenges'
import { searchProfilesByUserIds } from '../services/user'
Expand Down Expand Up @@ -794,6 +795,34 @@ export function loadScorecards (filters = {}) {
}
}

/**
* Load a specific scorecard by ID
* @param {string} scorecardId the scorecard ID
*/
export function loadScorecardById (scorecardId) {
return async (dispatch) => {
try {
const scorecard = await fetchScorecardById(scorecardId)
dispatch({
type: LOAD_CHALLENGE_METADATA_SUCCESS,
metadataKey: 'scorecardById',
metadataValue: {
...scorecard,
id: scorecardId
}
})
} catch (error) {
console.error('Error loading scorecard by ID:', error)
// Return null on error
dispatch({
type: LOAD_CHALLENGE_METADATA_SUCCESS,
metadataKey: 'scorecardById',
metadataValue: null
})
}
}
}

/**
* Load default reviewers
* @param {Object} filters filters for default reviewers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,7 @@

.error {
color: $tc-red;
background-color: #ffebee;
padding: 15px;
border-radius: 4px;
border: 1px solid #ffcdd2;
margin-bottom: 20px;
padding: 5px;
}

.validationErrors {
Expand All @@ -211,13 +207,6 @@
margin-bottom: 0;
}

.fieldError {
color: #DC3545;
font-size: 12px;
margin-top: 4px;
display: block;
}

// Responsive adjustments
@media (max-width: 768px) {
.formRow {
Expand Down
91 changes: 71 additions & 20 deletions src/components/ChallengeEditor/ChallengeReviewer-Field/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { connect } from 'react-redux'
import cn from 'classnames'
import { PrimaryButton, OutlineButton } from '../../Buttons'
import { REVIEW_OPPORTUNITY_TYPE_LABELS, REVIEW_OPPORTUNITY_TYPES, VALIDATION_VALUE_TYPE } from '../../../config/constants'
import { loadScorecards, loadDefaultReviewers, loadWorkflows } from '../../../actions/challenges'
import { loadScorecards, loadDefaultReviewers, loadWorkflows, loadScorecardById } from '../../../actions/challenges'
import styles from './ChallengeReviewer-Field.module.scss'
import { convertDollarToInteger, validateValue } from '../../../util/input-check'

Expand All @@ -31,18 +31,28 @@ class ChallengeReviewerField extends Component {
}

componentDidMount () {
this.loadScorecards()
if (this.props.readOnly) {
// In read-only mode, only load specific scorecards for existing reviewers
this.loadSpecificScorecards()
} else {
// In edit mode, load all scorecards for dropdown
this.loadScorecards()
}
this.loadDefaultReviewers()
this.loadWorkflows()
}

componentDidUpdate (prevProps) {
const { challenge } = this.props
const { challenge, readOnly } = this.props
const prevChallenge = prevProps.challenge

if (challenge && prevChallenge &&
(challenge.type !== prevChallenge.type || challenge.track !== prevChallenge.track)) {
this.loadScorecards()
if (readOnly) {
this.loadSpecificScorecards()
} else {
this.loadScorecards()
}
}

if (challenge && prevChallenge &&
Expand All @@ -69,6 +79,27 @@ class ChallengeReviewerField extends Component {
loadScorecards(filters)
}

loadSpecificScorecards () {
const { challenge, loadScorecardById } = this.props
const reviewers = challenge.reviewers || []

// Get unique scorecard IDs from reviewers
const scorecardIds = [...new Set(
reviewers
.filter(reviewer => reviewer.scorecardId)
.map(reviewer => reviewer.scorecardId)
)]

if (scorecardIds.length === 0) {
return
}

// Load each scorecard individually
scorecardIds.forEach(scorecardId => {
loadScorecardById(scorecardId)
})
}

loadDefaultReviewers () {
const { challenge, loadDefaultReviewers } = this.props

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

return (
<div key={`reviewer-${index}`} className={styles.reviewerForm}>
Expand Down Expand Up @@ -337,8 +368,10 @@ class ChallengeReviewerField extends Component {
))}
</select>
)}
{validationErrors.aiWorkflowId && (
<div className={styles.fieldError}>{validationErrors.aiWorkflowId}</div>
{!readOnly && challenge.submitTriggered && validationErrors.aiWorkflowId && (
<div className={styles.error}>
{validationErrors.aiWorkflowId}
</div>
)}
</div>
) : (
Expand All @@ -347,8 +380,15 @@ class ChallengeReviewerField extends Component {
{readOnly ? (
<span>
{(() => {
const { metadata = {} } = this.props
const specificScorecard = metadata.scorecardById
if (specificScorecard && specificScorecard.id === reviewer.scorecardId) {
return `${specificScorecard.name || 'Unknown'} - ${specificScorecard.type || 'Unknown'} (${specificScorecard.challengeTrack || 'Unknown'}) v${specificScorecard.version || 'Unknown'}`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider handling the case where specificScorecard.version is undefined or null separately, as using 'Unknown' for a version number might not be appropriate.

}

// Fallback to searching in the general scorecards array
const scorecard = scorecards.find(s => s.id === reviewer.scorecardId)
return scorecard ? `${scorecard.name} - ${scorecard.type} (${scorecard.challengeTrack}) v${scorecard.version}` : 'Not selected'
return scorecard ? `${scorecard.name || 'Unknown'} - ${scorecard.type || 'Unknown'} (${scorecard.challengeTrack || 'Unknown'}) v${scorecard.version || 'Unknown'}` : 'Not selected'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider handling the case where scorecard.version is undefined or null separately, as using 'Unknown' for a version number might not be appropriate.

})()}
</span>
) : (
Expand All @@ -359,13 +399,15 @@ class ChallengeReviewerField extends Component {
<option value=''>Select Scorecard</option>
{scorecards.map(scorecard => (
<option key={scorecard.id} value={scorecard.id}>
{scorecard.name} - {scorecard.type} ({scorecard.challengeTrack}) v{scorecard.version}
{scorecard.name || 'Unknown'} - {scorecard.type || 'Unknown'} ({scorecard.challengeTrack || 'Unknown'}) v{scorecard.version || 'Unknown'}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a more descriptive placeholder than 'Unknown' for missing scorecard details to provide clearer context to the user. For example, 'Name not available', 'Type not specified', etc.

</option>
))}
</select>
)}
{validationErrors.scorecardId && (
<div className={styles.fieldError}>{validationErrors.scorecardId}</div>
{!readOnly && challenge.submitTriggered && validationErrors.scorecardId && (
<div className={styles.error}>
{validationErrors.scorecardId}
</div>
)}
</div>
)}
Expand Down Expand Up @@ -409,8 +451,10 @@ class ChallengeReviewerField extends Component {
))}
</select>
)}
{validationErrors.phaseId && (
<div className={styles.fieldError}>{validationErrors.phaseId}</div>
{!readOnly && challenge.submitTriggered && validationErrors.phaseId && (
<div className={styles.error}>
{validationErrors.phaseId}
</div>
)}
</div>
</div>
Expand All @@ -433,8 +477,10 @@ class ChallengeReviewerField extends Component {
}}
/>
)}
{validationErrors.memberReviewerCount && (
<div className={styles.fieldError}>{validationErrors.memberReviewerCount}</div>
{!readOnly && challenge.submitTriggered && validationErrors.memberReviewerCount && (
<div className={styles.error}>
{validationErrors.memberReviewerCount}
</div>
)}
</div>

Expand All @@ -453,8 +499,10 @@ class ChallengeReviewerField extends Component {
}}
/>
)}
{validationErrors.basePayment && (
<div className={styles.fieldError}>{validationErrors.basePayment}</div>
{!readOnly && challenge.submitTriggered && validationErrors.basePayment && (
<div className={styles.error}>
{validationErrors.basePayment}
</div>
)}
</div>

Expand Down Expand Up @@ -617,13 +665,15 @@ ChallengeReviewerField.propTypes = {
metadata: PropTypes.shape({
scorecards: PropTypes.array,
defaultReviewers: PropTypes.array,
workflows: PropTypes.array
workflows: PropTypes.array,
scorecardById: PropTypes.object
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a default value for scorecardById in metadata to prevent potential undefined errors.

}),
isLoading: PropTypes.bool,
readOnly: PropTypes.bool,
loadScorecards: PropTypes.func.isRequired,
loadDefaultReviewers: PropTypes.func.isRequired,
loadWorkflows: PropTypes.func.isRequired
loadWorkflows: PropTypes.func.isRequired,
loadScorecardById: PropTypes.func.isRequired
}

const mapStateToProps = (state) => ({
Expand All @@ -634,7 +684,8 @@ const mapStateToProps = (state) => ({
const mapDispatchToProps = {
loadScorecards,
loadDefaultReviewers,
loadWorkflows
loadWorkflows,
loadScorecardById
}

export default connect(mapStateToProps, mapDispatchToProps)(ChallengeReviewerField)
44 changes: 44 additions & 0 deletions src/components/ChallengeEditor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,46 @@ class ChallengeEditor extends Component {
return true
}

isValidReviewers () {
const { challenge } = this.state
const reviewers = challenge.reviewers || []

if (reviewers.length === 0) {
return true
}

// Validate each reviewer
for (const reviewer of reviewers) {
const isAI = (reviewer.aiWorkflowId && reviewer.aiWorkflowId.trim() !== '') || !reviewer.isMemberReview

if (isAI) {
if (!reviewer.aiWorkflowId || reviewer.aiWorkflowId.trim() === '') {
return false
}
} else {
if (!reviewer.scorecardId) {
return false
}

const memberCount = parseInt(reviewer.memberReviewerCount) || 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parseInt function should specify a radix to avoid unexpected behavior. Consider using parseInt(reviewer.memberReviewerCount, 10).

if (memberCount < 1 || !Number.isInteger(memberCount)) {
return false
}

const basePayment = convertDollarToInteger(reviewer.basePayment || '0', '')
if (basePayment < 0) {
return false
}
}

if (!reviewer.phaseId) {
return false
}
}

return true
}

isValidChallenge () {
const { challenge } = this.state
if (this.props.isNew) {
Expand All @@ -790,6 +830,10 @@ class ChallengeEditor extends Component {
return false
}

if (!this.isValidReviewers()) {
return false
}

const requiredFields = [
'trackId',
'typeId',
Expand Down
9 changes: 9 additions & 0 deletions src/services/challenges.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,15 @@ export async function fetchScorecards (filters = {}) {
const response = await axiosInstance.get(`${SCORECARDS_API_URL}?${qs.stringify(query, { encode: false })}`)
return _.get(response, 'data', {})
}
/**
* Api request for fetching a specific scorecard by ID
* @param {string} scorecardId the scorecard ID
* @returns {Promise<*>}
*/
export async function fetchScorecardById (scorecardId) {
const response = await axiosInstance.get(`${SCORECARDS_API_URL}/${scorecardId}`)
return _.get(response, 'data', {})
}
/**
* Api request for fetching default reviewers
* @param {Object} filters filters for default reviewers
Expand Down