-
Notifications
You must be signed in to change notification settings - Fork 9
fix(PM-2177): Send comment creator details and changes comments structure #172
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.
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
Conversation
| - pm-1955_2 | ||
| - re-try-failed-jobs | ||
| - pm-2539 | ||
| - pm-2177_user_detais |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
There is a typo in the branch name pm-2177_user_detais. It should likely be pm-2177_user_details. This could lead to confusion or errors when triggering workflows based on branch names.
| .map((item) => item.createdBy as string); | ||
|
|
||
| const members = await this.memberPrisma.member.findMany({ | ||
| where: { userId: { in: createdByList.map((id) => BigInt(id)) } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[❗❗ correctness]
Using BigInt to convert userId might cause issues if createdByList contains non-numeric strings. Consider validating the input before conversion to ensure all entries are valid numeric strings.
| }); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| const normalized = members.map((m: any) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[maintainability]
The use of any type for members and commentsById can lead to runtime errors due to lack of type safety. Consider defining a specific type or interface for these objects to improve type safety and maintainability.
| for (const item of items) { | ||
| for (const comment of item.comments) { | ||
| if (comment.parentId) { | ||
| const parent = commentsById[comment.parentId]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[performance]
The nested loop structure for reconstructing comments could be optimized. Consider using a single pass to build the commentsById map and nest child comments, which would improve performance by reducing the complexity from O(n^2) to O(n).
What's in this PR?
Ticket link - https://topcoder.atlassian.net/browse/PM-2177