Remove the Permissions and Scopes columns from the user permissions users list - #6135
Conversation
9584c44 to
7e76668
Compare
7e76668 to
7d5042c
Compare
7d5042c to
805e675
Compare
314c233 to
e83e517
Compare
25cdb20 to
314c57e
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the user-permissions users list to display a per-dimension summary of content scopes (e.g. domain: 3, language: *) instead of a single contentScopesCount, avoiding misleading counts when wildcard ("*") dimension values are present.
Changes:
- Replaced
UserPermissionsUser.contentScopesCountwithcontentScopeSummary: [ContentScopeSummaryByDimension!]!in the CMS API GraphQL schema and resolvers. - Added request-scoped DataLoader logic to compute per-dimension scope summaries efficiently for batched user rows.
- Updated the CMS admin users grid to render the per-dimension summary text instead of a single “X of Y scopes” chip.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| storybook/.storybook/mocks/handlers.ts | Updates Storybook mock schema to reflect contentScopeSummary and the new summary type. |
| storybook/.storybook/mocks/currentUserHandler.ts | Updates mock typing/response shape to provide contentScopeSummary. |
| packages/api/cms-api/src/user-permissions/user.resolver.ts | Switches the GraphQL field resolver from contentScopesCount to contentScopeSummary. |
| packages/api/cms-api/src/user-permissions/user-permissions.service.ts | Exposes filterContentScopesForUser for reuse and removes now-unneeded helper methods. |
| packages/api/cms-api/src/user-permissions/user-content-scopes.resolver.ts | Refactors to use filterContentScopesForUser directly. |
| packages/api/cms-api/src/user-permissions/user-content-scopes-loader.service.ts | Introduces per-dimension summary computation and a DataLoader returning summaries. |
| packages/api/cms-api/src/user-permissions/user-content-scopes-loader.service.spec.ts | Adds tests for summary behavior and batched loading behavior. |
| packages/api/cms-api/src/user-permissions/dto/content-scope.ts | Adds the GraphQL DTO ContentScopeSummaryByDimension. |
| packages/api/cms-api/schema.gql | Updates generated schema to replace contentScopesCount with contentScopeSummary. |
| packages/admin/cms-admin/src/userPermissions/UserGrid.tsx | Renders per-dimension summary strings in the “Scopes” column and simplifies the supporting query. |
| demo/api/schema.gql | Updates demo schema to include the new type/field. |
| .changeset/user-permissions-content-scope-summary.md | Adds a changeset describing the new summary and the field replacement. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
314c57e to
75f5822
Compare
VPS-Obi
left a comment
There was a problem hiding this comment.
A screenshot would have been nice (added one for you). Tested this locally:
- The text should still be rendered in a Chip
- It doesn't work correctly for something like
[{ domain: "main", language: "*" }, { domain: "secondary", language: "de" }]. The result is "Domain: 2, Language: *". IMO it should be "Domain: main, Language: *" and "Domain: secondary, Language: en"
Is this better than showing "X out of Y scopes"? Or is this no longer possible since we can't expand "*" dimensions to the actual total count?
|
Needs human review — permission-related API change, stacked on #6114. Replaces Generated by Claude Code |
75f5822 to
496855c
Compare
496855c to
9dc3d14
Compare
9dc3d14 to
f3250ae
Compare
Both solutions don't really work anymore. Can I just remove this field? It never really worked in all circumstances, is a performance killer and the benefit is not very big. |
IMO yes. @VPS-thodax what do you think? |
f3250ae to
82b4ee0
Compare
82b4ee0 to
cd901d3
Compare
cd901d3 to
a4758bf
Compare
…sers list Rendering these two columns required resolving each user's permissions and content scopes per row - permissionsCount, and a per-dimension content scope summary computed through a dedicated request-scoped DataLoader. Remove the columns and everything behind them: the UserContentScopesLoaderService, the permissionsCount and contentScopeSummary resolve fields, and the ContentScopeSummaryByDimension type. The users list now fetches only name and email.
a4758bf to
9463ab7
Compare
Problem
The user permissions users list rendered a "Permissions" and a "Scopes" column for every user. Both required resolving per-user data for every row — a permission count and a per-dimension content scope summary computed through a dedicated request-scoped DataLoader. That added meaningful complexity (an extra service, DTO, and two resolve fields) and per-row cost for information that is already available in more detail on the user's edit page.
Solution
Remove both columns and everything behind them. The users list now shows only the name and email.
permissionsCountandcontentScopeSummaryfields fromUserPermissionsUser.ContentScopeSummaryByDimensiontype and theUserContentScopesLoaderService.Stacked pull request
This is layer 2 of 4 of a stacked pull request. Review and merge bottom-up (each layer targets the one above it):
main(Support wildcard values for content scope dimensions #6114)🤖 Generated with Claude Code