Skip to content

[PM-33046] Remove GetManyAbilities repo methods and cache interface#7960

Merged
JimmyVo16 merged 1 commit into
mainfrom
ac/pm-33046/admin-console-repos
Jul 15, 2026
Merged

[PM-33046] Remove GetManyAbilities repo methods and cache interface#7960
JimmyVo16 merged 1 commit into
mainfrom
ac/pm-33046/admin-console-repos

Conversation

@JimmyVo16

@JimmyVo16 JimmyVo16 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🎯 Objective

Part of removing the legacy in-memory application cache system (PM-33046).

Ticket: https://bitwarden.atlassian.net/browse/PM-33046

⚠️ Merge order (important)

This is one atomic removal split into 4 stacked-by-dependency PRs, all based on main. To keep main compiling and runtime-safe, they must merge in this order:

  1. events-di (Data Insights & Reporting) — remove DI consumer in Events
  2. core-services (Core / platform) — delete cache service impls + SharedWeb DI
  3. admin-console-repos (Admin Console) — delete cache interface + GetManyAbilitiesAsync repo methods
  4. dbops-drop-procs (DB Ops) — drop the stored procedures the Dapper code called

👉 This PR is step 3 of 4. Do not merge before the steps above it.

📋 Code Changes

  • src/Core/AdminConsole/AbilitiesCache/IVCurrentInMemoryApplicationCacheService.cs — deleted
  • src/Core/AdminConsole/Repositories/IOrganizationRepository.cs — remove GetManyAbilitiesAsync
  • src/Core/AdminConsole/Repositories/IProviderRepository.cs — remove GetManyAbilitiesAsync
  • src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs
  • src/Infrastructure.Dapper/AdminConsole/Repositories/ProviderRepository.cs
  • src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs
  • src/Infrastructure.EntityFramework/AdminConsole/Repositories/ProviderRepository.cs
  • test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationRepositoryTests.cs

Part of removing the legacy in-memory application cache system.
Removes GetManyAbilitiesAsync from the organization/provider repository
interfaces and their Dapper/EF implementations, deletes the
IVCurrentInMemoryApplicationCacheService interface, and drops the obsolete
EF integration test.
@bre-deploy bre-deploy Bot temporarily deployed to US-DEV Cloud July 13, 2026 19:02 Inactive
@JimmyVo16 JimmyVo16 reopened this Jul 13, 2026
@JimmyVo16 JimmyVo16 self-assigned this Jul 13, 2026
@JimmyVo16 JimmyVo16 added ai-review Request a Claude code review t:tech-debt Change Type - Tech debt labels Jul 13, 2026
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This is step 3 of 4 in the stacked removal of the legacy in-memory application cache (PM-33046). It deletes the IVCurrentInMemoryApplicationCacheService interface, removes GetManyAbilitiesAsync from the organization and provider repository interfaces plus their Dapper and EF Core implementations, and drops the corresponding EF integration test. I verified there are no remaining code references to the deleted interface or method, confirming compilation stays safe when this merges after steps 1 and 2. The stored procedures the Dapper code called (Organization_ReadAbilities, Provider_ReadAbilities) remain in place and are correctly deferred to step 4 (#7961) per the documented merge order.

Code Review Details

No blocking findings.

  • Interface files retain their OrganizationAbility / ProviderAbility imports, which are still required by the remaining GetAbilityAsync members — no dead usings left behind.
  • The unused Bit.Core.Models.Data.Organizations import was correctly removed from the test file alongside the deleted test.
  • The dual-ORM removal is symmetric (both Dapper and EF Core implementations dropped), preserving ORM parity.

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.60%. Comparing base (14fee75) to head (296dd2d).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7960      +/-   ##
==========================================
+ Coverage   66.13%   66.60%   +0.46%     
==========================================
  Files        2256     2276      +20     
  Lines       99392    99326      -66     
  Branches     8983     8978       -5     
==========================================
+ Hits        65732    66154     +422     
+ Misses      31409    30904     -505     
- Partials     2251     2268      +17     

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

@JimmyVo16 JimmyVo16 marked this pull request as ready for review July 13, 2026 19:51
@JimmyVo16 JimmyVo16 requested a review from a team as a code owner July 13, 2026 19:51
@JimmyVo16 JimmyVo16 requested a review from jrmccannon July 13, 2026 19:51
@JimmyVo16 JimmyVo16 merged commit c2d97d5 into main Jul 15, 2026
78 of 83 checks passed
@JimmyVo16 JimmyVo16 deleted the ac/pm-33046/admin-console-repos branch July 15, 2026 16:24
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 t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants