Skip to content

[PM-34389] Add refresh endpoint for organization invite links#7588

Open
r-tome wants to merge 13 commits intomainfrom
ac/pm-34389/refresh-invite-link
Open

[PM-34389] Add refresh endpoint for organization invite links#7588
r-tome wants to merge 13 commits intomainfrom
ac/pm-34389/refresh-invite-link

Conversation

@r-tome
Copy link
Copy Markdown
Contributor

@r-tome r-tome commented May 6, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-34389

📔 Objective

Adds POST /organizations/{orgId}/invite-link/refresh that replaces an existing invite link with a new one (new ID, code, and encryption key) while preserving the allowed domains.

Clients PR.

@r-tome r-tome added the ai-review-vnext Request a Claude code review using the vNext workflow label May 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new POST /organizations/{orgId}/invite-link/refresh endpoint, the RefreshOrganizationInviteLinkCommand, and the new IOrganizationInviteLinkRepository.RefreshAsync method (Dapper + EF). The previous review thread's atomicity concern is resolved: both repository implementations now wrap the delete and insert in a single database transaction, and integration tests verify the rollback path (unique-constraint violation leaves the original link intact). Authorization, DI registration, request validation, and error mapping follow the patterns established by the sibling Create/Update/Delete commands.

Code Review Details

No findings.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

❌ Patch coverage is 97.89474% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.17%. Comparing base (ab00d4f) to head (3f65375).

Files with missing lines Patch % Lines
...nviteLinks/RefreshOrganizationInviteLinkCommand.cs 96.66% 0 Missing and 1 partial ⚠️
...nviteLinks/RefreshOrganizationInviteLinkRequest.cs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7588      +/-   ##
==========================================
+ Coverage   59.76%   64.17%   +4.40%     
==========================================
  Files        2103     2106       +3     
  Lines       92738    92832      +94     
  Branches     8262     8266       +4     
==========================================
+ Hits        55426    59573    +4147     
+ Misses      35354    31208    -4146     
- Partials     1958     2051      +93     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@r-tome r-tome marked this pull request as ready for review May 6, 2026 14:56
@r-tome r-tome requested a review from a team as a code owner May 6, 2026 14:56
@r-tome r-tome requested a review from JimmyVo16 May 6, 2026 14:56
r-tome added 13 commits May 6, 2026 19:47
…izationInviteLinkCommand interface for invite link refresh functionality
…ing encrypted invite keys in invite link requests.
… validate invite link refresh functionality, including scenarios for valid input, missing links, and insufficient permissions.
…oller to handle invite link refresh requests
…InviteLinksController, validating link replacement and domain consistency.
…ository for atomic link replacement, ensuring transactional integrity during updates.
…ionInviteLinkRepository, verifying link replacement and rollback behavior on unique constraint violations.
…shAsync method for atomic link updates, simplifying invite link management.
@r-tome r-tome force-pushed the ac/pm-34389/refresh-invite-link branch from 54b3dd8 to 3f65375 Compare May 6, 2026 18:50
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 6, 2026

@JimmyVo16
Copy link
Copy Markdown
Contributor

Reviewing this will be my top priority today.


private async Task<bool> OrganizationHasInviteLinksAbilityAsync(Guid organizationId)
{
var ability = await applicationCacheService.GetOrganizationAbilityAsync(organizationId);
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.

Just a comment: I think it's safe to use the ability cache since we should be turning on the new ability cache pattern by the time this change goes out. However, in the event that we fall back to the old cache, just know that there will be a 10-minute lag. This probably isn't a big issue, but it can be frustrating during demos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants