Skip to content

Replaced handle with memberId for challenge createdBy comparison - #1

Merged
rishabhtc merged 5 commits into
developfrom
PM-2332
Oct 30, 2025
Merged

rishabhtc merged 5 commits into
developfrom
PM-2332

Conversation

@rishabhtc

Copy link
Copy Markdown
Contributor

No description provided.

@jmgasper jmgasper left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread src/services/ResourceService.js Resolved
Comment thread src/services/ResourceService.js Resolved
Comment thread 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)

Copy link
Copy Markdown

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.

Comment thread src/services/ResourceService.js Resolved
handle = memberInfoFromDb.handle
const userResources = allResources.filter((r) => _.toLower(r.memberHandle) === _.toLower(handle))

let restrictedRoleIds

Copy link
Copy Markdown

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.

@rishabhtc
rishabhtc requested a review from jmgasper October 29, 2025 17:31
@rishabhtc
rishabhtc merged commit 3bdb524 into develop Oct 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants