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
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ workflows:
context: org-global
filters: &filters-dev
branches:
only: ["develop", "pm-2917", "points", "pm-3270", "engagements"]
only: ["develop", "pm-2917", "points", "pm-3270", "projects-api-v6"]

# Production builds are exectuted only on tagged commits to the
# master branch.
Expand Down
2 changes: 1 addition & 1 deletion config/constants/development.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ module.exports = {
CHALLENGE_PHASES_URL: `${DEV_API_HOSTNAME}/v6/challenge-phases`,
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.

GROUPS_API_URL: `${DEV_API_HOSTNAME}/v6/groups`,
TERMS_API_URL: `${DEV_API_HOSTNAME}/v5/terms`,
RESOURCES_API_URL: `${DEV_API_HOSTNAME}/v6/resources`,
Expand Down
5 changes: 3 additions & 2 deletions config/constants/local.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const LOCAL_MEMBER_API = 'http://localhost:3003/v6'
const LOCAL_RESOURCE_API = 'http://localhost:3004/v6'
const LOCAL_REVIEW_API = 'http://localhost:3005/v6'
const LOCAL_SKILLS_API_V5 = 'http://localhost:3006/v5/standardized-skills'
const LOCAL_PROJECTS_API = 'http://localhost:3008/v6/projects'
// Lookups API available on 3007 if needed in future
// const LOCAL_LOOKUPS_API = 'http://localhost:3007/v6'

Expand Down Expand Up @@ -46,8 +47,8 @@ module.exports = {
// Copilots and other apps remain on dev
COPILOTS_URL: 'https://copilots.topcoder-dev.com',

// 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.


// Local groups/resources/review services
GROUPS_API_URL: `${LOCAL_GROUPS_API}/groups`,
Expand Down
2 changes: 1 addition & 1 deletion config/constants/production.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module.exports = {
CHALLENGE_PHASES_URL: `${PROD_API_HOSTNAME}/v6/challenge-phases`,
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.

GROUPS_API_URL: `${PROD_API_HOSTNAME}/v6/groups`,
TERMS_API_URL: `${PROD_API_HOSTNAME}/v5/terms`,
MEMBERS_API_URL: `${PROD_API_HOSTNAME}/v5/members`,
Expand Down
91 changes: 69 additions & 22 deletions src/components/ChallengeEditor/ChallengeReviewer-Field/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,45 @@ const normalizeTrackForScorecards = (challenge, metadata) => {
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.

.toString()
.toLowerCase()
.trim()
.replace(/\bphase\b$/, '')
.replace(/[-_\s]/g, '')

const normalizeIdValue = (value) => (
value === undefined || value === null
? ''
: value.toString()
)

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.

if (!normalizedPhaseId) {
return []
}

const selectedPhase = phases.find(phase => (
normalizeIdValue(phase.phaseId) === normalizedPhaseId ||
normalizeIdValue(phase.id) === normalizedPhaseId
))

if (!selectedPhase || !selectedPhase.name) {
return []
}

const normalizedPhaseName = normalizePhaseToken(selectedPhase.name)
if (!normalizedPhaseName) {
return []
}

return scorecards.filter(scorecard => (
scorecard &&
normalizePhaseToken(scorecard.type) === normalizedPhaseName
))
}

class ChallengeReviewerField extends Component {
constructor (props) {
super(props)
Expand Down Expand Up @@ -602,6 +641,31 @@ class ChallengeReviewerField extends Component {
baseCoefficient: defaultReviewer.baseCoefficient,
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 { metadata = {} } = this.props
const scorecardsForPhase = getScorecardsForPhase(
metadata.scorecards || [],
challenge.phases || [],
value
)
const currentScorecardId = normalizeIdValue(updatedReviewers[index].scorecardId)
const hasCurrentScorecard = scorecardsForPhase.some(scorecard => (
normalizeIdValue(scorecard.id) === currentScorecardId
))

if (!hasCurrentScorecard) {
const defaultScorecardId = normalizeIdValue(defaultReviewer && defaultReviewer.scorecardId)
const hasDefaultScorecard = defaultScorecardId && scorecardsForPhase.some(scorecard => (
normalizeIdValue(scorecard.id) === defaultScorecardId
))
const fallbackScorecardId = hasDefaultScorecard
? defaultScorecardId
: normalizeIdValue(scorecardsForPhase[0] && scorecardsForPhase[0].id)

fieldUpdate.scorecardId = fallbackScorecardId || ''
}
}
}

if (field === 'memberReviewerCount') {
Expand Down Expand Up @@ -661,29 +725,12 @@ class ChallengeReviewerField extends Component {
const { challenge, metadata = {}, readOnly = false } = this.props
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.

scorecards,
challenge.phases || [],
reviewer.phaseId
)
const isDesignChallenge = challenge && challenge.trackId === DES_TRACK_ID
const normalize = (value) => (value || '')
.toString()
.toLowerCase()
.trim()
.replace(/\bphase\b$/, '')
.replace(/[-_\s]/g, '')

const filteredScorecards = scorecards.filter(item => {
if (!selectedPhase || !selectedPhase.name || !item || !item.type) {
return false
}

const normalizedType = normalize(item.type)
const normalizedPhaseName = normalize(selectedPhase.name)

if (!normalizedType || !normalizedPhaseName) {
return false
}

return normalizedType === normalizedPhaseName
})

return (
<div key={`reviewer-${index}`} className={styles.reviewerForm}>
Expand Down
29 changes: 21 additions & 8 deletions src/components/ChallengeEditor/ChallengeView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import PhaseInput from '../../PhaseInput'
import CheckpointPrizesField from '../CheckpointPrizes-Field'
import { isBetaMode } from '../../../util/localstorage'
import WiproAllowedField from '../WiproAllowedField'

const ChallengeView = ({
projectDetail,
Expand Down Expand Up @@ -95,6 +96,7 @@ const ChallengeView = ({
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.

const phases = _.get(challenge, 'phases', [])
const showCheckpointPrizes = _.get(challenge, 'timelineTemplateId') === MULTI_ROUND_CHALLENGE_TEMPLATE_ID
const useDashboardData = _.find(challenge.metadata, { name: 'show_data_dashboard' })
Expand Down Expand Up @@ -195,6 +197,7 @@ const ChallengeView = ({
<>
{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.

<div className={cn(styles.row, styles.topRow)}>
<div className={styles.col}>
<span><span className={styles.fieldTitle}>Groups:</span> {groups}</span>
Expand Down Expand Up @@ -262,14 +265,24 @@ const ChallengeView = ({
token={token}
readOnly
/>}
<ChallengePrizesField challenge={challenge} readOnly />
{
showCheckpointPrizes && (
<CheckpointPrizesField challenge={challenge} readOnly />
)
}
<CopilotFeeField challenge={challenge} readOnly />
<ChallengeTotalField challenge={challenge} />
{isFunChallenge ? (
<div className={cn(styles.row, styles.topRow)}>
<div className={styles.col}>
<span><span className={styles.fieldTitle}>Fun Challenge:</span> True</span>
</div>
</div>
) : (
<>
<ChallengePrizesField challenge={challenge} readOnly />
{
showCheckpointPrizes && (
<CheckpointPrizesField challenge={challenge} readOnly />
)
}
<CopilotFeeField challenge={challenge} readOnly />
<ChallengeTotalField challenge={challenge} />
</>
)}
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
@use '../../../styles/includes' as *;

.row {
box-sizing: border-box;
display: flex;
flex-direction: row;
margin: 30px 30px 0 30px;
align-content: space-between;
justify-content: flex-start;

.tcCheckbox {
@include tc-checkbox;

height: 18px;
width: 210px;
margin: 0;
padding: 0;
vertical-align: bottom;
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.

display: none;
}

label {
@include roboto-light();

line-height: 17px;
font-weight: 300;
cursor: pointer;
position: absolute;
display: inline-block;
width: 14px;
height: 14px;
top: 0;
left: 0;
border: none;
box-shadow: none;
background: $tc-gray-30;
transition: all 0.15s ease-in-out;

&::after {
opacity: 0;
content: '';
position: absolute;
width: 9px;
height: 5px;
background: transparent;
top: 2px;
left: 2px;
border-top: none;
border-right: none;
transform: rotate(-45deg);
transition: all 0.15s ease-in-out;
}

&:hover::after {
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.

margin-left: 24px;
width: 300px;
}
}

input[type='checkbox']:checked ~ label {
background: $tc-blue-20;
}

input[type='checkbox']:checked + label::after {
border-color: $white;
}
}
}
47 changes: 47 additions & 0 deletions src/components/ChallengeEditor/FunChallengeField/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React from 'react'
import PropTypes from 'prop-types'
import styles from './FunChallengeField.module.scss'

/**
* Renders a checkbox to toggle the `funChallenge` flag for Marathon Match challenges.
*
* @param {Object} props component props
* @param {Object} props.challenge challenge data object that may include `funChallenge`
* @param {Function} props.onUpdateOthers callback used to update top-level challenge fields
* @param {boolean} props.readOnly when true, renders the control as read-only
* @returns {import('react').ReactNode} rendered fun challenge checkbox field
*/
const FunChallengeField = ({ challenge, onUpdateOthers, readOnly }) => {
const isFunChallenge = challenge.funChallenge === true

return (
<div className={styles.row}>
<div className={styles.tcCheckbox}>
<input
name='funChallenge'
type='checkbox'
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.

</label>
</div>
</div>
)
}

FunChallengeField.defaultProps = {
readOnly: false
}

FunChallengeField.propTypes = {
challenge: PropTypes.shape().isRequired,
onUpdateOthers: PropTypes.func.isRequired,
readOnly: PropTypes.bool
}

export default FunChallengeField
Loading
Loading