Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on April 10. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
|
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
|
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
|
| } | ||
| } | ||
| // Make the admins | ||
| await Promise.all( |
There was a problem hiding this comment.
Should we handle errors in here also, what if createGroupAdmin fails in here ?
| } | ||
| } | ||
| // Make the admins | ||
| await Promise.all( |
There was a problem hiding this comment.
Same here, should these Promise.all calls handle errors for consistency with the add/remove users fix above?
talissoncosta
left a comment
There was a problem hiding this comment.
Hey @kyle-ssg , thanks for handling this! I've left two inline comments, they're not blockers, but since the goal of this PR is to stop swallowing errors silently, it felt worth flagging for consistency.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
When adding a group member, the FE was sending up all existing group members. This caused an issue where
MANAGE_USER_GROUPSdid not have appropriate permissions to add themselves to a group.Also, if a change errors the FE was silently swallowing them instead of showing them.
How did you test this code?
Added and removed members during edit / creation of a group