Skip to content

Don't force skills selection on Topgear challenge edits #1706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 4, 2025
Merged

Don't force skills selection on Topgear challenge edits #1706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions src/components/ChallengeEditor/SkillsField/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Select from '../../Select'
import { searchSkills } from '../../../services/skills'
import cn from 'classnames'
import styles from './styles.module.scss'
import { AUTOCOMPLETE_DEBOUNCE_TIME_MS } from '../../../config/constants'
import { AUTOCOMPLETE_DEBOUNCE_TIME_MS, SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS } from '../../../config/constants'
import _ from 'lodash'

const fetchSkills = _.debounce((inputValue, callback) => {
Expand All @@ -27,12 +27,15 @@ const SkillsField = ({ readOnly, challenge, onUpdateSkills }) => {
value: skill.id
})), [challenge.skills])
const existingSkills = useMemo(() => selectedSkills.map(item => item.label).join(','), [selectedSkills])
const billingAccountId = _.get(challenge, 'billing.billingAccountId')
const normalizedBillingAccountId = _.isNil(billingAccountId) ? null : String(billingAccountId)
const skillsRequired = normalizedBillingAccountId ? !SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS.includes(normalizedBillingAccountId) : true
Copy link

@github-actions github-actions bot Nov 4, 2025

Choose a reason for hiding this comment

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

[💡 readability]
The logic for determining skillsRequired could be simplified. Consider using a direct check for billingAccountId and SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS to improve readability and reduce the need for normalizedBillingAccountId. For example: const skillsRequired = !billingAccountId || !SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS.includes(String(billingAccountId));


return (
<>
<div className={styles.row}>
<div className={cn(styles.field, styles.col1)}>
<label htmlFor='keywords'>Skills {!readOnly && (<span>*</span>)} :</label>
<label htmlFor='keywords'>Skills {!readOnly && skillsRequired && (<span>*</span>)} :</label>
</div>
<div className={cn(styles.field, styles.col2)}>
<input type='hidden' />
Expand All @@ -58,7 +61,7 @@ const SkillsField = ({ readOnly, challenge, onUpdateSkills }) => {
</div>
</div>

{ !readOnly && challenge.submitTriggered && (!selectedSkills || !selectedSkills.length) && <div className={styles.row}>
{ !readOnly && skillsRequired && challenge.submitTriggered && (!selectedSkills || !selectedSkills.length) && <div className={styles.row}>
<div className={cn(styles.field, styles.col1)} />
<div className={cn(styles.field, styles.col2, styles.error)}>
Select at least one skill
Expand Down
11 changes: 9 additions & 2 deletions src/components/ChallengeEditor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import {
PHASE_PRODUCT_CHALLENGE_ID_FIELD,
QA_TRACK_ID, DESIGN_CHALLENGE_TYPES, ROUND_TYPES,
MULTI_ROUND_CHALLENGE_TEMPLATE_ID, DS_TRACK_ID,
CHALLENGE_STATUS
CHALLENGE_STATUS,
SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS
} from '../../config/constants'
import {
getDomainTypes,
Expand Down Expand Up @@ -867,14 +868,20 @@ class ChallengeEditor extends Component {
return false
}

const billingAccountId = _.get(challenge, 'billing.billingAccountId')
const normalizedBillingAccountId = _.isNil(billingAccountId) ? null : String(billingAccountId)
Copy link

@github-actions github-actions bot Nov 4, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The conversion of billingAccountId to a string using String(billingAccountId) could potentially lead to unexpected behavior if billingAccountId is an object or a complex type. Consider using String(billingAccountId) only if you are certain that billingAccountId will always be a primitive type. Alternatively, ensure that billingAccountId is validated or sanitized before this conversion.

const isSkillsRequired = normalizedBillingAccountId ? !SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS.includes(normalizedBillingAccountId) : true
Copy link

@github-actions github-actions bot Nov 4, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The logic for determining isSkillsRequired assumes that SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS is an array of strings. Ensure that SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS is consistently defined as such, and consider adding validation to confirm that normalizedBillingAccountId is a valid string before checking its inclusion in the array.


const requiredFields = [
'trackId',
'typeId',
'name',
'description',
'skills',
'prizeSets'
]
if (isSkillsRequired) {
requiredFields.push('skills')
}
let isRequiredMissing = false

requiredFields.forEach((key) => {
Expand Down
1 change: 1 addition & 0 deletions src/config/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const {

export const CREATE_FORUM_TYPE_IDS = typeof process.env.CREATE_FORUM_TYPE_IDS === 'string' ? process.env.CREATE_FORUM_TYPE_IDS.split(',') : process.env.CREATE_FORUM_TYPE_IDS
export const PROJECTS_API_URL = process.env.PROJECTS_API_URL || process.env.PROJECT_API_URL
export const SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS = ['80000062']
Copy link

@github-actions github-actions bot Nov 4, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider making SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS configurable through environment variables instead of hardcoding the value. This would improve maintainability and flexibility, allowing changes without code modifications.


/**
* Filepicker config
Expand Down
Loading