Conversation
feat(PM-4068): Implemented categorized skills with max limit of number of skills shown per category in PDF download
PM-4168 on profile completeness check openToWork user trait
PM-4168 #30m Fix completed state for engagement availability
![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 using parseInt with a radix of 10 instead of Number for parsing process.env.PDF_SKILLS_PER_CATEGORY. This ensures that the conversion is always done in base 10, which can prevent unexpected results if the environment variable is prefixed with a non-decimal number.
| * 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.
[💡 readability]
The check !skillNames || skillNames.length === 0 is redundant since skillNames.length === 0 will suffice to check for an empty array. Consider simplifying the condition to if (skillNames.length === 0) return null.
| */ | ||
| function buildProfileTemplate (pdfData) { | ||
| const { member, workExperience, education, languages, basicInfo, skills, topcoderActivity, certifications, courses } = pdfData | ||
| const { member, workExperience, education, languages, basicInfo, skills, skillsByCategory, topcoderActivity, certifications, courses } = pdfData |
There was a problem hiding this comment.
[correctness]
The destructuring of pdfData now includes skillsByCategory. Ensure that this property is always present in the pdfData object to avoid potential runtime errors if it is undefined.
| if (principalSubsection) { | ||
| skillsContent.push(principalSubsection) | ||
| const hasPrincipalSkills = skills && skills.principal && (skills.principal.verified.length > 0 || skills.principal.notVerified.length > 0) | ||
| const hasAdditionalByCategory = skillsByCategory && skillsByCategory.length > 0 |
There was a problem hiding this comment.
[correctness]
The condition skillsByCategory && skillsByCategory.length > 0 is used to check for additional skills by category. Ensure that skillsByCategory is always an array to avoid potential runtime errors.
| // Avoid getting the member stats, since we don't need them here, and performance is | ||
| // better without them | ||
| const memberFields = { 'fields': 'userId,handle,handleLower,photoURL,description,skills,verified,availableForGigs,availableForGigsLastUpdateDate,lastProfileConfirmationDate,updatedAt,addresses' } | ||
| const memberFields = { 'fields': 'userId,handle,handleLower,photoURL,description,skills,verified,lastProfileConfirmationDate,updatedAt,addresses' } |
There was a problem hiding this comment.
[correctness]
The removal of availableForGigs and availableForGigsLastUpdateDate from memberFields might impact other parts of the code that rely on these fields. Ensure that these fields are not required elsewhere in the application logic.
| data.gigAvailabilityLastUpdateDate = member.availableForGigsLastUpdateDate || undefined | ||
| } | ||
|
|
||
| _.forEach(memberTraits, (item) => { |
There was a problem hiding this comment.
[correctness]
The removal of the if block checking member.availableForGigs may affect the completeness calculation logic. Verify that this change aligns with the intended behavior and does not omit necessary checks.
| principal: principalSkills, | ||
| additional: additionalSkills | ||
| }, | ||
| skills: { principal: principalSkills }, |
There was a problem hiding this comment.
[maintainability]
The restructuring of skills into skillsByCategory without retaining the additionalSkills object may affect any logic that previously relied on additionalSkills. Ensure that this change is consistent with the rest of the application and that no functionality is lost.
3 participants
https://topcoder.atlassian.net/browse/PM-4068
https://topcoder.atlassian.net/browse/PM-4168