Skip to content

[PM-32066] - Add Org Ability View#7194

Open
jrmccannon wants to merge 6 commits intomainfrom
jmccannon/ac/pm-32066-add-new-org-ability
Open

[PM-32066] - Add Org Ability View#7194
jrmccannon wants to merge 6 commits intomainfrom
jmccannon/ac/pm-32066-add-new-org-ability

Conversation

@jrmccannon
Copy link
Contributor

@jrmccannon jrmccannon commented Mar 10, 2026

🎟️ Tracking

PM-32066

📔 Objective

This adds a dedicated SQL View for the Organization Ability. This includes a new repository method and integration tests validating it correctly maps those fields. It will be used in future stories.

@jrmccannon jrmccannon added the ai-review Request a Claude code review label Mar 10, 2026
@claude
Copy link
Contributor

claude bot commented Mar 10, 2026

Claude finished @jrmccannon's task in 5m 16s —— View job


I'll analyze this and get back to you.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2026

Logo
Checkmarx One – Scan Summary & Details186d388d-832f-4a17-a2c5-d2b52710d1c5


Fixed Issues (1) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 202

@jrmccannon jrmccannon marked this pull request as ready for review March 10, 2026 20:10
@jrmccannon jrmccannon requested review from a team as code owners March 10, 2026 20:10
@jrmccannon jrmccannon requested a review from JaredScar March 10, 2026 20:10
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.50%. Comparing base (c7c17e8) to head (527e9f1).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7194      +/-   ##
==========================================
+ Coverage   57.39%   61.50%   +4.10%     
==========================================
  Files        2031     2032       +1     
  Lines       89273    89394     +121     
  Branches     7935     7944       +9     
==========================================
+ Hits        51235    54978    +3743     
+ Misses      36195    32491    -3704     
- Partials     1843     1925      +82     

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

@jrmccannon jrmccannon requested a review from JimmyVo16 March 11, 2026 19:21
Copy link
Contributor

@JimmyVo16 JimmyVo16 left a comment

Choose a reason for hiding this comment

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

Some small questions, but overall it looks good.

@jrmccannon jrmccannon requested a review from JimmyVo16 March 12, 2026 15:53
@sonarqubecloud
Copy link

@jrmccannon jrmccannon requested a review from mkincaid-bw March 17, 2026 23:56
@rkac-bw
Copy link
Contributor

rkac-bw commented Mar 19, 2026

Nice work on the view and per-ID sproc. This is a solid foundation.

One question though: is FusionCache part of this change, like the org abilities example in CACHING.md, or is that happening in a follow-up?

The reason I ask is the current bulk Organization_ReadAbilities only runs around ~1K times per hour. If callers move to per-ID lookups without a cache layer in front of it, that likely turns into something closer to 500–700K DB round trips per hour based on current sync and cipher volume. With FusionCache and stampede protection, that should stay much closer to ~10–15K per hour since you only pay once per unique org per TTL window.

@jrmccannon
Copy link
Contributor Author

Nice work on the view and per-ID sproc. This is a solid foundation.

One question though: is FusionCache part of this change, like the org abilities example in CACHING.md, or is that happening in a follow-up?

The reason I ask is the current bulk Organization_ReadAbilities only runs around ~1K times per hour. If callers move to per-ID lookups without a cache layer in front of it, that likely turns into something closer to 500–700K DB round trips per hour based on current sync and cipher volume. With FusionCache and stampede protection, that should stay much closer to ~10–15K per hour since you only pay once per unique org per TTL window.

FusionCache is a later update we will be making.

@rkac-bw
Copy link
Contributor

rkac-bw commented Mar 20, 2026

Nice work on the view and per-ID sproc. This is a solid foundation.
One question though: is FusionCache part of this change, like the org abilities example in CACHING.md, or is that happening in a follow-up?
The reason I ask is the current bulk Organization_ReadAbilities only runs around ~1K times per hour. If callers move to per-ID lookups without a cache layer in front of it, that likely turns into something closer to 500–700K DB round trips per hour based on current sync and cipher volume. With FusionCache and stampede protection, that should stay much closer to ~10–15K per hour since you only pay once per unique org per TTL window.

FusionCache is a later update we will be making.

Thanks! Makes sense. Is the plan to have FusionCache wired up before the caller migration PRs (#7202,
#7214, #7240) start landing? Just want to make sure we want to be aware if there will be a window where per-ID calls are hitting the DB uncached.

@JimmyVo16
Copy link
Contributor

Thanks! Makes sense. Is the plan to have FusionCache wired up before the caller migration PRs (#7202,
#7214, #7240) start landing? Just want to make sure we want to be aware if there will be a window where per-ID calls are hitting the DB uncached.

Yes, we’re going to migrate to FusionCache and eventually move away from Organization_ReadAbilities completely.

The PRs that you posted will call the new interfaces, but the underlying implementations will still use the existing cache. It will not trigger a database call if they’re not in the cache, since Organization_ReadAbilities is called by a scheduler at 10-minute intervals rather than by cache requests, so I don’t think this change will increase the rate of requests.

We have future tickets that will change the implementation to call per request ID and cache it appropriately with FusionCache.

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.

5 participants