[Backport v8] Improve performance of contentScopesCount via a request-scoped DataLoader - #6144
Merged
Merged
Conversation
…ader (#6113) ## Problem When the admin queries the users list (`userPermissionsUsers`), the `contentScopesCount` field resolver runs **once per row**. Each invocation calls `getContentScopes(user)`, which recomputes the full list of available content scopes (build labels + `uniqWith` deduplication) from scratch. For a page of users this repeats the same shared computation N times, which becomes very slow when a project configures many content scopes. ## Solution Resolve the available content scopes **once per request** by routing `contentScopesCount` through a request-scoped `DataLoader` (`UserContentScopesLoaderService`), following the existing DataLoader pattern in cms-api (e.g. `AttachedDocumentLoaderService`). All users of a request are batched into a single `getContentScopesForUsers(users)` call that computes the available content scopes only once and then maps each user's scopes. This PR **keeps the existing `lodash.uniqWith` deduplication** unchanged — the only change is *how often* the available content scopes are computed. ## Speed comparison (25 users) Measured by resolving `contentScopesCount` for 25 users (median of 3 runs), comparing the current per-row behavior against the DataLoader: **Typical case — each user resolves to a few content scopes** | Available content scopes | Current (per row) | DataLoader | Speedup | | ------------------------ | ----------------- | ---------- | ------- | | 250 | 359 ms | 15 ms | ~25× | | 500 | 1 377 ms | 55 ms | ~25× | | 1 000 | 5 400 ms | 209 ms | ~26× | | 2 000 | 20 738 ms | 821 ms | ~25× | **Worst case — every user resolves to *all* content scopes** (the per-user `uniqWith`, which this PR does not change, then dominates in both variants): | Available content scopes | Current (per row) | DataLoader | Speedup | | ------------------------ | ----------------- | ---------- | ------- | | 250 | 558 ms | 268 ms | ~2.1× | | 500 | 2 229 ms | 998 ms | ~2.2× | | 1 000 | 9 305 ms | 4 075 ms | ~2.3× | | 2 000 | 35 057 ms | 15 771 ms | ~2.2× | So the DataLoader removes 24 of 25 redundant available-content-scope computations per request; the remaining cost is the per-user work, which is unchanged. ## Example / test `user-permissions.service.spec.ts` covers the batching guarantee (available content scopes computed only once for all users) and that each user's scopes are returned in input order. ## Notes Supersedes #5999 (which additionally replaced the `uniqWith` deduplication with a custom algorithm). This PR isolates just the DataLoader change. https://claude.ai/code/session_01RGR2hkqpqcXL4xk7rGqz7n Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 8c6be74)
VPS-thodax
marked this pull request as ready for review
August 7, 2026 07:44
VPS-thodax
marked this pull request as draft
August 7, 2026 07:45
VPS-thodax
marked this pull request as ready for review
August 7, 2026 07:46
VPS-Obi
approved these changes
Aug 10, 2026
Contributor
|
Needs human review — touches permission-related core logic. Refactors Generated by Claude Code |
fraxachun
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #6113 to
v8.x.x.Note: the cherry-picked test file (
user-permissions.service.spec.ts) was converted fromvitesttojest, since this branch still uses jest for@comet/cms-api. No other changes were needed on top of the cherry-pick.Verification on this branch:
pnpm --filter '@comet/cms-api' run build— passespnpm run lint(inpackages/api/cms-api) — passespnpm exec jest src/user-permissions/user-permissions.service.spec.ts— passesdemo/apiboots and initializesAppModule/GraphQLModulecorrectly with the change (verified up to the expected Postgres connection error, since this sandbox can't pull Docker images to run the full demo)Generated by Claude Code