Skip to content

[PM 40199] Add shared folder alias routes to CiphersController#8043

Draft
jengstrom-bw wants to merge 6 commits into
mainfrom
vault/pm-40199/add-shared-folder-alias-routes-to-cipherscontroller
Draft

[PM 40199] Add shared folder alias routes to CiphersController#8043
jengstrom-bw wants to merge 6 commits into
mainfrom
vault/pm-40199/add-shared-folder-alias-routes-to-cipherscontroller

Conversation

@jengstrom-bw

Copy link
Copy Markdown
Contributor

🎟️ Tracking

Jira

📔 Objective

Add shared-folder-named alias routes for the 7 Vault-owned cipher↔collection routes in src/Api/Vault/Controllers/CiphersController.cs; each new action delegates to the existing handler with identical DTOs. Mark each legacy route [Obsolete("Replaced by /shared-folders route; retained for backwards compatibility")] (retained indefinitely — no removal, no Sunset header). No business-logic/DTO/model changes; no server-side flag check (unflagged/additive). Out of scope: AC collection-resource CRUD, public API, /sync, cipher create/edit.

Route map: /collections→/shared-folders, /collections_v2→/shared-folders_v2, /collections-admin→/shared-folders-admin (PUT+POST each), /bulk-collections→/bulk-shared-folders.

Files: src/Api/Vault/Controllers/CiphersController.cs, test/Api.Test/Vault/Controllers/CiphersControllerTests.cs

@jengstrom-bw jengstrom-bw added the t:feature Change Type - Feature Development label Jul 22, 2026
@jengstrom-bw jengstrom-bw added the ai-review-vnext Request a Claude code review using the vNext workflow label Jul 22, 2026
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of /shared-folders* alias routes to CiphersController and their delegation from the now-[Obsolete] /collections* routes. The change is purely additive: each obsolete route delegates to a non-obsolete SharedFolders handler that holds the moved-but-unchanged logic, so authorization checks, DTOs, and behavior are preserved. Verified that no internal callers reference the obsoleted methods (avoiding CS0618 errors under TreatWarningsAsErrors), that the new tests wrap obsolete-route calls in #pragma warning disable CS0618, and that no OpenAPI specs or integration tests reference these routes. Test coverage exercises success, not-found, and delegation paths for every new and aliased route.

Code Review Details

No blocking findings.

Notes:

  • All obsolete routes delegate to non-obsolete SharedFolders targets, so no internal CS0618 warnings are produced despite TreatWarningsAsErrors being enabled.
  • PostSharedFolders* routes are correctly introduced already [Obsolete] ("Use PUT method instead"), preserving the existing POST-deprecation guidance for this resource.
  • Authorization surface is unchanged — new routes reuse the same handlers and checks as the legacy routes.

Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
Comment thread test/Api.Test/Vault/Controllers/CiphersControllerTests.cs Dismissed
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.57%. Comparing base (2500ba2) to head (44ba6e8).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #8043       +/-   ##
===========================================
+ Coverage   14.98%   62.57%   +47.58%     
===========================================
  Files        1392     2299      +907     
  Lines       60466   100093    +39627     
  Branches     4797     9023     +4226     
===========================================
+ Hits         9063    62630    +53567     
+ Misses      51248    35277    -15971     
- Partials      155     2186     +2031     

☔ View full report in Codecov by Harness.
📢 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.

@jengstrom-bw
jengstrom-bw marked this pull request as ready for review July 23, 2026 21:57
@jengstrom-bw
jengstrom-bw requested a review from a team as a code owner July 23, 2026 21:57
@jengstrom-bw
jengstrom-bw requested a review from nikwithak July 23, 2026 21:57
@jengstrom-bw
jengstrom-bw marked this pull request as draft July 23, 2026 22:22
@jengstrom-bw
jengstrom-bw removed the request for review from nikwithak July 23, 2026 22:23
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 t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant