-
Notifications
You must be signed in to change notification settings - Fork 2
Replaced handle with memberId for challenge createdBy comparison #1
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
jmgasper
left a comment
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.
We'll have to test this after the migration because I'm guessing we may not import the legacy data correctly. Shouldn't matter for past tasks, but something to think about.
src/services/ResourceService.js
Outdated
| const existingRestrictedRole = _.find(userResources, r => restrictedRoleIds.includes(r.roleId)) | ||
| if (existingRestrictedRole) { | ||
| // Get the role name for better error message | ||
| const existingRole = await getResourceRole(existingRestrictedRole.roleId, false) |
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 getResourceRole function is called to retrieve the role name for error messaging. If the role name is not critical for the error message, consider simplifying the error message to avoid this additional database call, which could improve performance.
| handle = memberInfoFromDb.handle | ||
| const userResources = allResources.filter((r) => _.toLower(r.memberHandle) === _.toLower(handle)) | ||
|
|
||
| let restrictedRoleIds |
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 variable restrictedRoleIds is initialized twice when isCreated is true, which is unnecessary and could lead to confusion. Consider initializing it once before the conditional blocks to improve clarity and maintainability.
No description provided.