Skip to content

[PM-37237] Move OrganizationsNew into ProfileResponseModel#7627

Merged
eliykat merged 3 commits into
mainfrom
ac/pm-37237/server-move-organizationsnew-into-profileresponse-model
May 18, 2026
Merged

[PM-37237] Move OrganizationsNew into ProfileResponseModel#7627
eliykat merged 3 commits into
mainfrom
ac/pm-37237/server-move-organizationsnew-into-profileresponse-model

Conversation

@eliykat
Copy link
Copy Markdown
Member

@eliykat eliykat commented May 13, 2026

🎟️ Tracking

PM-37237

📔 Objective

OrganizationsNew is the feature-flagged counterpart to ProfileResponseModel.Organizations — both expose the user's organizations, the only difference being scope (Confirmed-only vs. Confirmed+Accepted). The current placement is awkward: OrganizationsNew lives on SyncResponseModel while Organizations lives on the nested ProfileResponseModel, even though they are logically siblings.

This PR colocates them by moving OrganizationsNew onto ProfileResponseModel next to Organizations. The data now flows through the profile object, and GET /accounts/profile also returns it when the feature flag is enabled.

This feature is still behind the pm-34145-policies-in-accepted-state flag and has not been released, so no backwards-compatibility is needed for clients.

@eliykat eliykat added the ai-review Request a Claude code review label May 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR colocates OrganizationsNew with its sibling Organizations on ProfileResponseModel, removing it from SyncResponseModel and extending AccountsController.GetProfile with the same feature-flagged repository call. The change is well-scoped (4 files, +62/-12), the feature flag pm-34145-policies-in-accepted-state is unreleased so no client backwards-compatibility burden exists, and the new constructor parameter defaults to null, leaving existing PutProfile/PutAvatar callers unaffected. Test updates correctly migrate SyncControllerTests assertions to result.Profile.OrganizationsNew, new AccountsControllerTests cover both flag-enabled and flag-disabled branches, no stale references to the removed SyncResponseModel.OrganizationsNew property remain in the codebase, and the XML doc was relocated with the cross-reference correctly updated.

Code Review Details

No findings.

@eliykat eliykat marked this pull request as ready for review May 13, 2026 05:10
@eliykat eliykat requested review from a team as code owners May 13, 2026 05:10
@eliykat eliykat changed the title [PM-37237] refactor: Move OrganizationsNew into ProfileResponseModel [PM-37237] Move OrganizationsNew into ProfileResponseModel May 13, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.98%. Comparing base (1c3dc37) to head (c35f221).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7627   +/-   ##
=======================================
  Coverage   59.98%   59.98%           
=======================================
  Files        2133     2133           
  Lines       93725    93731    +6     
  Branches     8310     8311    +1     
=======================================
+ Hits        56217    56226    +9     
+ Misses      35528    35527    -1     
+ Partials     1980     1978    -2     

☔ 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.

Copy link
Copy Markdown
Contributor

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auth file changes LGTM! (there are merge conflicts on the PR though)

…ve-organizationsnew-into-profileresponse-model
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@gbubemismith gbubemismith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good

@eliykat eliykat merged commit 766c33b into main May 18, 2026
44 checks passed
@eliykat eliykat deleted the ac/pm-37237/server-move-organizationsnew-into-profileresponse-model branch May 18, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants