Skip to content

feat(users): Roles tab wired to real endpoints (#36718) - #37082

Queued
AP2300 wants to merge 5 commits into
mainfrom
issue-36718-users-portlet-roles-tab
Queued

feat(users): Roles tab wired to real endpoints (#36718)#37082
AP2300 wants to merge 5 commits into
mainfrom
issue-36718-users-portlet-roles-tab

Conversation

@AP2300

@AP2300 AP2300 commented Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #36718

Depends on #36990 (Profile tab shell). This PR's diff currently includes the shell commits; once #36990 is merged to main, this branch will be rebased onto the new main and the diff will show only the Roles tab changes.

Summary

  • Replaces the placeholder "Coming soon" Roles panel with a shuttle-style picker: Available tree on the left, Granted list on the right, arrows in the middle.
  • Available side renders the full N-level role hierarchy from GET /api/v1/roles?loadChildrenRoles=true + per-node fan-out on non-root roles. Parent nodes get a bulk-select checkbox that toggles every grantable leaf underneath.
  • Granted list is seeded from GET /api/v1/roles/users/{userId} and refreshed on every move; the parent dialog picks the change up via grantedChange and folds it into PUT /api/v1/users as the roles field.
  • Non-grantable branches (Publisher / Legal, isolated organizational nodes) render without checkboxes.
  • Empty roots are pruned from the Available panel once every grantable descendant has been moved to Granted (no dead containers).

Notable non-obvious calls

  • Personal-role filter on save (roleKey === userId, editUsers=false). The backend's `UserResource#processRoles` calls `removeAllRolesFromUser` then loops `addRoleToUser`; the personal role trips the `editUsers` guard and rolls the save back with "Cannot alter users on this role". Filtering it in the outbound payload sidesteps the guard. Proper fix belongs on the backend.
  • Empty roles list is currently a no-op on the backend — `UtilMethods.isSet(roles)` treats `[]` as "don't touch". Not fixed here; documented in the Profile branch PR.

Test plan

  • Open the edit dialog on a user; Roles tab hydrates with the current granted set on the right
  • Move a leaf across via the arrow; save → reopen → the leaf sticks
  • Check a parent → all grantable leaves get selected; move them across; the parent disappears from Available
  • Check Publisher / Legal roots render without a checkbox
  • Save with a personal-role-only user; no "Cannot alter users on this role" error

🤖 Generated with Claude Code

@github-actions github-actions Bot mentioned this pull request Aug 17, 2026
Open
7 tasks
@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Aug 17, 2026
hmoreras
hmoreras reviewed Aug 20, 2026
View reviewed changes
Comment thread core-web/libs/portlets/dot-users/src/lib/dot-users-create/dot-users-create.component.ts Outdated
Comment thread ...dot-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.ts
Comment thread ...dot-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.ts
Comment thread ...t-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.html Outdated
Comment thread core-web/libs/portlets/dot-users/src/lib/dot-users-create/dot-users-create.component.ts Outdated
Comment thread ...dot-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.ts
Comment thread ...dot-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.ts Outdated
Comment thread ...dot-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.ts
Comment thread ...dot-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.ts
Comment thread ...t-users/src/lib/dot-users-create/tabs/dot-users-roles-tab/dot-users-roles-tab.component.html
AP2300 added a commit that referenced this pull request Aug 25, 2026
@AP2300 @claude
refactor(users): PR #37082 review — roles-tab correctness + hygiene +…
28c9a8c 
… spec

Actionable follow-ups from hmoreras' review on the roles branch;
three roles-endpoint threads (L348 / L141 / L443) remain open,
tracked against the profile PR's team-convo chain.

Correctness
- L483 editUsers guard: RoleOption now carries `editUsers` (defaults
  true when the field is missing so we don't over-hide). isGrantableLeaf
  returns false for editUsers=false leaves — no checkbox surfaces, so
  addRoleToUser can't throw `Cannot alter users on this role` and roll
  back the transactional PUT.
- L97 @for track: `track role.id` on the granted list (id is always
  unique; roleKey can be empty for keyless roles → NG0955). Click +
  data-testid still use the grantIdentifier so `revoke()` still finds
  the entry in `selectedGranted`.

UX
- L349 defer role fetch: shell wraps the roles p-tabpanel body in
  `@defer (when activeTab() === 1)` so `loadRoles()` only fires the
  first time the user opens the tab, not on every dialog open.
- L246 loading state: roles panel distinguishes loading from empty
  via `$isLoading`. New i18n key `users.dialog.roles.loading`.

Hygiene (L69)
- Drop `standalone: true`, `$`-prefix all signals + computeds,
  `#`-prefix injections, drop `block` from host class. Kept
  `changeDetection: OnPush` (matches profile branch decision — app
  is zoned).

Docs
- L105 inline HTML comment explaining why `text-*!` is needed on the
  Material Symbols icons (Google's `.material-symbols-rounded 24px`
  ties tailwind on specificity).

Tests
- L72 new roles-tab spec (15 tests → 82% stmts): grantableLeavesByRole
  memoisation via canSelectRole, isPartiallyChecked on multi-level
  parents, availableTree pruning (root drops when last grantable leaf
  moves across), grant/revoke, service error path.

Held for team convo (roleKey vs id endpoint contract):
- L348, L141, L443 — same chain as profile PR L440. Cross-referenced
  on the threads so the two branches move together.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300
AP2300 force-pushed the issue-36718-users-portlet-roles-tab branch from 060e9b1 to 28c9a8c Compare August 25, 2026 16:27
AP2300 added a commit that referenced this pull request Aug 25, 2026
@AP2300 @claude
refactor(users): PR #37082 review — roles-tab correctness + hygiene +…
1f266f0 
… spec

Actionable follow-ups from hmoreras' review on the roles branch;
three roles-endpoint threads (L348 / L141 / L443) remain open,
tracked against the profile PR's team-convo chain.

Correctness
- L483 editUsers guard: RoleOption now carries `editUsers` (defaults
  true when the field is missing so we don't over-hide). isGrantableLeaf
  returns false for editUsers=false leaves — no checkbox surfaces, so
  addRoleToUser can't throw `Cannot alter users on this role` and roll
  back the transactional PUT.
- L97 @for track: `track role.id` on the granted list (id is always
  unique; roleKey can be empty for keyless roles → NG0955). Click +
  data-testid still use the grantIdentifier so `revoke()` still finds
  the entry in `selectedGranted`.

UX
- L349 defer role fetch: shell wraps the roles p-tabpanel body in
  `@defer (when activeTab() === 1)` so `loadRoles()` only fires the
  first time the user opens the tab, not on every dialog open.
- L246 loading state: roles panel distinguishes loading from empty
  via `$isLoading`. New i18n key `users.dialog.roles.loading`.

Hygiene (L69)
- Drop `standalone: true`, `$`-prefix all signals + computeds,
  `#`-prefix injections, drop `block` from host class. Kept
  `changeDetection: OnPush` (matches profile branch decision — app
  is zoned).

Docs
- L105 inline HTML comment explaining why `text-*!` is needed on the
  Material Symbols icons (Google's `.material-symbols-rounded 24px`
  ties tailwind on specificity).

Tests
- L72 new roles-tab spec (15 tests → 82% stmts): grantableLeavesByRole
  memoisation via canSelectRole, isPartiallyChecked on multi-level
  parents, availableTree pruning (root drops when last grantable leaf
  moves across), grant/revoke, service error path.

Held for team convo (roleKey vs id endpoint contract):
- L348, L141, L443 — same chain as profile PR L440. Cross-referenced
  on the threads so the two branches move together.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300
AP2300 force-pushed the issue-36718-users-portlet-roles-tab branch from 28c9a8c to 1f266f0 Compare August 25, 2026 19:08
AP2300 added a commit that referenced this pull request Aug 25, 2026
@AP2300 @claude
refactor(users): PR #37082 review — roles-tab correctness + hygiene +…
56b80b2 
… spec

Actionable follow-ups from hmoreras' review on the roles branch;
three roles-endpoint threads (L348 / L141 / L443) remain open,
tracked against the profile PR's team-convo chain.

Correctness
- L483 editUsers guard: RoleOption now carries `editUsers` (defaults
  true when the field is missing so we don't over-hide). isGrantableLeaf
  returns false for editUsers=false leaves — no checkbox surfaces, so
  addRoleToUser can't throw `Cannot alter users on this role` and roll
  back the transactional PUT.
- L97 @for track: `track role.id` on the granted list (id is always
  unique; roleKey can be empty for keyless roles → NG0955). Click +
  data-testid still use the grantIdentifier so `revoke()` still finds
  the entry in `selectedGranted`.

UX
- L349 defer role fetch: shell wraps the roles p-tabpanel body in
  `@defer (when activeTab() === 1)` so `loadRoles()` only fires the
  first time the user opens the tab, not on every dialog open.
- L246 loading state: roles panel distinguishes loading from empty
  via `$isLoading`. New i18n key `users.dialog.roles.loading`.

Hygiene (L69)
- Drop `standalone: true`, `$`-prefix all signals + computeds,
  `#`-prefix injections, drop `block` from host class. Kept
  `changeDetection: OnPush` (matches profile branch decision — app
  is zoned).

Docs
- L105 inline HTML comment explaining why `text-*!` is needed on the
  Material Symbols icons (Google's `.material-symbols-rounded 24px`
  ties tailwind on specificity).

Tests
- L72 new roles-tab spec (15 tests → 82% stmts): grantableLeavesByRole
  memoisation via canSelectRole, isPartiallyChecked on multi-level
  parents, availableTree pruning (root drops when last grantable leaf
  moves across), grant/revoke, service error path.

Held for team convo (roleKey vs id endpoint contract):
- L348, L141, L443 — same chain as profile PR L440. Cross-referenced
  on the threads so the two branches move together.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300
AP2300 force-pushed the issue-36718-users-portlet-roles-tab branch from 1f266f0 to 56b80b2 Compare August 25, 2026 21:04
AP2300 and others added 4 commits August 25, 2026 17:15
@AP2300 @claude
feat(users): Roles tab (#36718)
8f2f383 
Adds the Roles tab to the Create/Edit User dialog. Dual-panel shuttle
picker (Available tree on the left, Granted list on the right) with
independent filters and Grant/Revoke arrows.

Currently backed by a mock catalog in dot-users-roles.data.ts —
wiring against `GET /api/v1/roles` and the "user's assigned roles"
source of truth is a follow-up decision (see the parent ticket
discussion around DWR vs a small REST addition).

Also swaps the placeholder tabpanel from #36717 for the real
component, and adds the tab-specific i18n keys.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300 @claude
feat(users): wire Roles tab against real endpoints (#36718)
a6db791 
Replaces the mock catalog with real backend data and lifts state up
so the shell's save payload actually carries the granted role list.

Service:
- New `getUserRoles(userIdOrEmail)` — user's currently assigned roles
  via GET /api/v1/roles/users/{userIdOrEmail}.
- New `getAllRoles()` — full system role tree. Uses
  /api/v1/roles?loadChildrenRoles=true for roots + immediate children,
  then recursively fetches /api/v1/roles/{id}?loadChildrenRoles=true
  per non-root discovered so far until a round yields no new roles.
  Bounded by the number of non-root roles in the system. `_search`
  would be simpler but returns SmallRoleView (no parent, no hierarchy)
  and can't be reconstructed into a client-side tree.
- New `DotRoleView` interface + `flattenRoleTree` no longer needed —
  the recursive walker flattens on the fly and sets `parent` on every
  descendant.

Roles tab:
- Deletes the visual-only `dot-users-roles.data.ts` mock catalog.
- `initialGrantedKeys: string[]` input + `grantedChange: string[]`
  output. An effect seeds `granted` from the first non-empty input
  value; later parent mutations don't clobber in-flight edits.
- Drops the synthetic System/Custom bucketing — root roles
  (`System`, `Categories`, `Intranet`, `Publisher / Legal`) are the
  natural top-level nodes and their children indent underneath.
- Bulk-select via parent checkbox: checking a parent selects every
  grantable-leaf descendant in one action; indeterminate state when
  some are selected; unchecking clears them all. Precomputed
  `grantableLeavesByRole` keeps this O(1) per row.
- Grantable = leaf only (parents excluded so their checkbox is a
  bulk-select shortcut, not a role grant). `roleKey` is preferred
  when the backend has one; `role.id` used as fallback so legacy /
  manually-imported keyless roles still work in the shuttle.
- Root rows get extra top spacing and a bold, slightly larger label
  for visual hierarchy.

Shell:
- `loadUserDetail` now forkJoins getUser + getUserRoles. Roles fetch
  catches errors to [] so a failed lookup doesn't block profile
  hydration.
- New signals: `initialGrantedRoleKeys` (down to the tab) and
  `currentRoleKeys` (mirror updated on every grantedChange). Both
  seeded from the initial fetch so save preserves membership even
  when the user never opens the Roles tab.
- `buildPayload` sends `payload.roles` only when the current key
  list is non-empty — an empty array on PUT would wipe membership
  (processRoles calls removeRoles then re-adds), a missing field
  is the safer "leave untouched" signal.

Test coverage: 56 tests + lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300 @claude
fix(users): personal role filter on save, prune empty roots in Available
c8862ba 
- Drop the user's personal role (roleKey === userId) from the save
  payload so `UserResource.processRoles` no longer trips its
  `Cannot alter users on this role` guard on editUsers=false roles.
- Prune parents from the Available tree once every grantable
  descendant has been moved to Granted, so an empty root can no
  longer linger as a dead container. Workflow-only branches with
  zero grantable descendants (Publisher / Legal) are unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300 @claude
refactor(users): PR #37082 review — roles-tab correctness + hygiene +…
76ed444 
… spec

Actionable follow-ups from hmoreras' review on the roles branch;
three roles-endpoint threads (L348 / L141 / L443) remain open,
tracked against the profile PR's team-convo chain.

Correctness
- L483 editUsers guard: RoleOption now carries `editUsers` (defaults
  true when the field is missing so we don't over-hide). isGrantableLeaf
  returns false for editUsers=false leaves — no checkbox surfaces, so
  addRoleToUser can't throw `Cannot alter users on this role` and roll
  back the transactional PUT.
- L97 @for track: `track role.id` on the granted list (id is always
  unique; roleKey can be empty for keyless roles → NG0955). Click +
  data-testid still use the grantIdentifier so `revoke()` still finds
  the entry in `selectedGranted`.

UX
- L349 defer role fetch: shell wraps the roles p-tabpanel body in
  `@defer (when activeTab() === 1)` so `loadRoles()` only fires the
  first time the user opens the tab, not on every dialog open.
- L246 loading state: roles panel distinguishes loading from empty
  via `$isLoading`. New i18n key `users.dialog.roles.loading`.

Hygiene (L69)
- Drop `standalone: true`, `$`-prefix all signals + computeds,
  `#`-prefix injections, drop `block` from host class. Kept
  `changeDetection: OnPush` (matches profile branch decision — app
  is zoned).

Docs
- L105 inline HTML comment explaining why `text-*!` is needed on the
  Material Symbols icons (Google's `.material-symbols-rounded 24px`
  ties tailwind on specificity).

Tests
- L72 new roles-tab spec (15 tests → 82% stmts): grantableLeavesByRole
  memoisation via canSelectRole, isPartiallyChecked on multi-level
  parents, availableTree pruning (root drops when last grantable leaf
  moves across), grant/revoke, service error path.

Held for team convo (roleKey vs id endpoint contract):
- L348, L141, L443 — same chain as profile PR L440. Cross-referenced
  on the threads so the two branches move together.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AP2300
AP2300 force-pushed the issue-36718-users-portlet-roles-tab branch from 56b80b2 to 76ed444 Compare August 25, 2026 21:17
@AP2300
AP2300 enabled auto-merge August 26, 2026 13:48
@AP2300
AP2300 added this pull request to the merge queue Aug 26, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hmoreras hmoreras hmoreras approved these changes

Assignees

No one assigned

Labels

Area : Frontend PR changes Angular/TypeScript frontend code

Projects

dotCMS - Product Planning
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Users portlet: Roles tab (dual-list)

2 participants

@AP2300 @hmoreras