feat(dot-roles): Tools tab in Angular, roles service consolidation, and edit-gate fixes

[hmoreras]

Proposed Changes

role-portlet-progress.mp4

Follow-up work on the Roles (Beta) Angular portlet.

Consume the backends that landed — GET /v1/roles/{roleId}/users (#37070) replaces the previous pair of member-loading calls. The old id-based /rolehierarchyanduserroles fallback returned Role objects rather than users, which is why the Users tab rendered an empty EMAIL column; that column now populates. childCount and userCount (#37071) drive leaf-vs-chevron detection on first paint — no more chevrons that expand into nothing — and unblock the per-row user-count badge the design called for.

Worth flagging: the code's comments predicted /roles/{id}/users would resolve inheritance server-side and collapse the whole flow into one call. It shipped as a direct grants only resource by design, so the ancestor-chain fan-out stays and is now documented as permanent rather than as a stopgap. For the same reason the members table keeps client-side paging — a server page of one ancestor is not a page of the merged union, so [lazy]="true" would be incorrect, not pending.

Migrate the Tools tab to Angular — replaces the Dojo iframe. Scope is deliberately narrower than the JSP it replaces: it lists tool groups and toggles which ones the role gets. Creating, editing and deleting tool groups is not here — those have no v1 endpoints (only RoleAjax DWR) and belong to a Tools portlet of their own. Inherited grants render checked but locked; echoing one back into the full-replace POST would silently promote it to a direct grant. The iframe component and its two JSPs are deleted rather than left behind.

Consolidate the roles surface into DotRolesService — three libs had grown their own implementation of the same /api/v1/roles/** surface, and three TypeScript models of the same backend RoleView. Reads, writes and models now live in one place; method names drop the redundant role (createRole → create). get and search keep their legacy names — the workflow assign components inject this service and those paths are untouched. The full-hierarchy walk did not move into the service: it is one way of composing two endpoints for one UI, so it lives as a pure, HTTP-free function in dot-users, and it now prunes on childCount — most roles below the first level are leaves, which removes the large majority of the request burst the Roles tab produced on open.

Fix edit gates that were stricter than the backend — the Tools tab greyed itself out for CMS Administrator while the legacy portlet allows the edit. RoleHelper applies a different contract per operation: update/delete checks isSystem() || isLocked(), user grants check isEditUsers() alone, and saving layouts places no restriction on the target role at all.

Review fixes

A multi-agent review and hands-on QA surfaced defects that are fixed in this PR:

• Two cross-role races. saveToolGroups and the user grant/remove flows captured the role id before their await and reconciled with it afterwards, so a role switch mid-flight repainted the new role's tab with the old role's data. For tool groups this was worse than cosmetic: the tab derives its next POST payload from toolGroups, so one role's grants could be written onto another. Guarded at both ends, with regression tests verified by neutering each guard and confirming they go red.
• "Could not verify" was being rendered as "not granted". A per-ancestor catchError returned an empty set on failure, so a transient error showed an inherited tool group unchecked — and an admin trusting the grid would create a redundant direct grant. Failures now surface instead of being absorbed.
• childCount regressions. Local tree mutations left it stale, so a parent that just gained its first child rendered as a leaf and the new role was unreachable. And POST /v1/roles answers with Role.toMap(), which carries no childCount, so a freshly created role drew as a folder.
• The tree user-count badge never refreshed after a grant or revoke. It now syncs from the direct-grant count.
• dot-users-list.store.spec.ts still mocked a method deleted in this PR and never mocked DotRolesService, so the roles column ran against the real root service and was effectively untested.

Add / Edit role dialogs

Required markers use the dotFieldRequired directive (which is what makes them red) instead of hand-written text, and dot-field-validation-message is wired in — there was no validation feedback at all. The dialog title moves from an h2 in the body into the dialog header.

Parent selection becomes a p-treeSelect: indentation alone gets ambiguous past two levels. It starts collapsed, hydrates a branch on expand (the backend sends two levels per request), and its filter runs the same deep search the roles tree uses, so a role in an unloaded branch is findable.

Two PrimeNG behaviours needed working around, both from one cause — the component keeps state inside the node objects it is given, and these options come from a computed that hands it new ones. Expansion is mutated onto node.expanded, so branches snapped shut the moment their children loaded. And Tree.getRootNode() returns its cached filteredNodes once the client filter has run, ignoring value, so server results never rendered. Both are handled explicitly, with the reasoning in the code — the second reaches one internal method, which is the most upgrade-fragile line in this PR.

Design review

Empty states now use the shared DotEmptyContainerComponent; the hand-rolled dashed card they replaced exists nowhere else in the product. shield → shield_person. The + on a tree row is primary with a pointer cursor. Untranslated portlet titles no longer print as raw i18n keys.

The user-count badge takes a real grey. text-color-secondary is a PrimeFlex-era class — PrimeFlex is not installed and tailwindcss-primeui does not provide it either, so it compiles to nothing and elements inherit the label's near-black. Verified against the built CSS rather than assumed. 104 more occurrences survive across 35 files and need their own pass.

Checklist

• Tests
• Translations
• Security Implications Contemplated (add notes if applicable)

Additional Info

Verification — 36 projects linted, 35 tested, nx format:check clean, and the dotcms-ui production build green. The dot-roles suite is at 136 tests.

Security note. While tracing the tool-group endpoints, GET /api/v1/roles/layouts turned out to have no authentication gate — the only one of 16 endpoints in RoleResource without a WebResource.InitBuilder block. Confirmed at runtime: it returns 200 with the full tool-group catalog to an unauthenticated caller, while three sibling endpoints return 401 under identical conditions. It predates this work (introduced 2023-05-26, shipped since v23.06) and is filed separately as #37259. This PR does not change that behaviour, but it does put the endpoint on a more exercised path.

Standards note. ANGULAR_STANDARDS.md asks for Signal Forms on new forms, but the standard validation components are Reactive-Forms-bound — dotFieldRequired injects FormGroupDirective and dot-field-validation-message takes an AbstractControl. These dialogs stay on Reactive Forms; adopting Signal Forms would mean dropping both.

Follow-ups not included here

• loadRootRoles(true) → false is now possible thanks to childCount, and would lighten the initial payload considerably on installs with many roles. It changes lazy-load behaviour, so it deserves measuring on its own.
• onNodeExpand marks a node fetched before the load resolves, so a failed lazy-load never retries.
• Creating a role under a parent that is not currently loaded splices into a node that is not there — the role is created but does not appear until a reload.
• The text-color-secondary sweep described above.

🤖 Generated with Claude Code

This PR fixes: #36930

[claude]

Claude finished @hmoreras's task in 4m 54s —— View job

---

Claude Code Review

Reviewed the full diff against origin/main: the shared DotRolesService consolidation, the store races/error-path fixes, the new Tools tab, the tree/hierarchy childCount handling, and the add/edit dialog rework. The race guards (selectedRoleId() === roleId at both ends of saveToolGroups/grant/remove), the failed-vs-empty distinction in loadMembers/loadToolGroups, the childCount propagation in appendChildToParent/removeNodeFromTree, and the #searchToken guard in the add dialog are all correct and well-tested. I verified there are no dangling references to the removed service methods/types and that every i18n key used in the changed templates exists in Language.properties.

One introduced regression worth addressing:

New Issues

• 🟡 Medium: ChangeDetectionStrategy.OnPush was removed from six components that previously declared it — the components now fall back to default (zone) change detection:

  • dot-roles-page/dot-roles-page.component.ts:34
  • dot-roles-page/components/dot-roles-tree/dot-roles-tree.component.ts:56
  • dot-roles-page/components/dot-role-users-tab/dot-role-users-tab.component.ts:61
  • dot-roles-page/components/dot-roles-detail-header/dot-roles-detail-header.component.ts:19
  • dot-roles-add/dot-roles-add.component.ts:38
  • dot-roles-edit/dot-roles-edit.component.ts (present on origin/main, gone here)

  Plus the new dot-role-tools-tab.component.ts ships without it. These are fully signal-driven components (store signals + computed), which is exactly the case OnPush is meant for; the repo's Angular standards call for OnPush on new/edited components (progressive enhancement). Nothing in the PR body explains the removal, so it reads as an accidental strip during the rewrite rather than a deliberate choice. Non-blocking, but it's a real perf/convention regression across the portlet. If a specific component needed default CD (e.g. a PrimeNG internal-state mutation not triggering a re-check), keep that one and document why, and restore OnPush on the rest. Fix this →

Everything else in the diff checks out — no correctness, security, or replay-safety issues found in the changed code. The security note about the unauthenticated GET /api/v1/roles/layouts is correctly filed separately (#37259) and not introduced here.

_{· branch issue-36930-follow-up}