Don't force skills selection on Topgear challenge edits #1706
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| 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 |
There was a problem hiding this comment.
[💡 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));
| } | ||
|
|
||
| const billingAccountId = _.get(challenge, 'billing.billingAccountId') | ||
| const normalizedBillingAccountId = _.isNil(billingAccountId) ? null : String(billingAccountId) |
There was a problem hiding this comment.
[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 billingAccountId = _.get(challenge, 'billing.billingAccountId') | ||
| const normalizedBillingAccountId = _.isNil(billingAccountId) ? null : String(billingAccountId) | ||
| const isSkillsRequired = normalizedBillingAccountId ? !SKILLS_OPTIONAL_BILLING_ACCOUNT_IDS.includes(normalizedBillingAccountId) : true |
There was a problem hiding this comment.
[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.
|
|
||
| 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'] |
There was a problem hiding this comment.
[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.
No description provided.