Skip to content

Projects api v6#1735

Merged
jmgasper merged 2 commits intodevelopfrom
projects-api-v6
Mar 3, 2026
Merged

Projects api v6#1735
jmgasper merged 2 commits intodevelopfrom
projects-api-v6

Conversation

@jmgasper
Copy link
Collaborator

@jmgasper jmgasper commented Mar 3, 2026

No description provided.

@jmgasper jmgasper merged commit 82358f6 into develop Mar 3, 2026
4 of 5 checks passed
CHALLENGE_TIMELINES_URL: `${DEV_API_HOSTNAME}/v6/challenge-timelines`,
COPILOTS_URL: 'https://copilots.topcoder-dev.com',
PROJECT_API_URL: `${DEV_API_HOSTNAME}/v5/projects`,
PROJECT_API_URL: `${DEV_API_HOSTNAME}/v6/projects`,
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
Ensure that all dependent services and clients are updated to use the new v6 endpoint for projects. This change could break functionality if any part of the system still relies on the v5 endpoint.

// Projects API: keep dev unless you run projects locally
PROJECT_API_URL: `${DEV_API_HOSTNAME}/v5/projects`,
// Projects API v6: keep dev default (switch to LOCAL_PROJECTS_API when needed)
PROJECT_API_URL: `${DEV_API_HOSTNAME}/v6/projects`,
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
The change from v5 to v6 in the PROJECT_API_URL may require verification that all dependent services and clients are compatible with the new API version. Ensure that any breaking changes in the API are addressed.

CHALLENGE_TIMELINES_URL: `${PROD_API_HOSTNAME}/v6/challenge-timelines`,
COPILOTS_URL: `https://copilots.${DOMAIN}`,
PROJECT_API_URL: `${PROD_API_HOSTNAME}/v5/projects`,
PROJECT_API_URL: `${PROD_API_HOSTNAME}/v6/projects`,
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
Ensure that all dependencies and services consuming the PROJECT_API_URL are compatible with the new API version v6. This change might introduce breaking changes if the API endpoints have different contracts between v5 and v6.

return null
}

const normalizePhaseToken = (value) => (value || '')
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
The normalizePhaseToken function removes the word 'phase' from the end of the string. Ensure this behavior is intended, as it might lead to unexpected results if 'phase' is a meaningful part of the phase name.

)

const getScorecardsForPhase = (scorecards = [], phases = [], phaseId) => {
const normalizedPhaseId = normalizeIdValue(phaseId)
Copy link

Choose a reason for hiding this comment

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

[💡 maintainability]
In getScorecardsForPhase, the function returns an empty array if normalizedPhaseId is falsy. Consider logging or handling this scenario if it indicates a potential issue with the input data.

incrementalCoefficient: defaultReviewer.incrementalCoefficient
})

if (updatedReviewers[index] && (updatedReviewers[index].isMemberReview !== false)) {
Copy link

Choose a reason for hiding this comment

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

[💡 readability]
The logic for updating fieldUpdate.scorecardId when hasCurrentScorecard is false could be simplified for clarity. Consider refactoring to make the fallback logic more explicit.

const { scorecards = [], workflows = [] } = metadata
const validationErrors = challenge.submitTriggered ? this.validateReviewer(reviewer) : {}
const selectedPhase = challenge.phases.find(p => p.phaseId === reviewer.phaseId)
const filteredScorecards = getScorecardsForPhase(
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
The getScorecardsForPhase function is used to filter scorecards based on phase. Ensure that the filtering logic aligns with the intended business rules, especially if phase names or IDs can change.

if (isLoading || _.isEmpty(metadata.challengePhases) || challenge.id !== challengeId) return <Loader />
const showTimeline = false // disables the timeline for time being https://github.com/topcoder-platform/challenge-engine-ui/issues/706
const isTask = _.get(challenge, 'task.isTask', false)
const isFunChallenge = challenge.funChallenge === true
Copy link

Choose a reason for hiding this comment

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

[💡 correctness]
The isFunChallenge variable is set using a strict equality check (=== true). Consider using a more flexible check like !!challenge.funChallenge to handle cases where funChallenge might be truthy but not strictly true.

<>
{dashboardToggle}
<NDAField beta challenge={challenge} readOnly />
<WiproAllowedField challenge={challenge} onUpdateOthers={() => {}} readOnly />
Copy link

Choose a reason for hiding this comment

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

[💡 maintainability]
The onUpdateOthers prop for WiproAllowedField is passed an empty function. If this is intentional, consider adding a comment explaining why it's necessary to pass this prop with an empty function. If not needed, it might be cleaner to omit it.

position: relative;
display: inline-block;

input[type='checkbox'] {
Copy link

Choose a reason for hiding this comment

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

[⚠️ accessibility]
Hiding the checkbox input with display: none; can cause accessibility issues, as screen readers may not be able to interact with it. Consider using opacity: 0; and position: absolute; to hide it visually while keeping it accessible.

opacity: 0.3;
}

div {
Copy link

Choose a reason for hiding this comment

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

[⚠️ design]
The div inside the label might cause layout issues if the label is intended to be a clickable area for the checkbox. Ensure that the div does not interfere with the expected behavior of the label.

id='funChallenge'
checked={isFunChallenge}
readOnly={readOnly}
onChange={() => onUpdateOthers({ field: 'funChallenge', value: !isFunChallenge })}
Copy link

Choose a reason for hiding this comment

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

[⚠️ performance]
The onChange handler directly calls onUpdateOthers with a new object every time the checkbox is toggled. Consider memoizing the handler using useCallback to prevent unnecessary re-renders and improve performance.

/>
<label htmlFor='funChallenge'>
<div>Fun Challenge</div>
<input type='hidden' />
Copy link

Choose a reason for hiding this comment

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

[💡 maintainability]
The <input type='hidden' /> inside the <label> seems unnecessary and could be removed unless it serves a specific purpose not evident from the current context.

position: relative;
display: inline-block;

input[type='checkbox'] {
Copy link

Choose a reason for hiding this comment

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

[❗❗ accessibility]
Hiding the checkbox input with display: none; can cause accessibility issues, as screen readers may not be able to interact with it. Consider using visually-hidden techniques instead to ensure accessibility.

line-height: 17px;
font-weight: 300;
cursor: pointer;
position: absolute;
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using position: absolute; for the label without specifying a containing block can lead to unexpected positioning issues. Ensure that the parent element has position: relative; to avoid layout problems.

border: none;
box-shadow: none;
background: $tc-gray-30;
transition: all 0.15s ease-in-out;
Copy link

Choose a reason for hiding this comment

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

[⚠️ performance]
The use of transition: all 0.15s ease-in-out; can lead to performance issues as it transitions all properties. Consider specifying only the properties that need to be transitioned to improve performance.

}

input[type='checkbox']:checked + label::after {
border-color: $white;
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
The border-color property is being set to $white, but the ::after pseudo-element does not have a border initially. Ensure that a border is defined for the ::after element when checked.

id='wiproAllowed'
checked={isWiproAllowed}
readOnly={readOnly}
onChange={() => onUpdateOthers({ field: 'wiproAllowed', value: !isWiproAllowed })}
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider adding a check to ensure onUpdateOthers is a function before calling it to prevent potential runtime errors if it's undefined or not a function.

/>
<label htmlFor='wiproAllowed'>
<div>Wipro Allowed</div>
<input type='hidden' />
Copy link

Choose a reason for hiding this comment

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

[💡 maintainability]
The hidden input inside the label seems unnecessary unless it serves a specific purpose not evident from the current code. Consider removing it to simplify the component.

}

WiproAllowedField.propTypes = {
challenge: PropTypes.shape().isRequired,
Copy link

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The PropTypes.shape() for challenge is too generic. Consider defining the expected shape of the challenge object to improve type safety and maintainability.

if (!isSub) {
newChallenge[field] = option
if (field === 'typeId' && option !== MARATHON_TYPE_ID) {
newChallenge.funChallenge = false
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
The condition option !== MARATHON_TYPE_ID could potentially lead to unexpected behavior if option is null or undefined. Consider explicitly checking for these cases to ensure funChallenge is only reset when option is a valid non-Marathon type.

}

const copilotFee = _.find(challenge.prizeSets, p => p.type === PRIZE_SETS_TYPE.COPILOT_PAYMENT, [])
if (copilotFee && parseInt(copilotFee.prizes[0].value) > 0 && !challenge.copilot) {
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
Ensure that copilotFee.prizes[0] exists before accessing value to avoid potential runtime errors. Consider adding a check for copilotFee.prizes and its length.

challenge.timelineTemplateId = _.get(this.getCurrentTemplate(), 'id')
challenge.projectId = this.props.projectId
challenge.prizeSets = challenge.prizeSets.map(p => {
challenge.prizeSets = (challenge.prizeSets || []).map(p => {
Copy link

Choose a reason for hiding this comment

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

[❗❗ correctness]
The use of map on challenge.prizeSets assumes it is always an array. Consider adding a check to ensure prizeSets is defined and is an array to prevent potential runtime errors.

return { ...p, prizes }
})
if (challenge.funChallenge === true) {
delete challenge.prizeSets
Copy link

Choose a reason for hiding this comment

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

[⚠️ design]
Deleting prizeSets when funChallenge is true might lead to unexpected behavior if other parts of the code rely on prizeSets being present. Ensure that this deletion is safe and won't cause issues elsewhere.

loadChallengeDetails,
loadResources,
loadSubmissions,
submissionsPerPage
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
The submissionsPerPage parameter is added to the fetchChallengeDetails method call. Ensure that this parameter is correctly handled within the fetchChallengeDetails method to avoid potential issues with pagination or data fetching.

fetchNextProjects
} = this.props
const { challengeTypes = [] } = metadata
const isActiveProjectLoaded =
Copy link

Choose a reason for hiding this comment

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

[💡 performance]
The use of template literals for comparing reduxProjectInfo.id and activeProjectId is unnecessary and could be replaced with a direct comparison. This would improve performance slightly by avoiding the overhead of string conversion.

*/
export async function fetchSubmissions (challengeId, pageObj) {
const { page, perPage } = pageObj
export async function fetchSubmissions (challengeId, pageObj = {}) {
Copy link

Choose a reason for hiding this comment

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

[⚠️ correctness]
The pageObj parameter now has a default value of an empty object. Ensure that all usages of fetchSubmissions handle this change appropriately, as previously passing undefined would have resulted in a destructuring error.

export async function fetchSubmissions (challengeId, pageObj = {}) {
const page = _.get(pageObj, 'page', 1)
const perPage = _.get(pageObj, 'perPage', 10)
const response = await axiosInstance.get(`${SUBMISSIONS_API_URL}?challengeId=${challengeId}&perPage=${perPage}&page=${page}`)
Copy link

Choose a reason for hiding this comment

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

[⚠️ security]
Consider using encodeURIComponent for challengeId, perPage, and page when constructing the URL to prevent potential issues with special characters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant