Issue 36930 fe signed

[hmoreras]

Proposed Changes

• change 1
• change 2

Checklist

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

Additional Info

** any additional useful context or info **

Screenshots

Original	Updated
** original screenshot **	** updated screenshot **

Closed unmerged — superseded by #37234.

This PR fixes: #36930

[claude]

Claude finished @hmoreras's task in 3m 38s —— View job

---

Claude Code Review

Reviewed the new dot-roles portlet (store, services, adapters, tree utils, components), the wrapper JSPs, the shared DotHttpErrorManagerService change, and portlet registration.

Overall the PR is in good shape. The security-sensitive spots are handled well: the wrapper JSPs guard the reflected roleId with an anchored UUID Pattern.matches() before interpolating it into inline <script>, use textContent (not innerHTML) for the role name, and the Angular iframes mirror the same UUID check + encodeURIComponent before bypassSecurityTrustResourceUrl. Immutable tree helpers are pure and unit-tested, switchMap/rxMethod cancellation prevents stale-write races, and firstValueFrom(...take(1)) avoids leaked subscriptions. roles-beta is declared in portlet.xml and pinned by id in SerializationHelperTest.

Findings below are all non-blocking polish items — no correctness or security blockers.

New Issues

• 🟡 Medium: core-web/libs/portlets/dot-roles/src/lib/dot-roles-page/components/dot-role-users-tab/dot-role-users-tab.component.ts:194-199 — single-row removal uses the bulk-phrased copy. roles.users.confirm.remove.message is Are you sure you want to remove {0} user(s) from this role? and onRemoveMember passes a user name as {0}, producing "…remove John Doe user(s) from this role?". The header roles.users.confirm.remove.header is also plural (Remove Users) for a single user. Either add a singular key (e.g. roles.users.confirm.remove.single using {0} as a name) or keep the count semantics and pass 1.
  Fix this →

• 🟡 Medium: core-web/libs/portlets/dot-roles/src/lib/dot-roles-edit/dot-roles-edit.component.ts:144-145 — the delete accept label reads roles.action.delete, which resolves to Delete Role (Language.properties:7687), directly contradicting the adjacent comment (Plain "Delete" (not "Delete Role")) and diverging from the tree context-menu delete, which uses the plain Delete key (dot-roles-tree.component.ts:261). Pick one label for both delete confirmations.

• 🟡 Medium: core-web/libs/portlets/dot-roles/src/lib/dot-roles-page/components/dot-roles-tree/dot-roles-tree.component.ts:268-270 — the tree's delete confirm ignores the deleteRole result. The store only routes thrown HTTP errors through httpErrorManager; a 200 with deleted: false returns normally, so this path shows no feedback and silently leaves the node in the tree. The edit dialog explicitly handles that case (dot-roles-edit.component.ts:160-165) — the tree path should too.
  Assumption: the BE can return 200 { deleted:false } (the service doc at dot-roles-portlet.service.ts:276 documents rejections as 403/404/409, which throw). What to verify: whether DELETE /v1/roles/{id} ever responds 200 deleted:false; if it always throws on rejection, this is moot.

Notes (non-blocking)

• dot-roles-tree.component.ts:261 uses this.#messageService.get('Delete') (a bare, non-namespaced key) while the rest of the portlet uses the roles.* namespace introduced in commit 4a1ed8cd. Consider roles.action.delete (once its value is settled per the finding above) for consistency.

· issue-36930-fe-signed