Conversation
jmgasper
left a comment
There was a problem hiding this comment.
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.
[💡 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.
[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.