[PM-28260] Optimize bulk reinvite endpoint #6670
Conversation
- Added IBulkResendOrganizationInvitesCommand interface to define the bulk resend operation. - Created BulkResendOrganizationInvitesCommand class to handle the logic for resending invites to multiple organization users. - Integrated logging and validation to ensure only valid users receive invites. - Included error handling for non-existent organizations and invalid user statuses.
- Implemented comprehensive test cases for the BulkResendOrganizationInvitesCommand class. - Validated user statuses and ensured correct handling of valid and invalid users during bulk invite resends. - Included tests for scenarios such as organization not found and empty user lists. - Utilized Xunit and NSubstitute for effective testing and mocking of dependencies.
- Registered IBulkResendOrganizationInvitesCommand in the service collection for dependency injection.
…InvitesCommand - Added IBulkResendOrganizationInvitesCommand to the OrganizationUsersController for handling bulk invite resends based on feature flag. - Updated BulkReinvite method to conditionally use the new command or the legacy service based on the feature flag status. - Enhanced unit tests to verify correct command usage depending on feature flag state, ensuring robust testing for both scenarios.
|
New Issues (2)Checkmarx found the following issues in this Pull Request
Fixed Issues (5)Great job! The following issues were fixed in this Pull Request
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6670 +/- ##
==========================================
- Coverage 57.27% 53.48% -3.80%
==========================================
Files 1917 1918 +1
Lines 85518 85565 +47
Branches 7673 7678 +5
==========================================
- Hits 48983 45764 -3219
- Misses 34691 38030 +3339
+ Partials 1844 1771 -73 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Claude finished @r-tome's task —— View job Code Review CompleteI've reviewed the PR that adds 👍 Strengths
🎨 Suggestions for Improvement1. Parameter Naming Inconsistency (Minor)Location: The parameter is named // Current
IEnumerable<Guid> organizationUsersId
// Suggested
IEnumerable<Guid> organizationUserIdsThis applies to:
2. Unused Parameter (Minor)Location: The
Current code: public async Task<IEnumerable<Tuple<OrganizationUser, string>>> BulkResendInvitesAsync(
Guid organizationId,
Guid? invitingUserId, // ← Never used
IEnumerable<Guid> organizationUsersId)3. Logging Diagnostics (Question)Location: _logger.LogUserInviteStateDiagnostics(orgUsers);
ℹ️ Architecture Questions4. Error Handling StrategyLocation: The command filters invalid users and returns them with error messages, while valid users proceed to send invites. This is good, but consider:
5. Performance Implications of GetManyAsyncLocation: var orgUsers = await _organizationUserRepository.GetManyAsync(organizationUsersId);For 8,000+ users:
✅ Security Review✅ No security concerns identified. The implementation:
♻️ Consider for Future Improvements
📝 Test Coverage NotesThe test coverage is excellent. The controller tests properly verify:
The command tests cover:
SummaryThis is a solid implementation that delivers significant performance improvements while maintaining code quality and test coverage. The suggestions above are mostly minor improvements that don't block merging. Recommendation: ✅ Approve with minor suggestions for consideration. The most important items to address:
|
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-28260
📔 Objective
Adds
BulkResendOrganizationInvitesCommandto improve bulk reinvite performance by reusing the existingSendOrganizationInvitesCommandfor batched sending.The new path is behind the
IncreaseBulkReinviteLimitForCloudfeature flag.With the flag enabled, reinviting 8,000 users dropped from ~36,719ms to ~1,400ms in a local development environment, roughly a 26× improvement.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes