Skip to content

[PM-38326] Utilize cache for UsePolicies - #8205

Open
JaredScar wants to merge 1 commit into
mainfrom
ac/pm-38326-read-usePolicies-ability-from-cache-instead-of-database
Open

[PM-38326] Utilize cache for UsePolicies#8205
JaredScar wants to merge 1 commit into
mainfrom
ac/pm-38326-read-usePolicies-ability-from-cache-instead-of-database

Conversation

@JaredScar

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

Replaces the per-request Organization DB lookup in PoliciesController.GetByToken and GetMasterPasswordPolicy with a read from the existing OrganizationAbility cache, since the only thing needed (UsePolicies) is already cached there. This avoids an unnecessary full database call on every invite-link click and master password policy check

…e service in PoliciesController

Updated PoliciesController to utilize IOrganizationAbilityCacheService instead of IOrganizationRepository for fetching organization abilities. Adjusted related tests to reflect this change, ensuring proper handling of organization policies based on the new service.
@JaredScar
JaredScar requested a review from a team as a code owner August 13, 2026 16:49
@JaredScar JaredScar added ai-review Request a Claude code review t:tech-debt Change Type - Tech debt labels Aug 13, 2026
@JaredScar
JaredScar requested a review from r-tome August 13, 2026 16:49
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR swaps the per-request IOrganizationRepository.GetByIdAsync lookup in PoliciesController.GetByToken and GetMasterPasswordPolicy for IOrganizationAbilityCacheService.GetOrganizationAbilityAsync, and updates the unit tests to match. I verified behavioral equivalence: OrganizationAbility.UsePolicies is populated on both data paths (the Dapper Organization_ReadAbilityByIdOrganizationAbilityView projection, which selects [UsePolicies], and the EF GetAbilityAsync projection via new OrganizationAbility(e)), the cache service returns OrganizationAbility? so the is not { UsePolicies: true } pattern still yields NotFoundException for a missing organization, and both remaining using directives in the controller are still required by IPolicyRepository and IOrganizationUserRepository. Cache correctness is backed by the existing UpsertOrganizationAbilityAsync / DeleteOrganizationAbilityAsync invalidation call sites plus the FusionCache Redis backplane for cross-instance invalidation, and the consumption pattern matches SyncController, CiphersController, and GroupsController; the test changes mirror the dependency swap one-for-one with no loss of coverage.

Code Review Details

No findings.

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.22%. Comparing base (5358a2f) to head (c4c1745).

Additional details and impacted files
@@            Coverage Diff            @@
##           main    #8205       +/-   ##
=========================================
+ Coverage      0   63.22%   +63.22%     
=========================================
  Files         0     2381     +2381     
  Lines         0   103757   +103757     
  Branches      0     9385     +9385     
=========================================
+ Hits          0    65603    +65603     
- Misses        0    35925    +35925     
- Partials      0     2229     +2229     

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

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 needs-qa t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants