Conversation
fix(PM-3734): QA feedbacks on member stats in profile PDF
fix(PM-3847): Don't send links to other users
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const wins = stat.wins ?? 0 | ||
| const submissions = stat.submissions ?? 0 | ||
| const challenges = stat.challenges ?? 0 | ||
| const valueText = `${wins} ${wins === 1 ? 'win' : 'wins'}, ${submissions} ${submissions === 1 ? 'submission' : 'submissions'}, ${challenges} ${challenges === 1 ? 'challenge' : 'challenges'}` |
There was a problem hiding this comment.
[maintainability]
The logic for pluralizing 'wins', 'submissions', and 'challenges' is repeated. Consider extracting this logic into a helper function to improve maintainability and reduce duplication.
| } | ||
|
|
||
| /** Track enum to display name (for standard tracks: wins, submissions, challenges) */ | ||
| /** Track enum to display name (wins, submissions, challenges) */ |
There was a problem hiding this comment.
[💡 readability]
The comment for TRACK_DISPLAY_NAMES no longer accurately describes the mapping, as it mentions 'wins, submissions, challenges' which are not directly related to the purpose of this constant. Consider updating the comment to reflect the actual purpose of the mapping.
| * @param {Object} challengesPrisma challenges Prisma client | ||
| * @param {Object} resourcesPrisma resources Prisma client | ||
| * @returns {Promise<Array<{ trackName: string, wins: number, submissions: number, challenges: number, rating?: number, competitions?: number }>>} | ||
| * @returns {Promise<Array<{ trackName: string, wins: number, submissions: number, challenges: number }>>} |
There was a problem hiding this comment.
[correctness]
The return type in the JSDoc comment for fetchMemberStatsByTrack has been updated to remove rating and competitions. Ensure that this change is intentional and that no other parts of the code rely on these fields being present in the returned data.
| const winners = await challengesPrisma.ChallengeWinner.findMany({ | ||
| where: { userId: numUserId }, | ||
| const winnerRows = await challengesPrisma.ChallengeWinner.findMany({ | ||
| where: { |
There was a problem hiding this comment.
[correctness]
The where clause for findMany now includes a filter on type with values PLACEMENT and PASSED_REVIEW. Verify that these are the only types needed for the logic and that no other types should be considered.
| challenges: counts.challenges ?? 0 | ||
| }) | ||
| } | ||
| if (!hasAny) continue |
There was a problem hiding this comment.
[correctness]
The check for hasAny now only considers numeric values greater than zero. Ensure that this logic change aligns with the intended behavior, especially if there were cases where zero values were previously significant.
| @@ -1736,28 +1721,6 @@ async function aggregatePDFData (currentUser, handle) { | |||
| logger.warn(`aggregatePDFData: statsByTrack failed for ${handle}: ${err.message}`) | |||
There was a problem hiding this comment.
[❗❗ correctness]
The removal of the StatisticsService call and related logic for merging Competitive Programming ratings means this data will no longer be included. Confirm that this removal is intentional and that the application does not require this data elsewhere.
| _.forEach(result, (item) => { | ||
| if (item.traitId === 'personalization' && item.traits && item.traits.data) { | ||
| _.forEach(item.traits.data, (dataEntry) => { | ||
| delete dataEntry.links |
There was a problem hiding this comment.
[performance]
The use of delete in a loop can lead to performance issues because it forces the JavaScript engine to re-optimize the object. Consider setting dataEntry.links to undefined instead, which is generally more performant.
2 participants
No description provided.