feat(PM-4068): Implemented categorized skills with max limit of number of skills shown per category in PDF download#87
feat(PM-4068): Implemented categorized skills with max limit of number of skills shown per category in PDF download#87hentrymartin wants to merge 4 commits intodevelopfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| LIST_FETCH_COUNT: process.env.MAILCHIMP_LIST_FETCH_COUNT ? Number(process.env.MAILCHIMP_LIST_FETCH_COUNT) : 1000 | ||
| } | ||
| }, | ||
| PDF_SKILLS_PER_CATEGORY: process.env.PDF_SKILLS_PER_CATEGORY ? Number(process.env.PDF_SKILLS_PER_CATEGORY) : 5, |
There was a problem hiding this comment.
[correctness]
Consider validating that process.env.PDF_SKILLS_PER_CATEGORY is a positive integer before converting it with Number(). This will prevent potential issues if the environment variable is set to a non-numeric or negative value.
| * Create one category line in same style as verified/not verified: bullet label + skills on same line | ||
| */ | ||
| function createCategorySkillsBlock (categoryName, skillNames) { | ||
| if (!skillNames || skillNames.length === 0) return null |
There was a problem hiding this comment.
[correctness]
Returning null when skillNames is empty might lead to unexpected behavior if the caller does not handle null values properly. Consider returning an empty React element instead to avoid potential issues.
| skillsContent.push(principalSubsection) | ||
| const hasPrincipalSkills = skills && skills.principal && (skills.principal.verified.length > 0 || skills.principal.notVerified.length > 0) | ||
| const hasAdditionalByCategory = skillsByCategory && skillsByCategory.length > 0 | ||
| if (hasPrincipalSkills || hasAdditionalByCategory) { |
There was a problem hiding this comment.
[correctness]
The condition skillsByCategory && skillsByCategory.length > 0 assumes skillsByCategory is always an array. If skillsByCategory can be null or undefined, this is fine, but if it can be another type, consider adding a type check to ensure it is an array.
1 participant
What's in this PR?
Ticket link - https://topcoder.atlassian.net/browse/PM-4068