#36939: feat(roles): add DELETE /v1/roles/{roleId} for role deletion

[hassandotcms]

Adds DELETE /v1/roles/{roleId}, replacing DWR RoleAjax#deleteRole for the Angular portlet rewrite (#36909).

Delete cascades, same as legacy RoleAPIImpl.delete: role removed from all users, permissions deleted, layouts detached. Cascading (rather than the 409-on-users the issue suggested) keeps legacy behavior intact; the blast radius is returned as usersAffected (direct assignments) so the FE can warn before calling (#36930). Returns {deleted, roleId, usersAffected}.

Blocks: children → 409, workflow-action Assign To reference → 409 (pre-checked; inside delete() it degrades to a generic error), system/locked → 403. Auth: backend user + roles portlet + CMS admin (legacy gated on users-portlet only; tightened since this is destructive and token-scriptable — DWR path untouched until Dojo removal).

Testing

10 new ITs in RoleResourceIntegrationTest, 26/26 green: happy path, cascade pinned (user/permission/layout rows verified gone), both 409s, 403 system/locked, 404, 401, both auth 403s.

resolved #36939

[claude]

Claude finished @hassandotcms's task in 2m 38s —— View job

---

Code Review — DELETE /v1/roles/{roleId}

Reviewed the delete path end-to-end: RoleHelper.deleteRole, the pre-flight checks, the RoleResource endpoint, the immutable view, and the 10 new ITs against legacy RoleAPIImpl.delete.

This is a clean, well-tested change. Cascade parity is correctly preserved, the auth gate is appropriately tightened, exception→HTTP mapping is all in place (DotConflictExceptionMapper → 409, DoesNotExistException → 404, DotSecurityException → 403), and the audit lines correctly carry name/key/blast-radius since the row is gone post-delete. A couple of non-blocking notes below.

New Issues

• 🟡 Medium: RoleHelper.java:200 & :235 — the workflow-dependency scan runs twice on every successful delete. checkNoDependentWorkflowActions walks all schemes × actions as a pre-flight, then roleAPI.delete() → findDependentWorkflowActions (RoleAPIImpl.java:194) walks them again inside the transaction. For instances with many schemes/actions this doubles that work on the delete path. It's a deliberate trade for the structured 409 (the note at RoleHelper.java:167-169 explains why the inner one can't be relied on), so acceptable — just flagging the cost.

• 🟡 Medium: RoleHelper.java:193-198 — the children pre-check runs outside roleAPI.delete()'s transaction, and delete() re-validates system/locked (throws DotStateException) but does not re-check for children. Assumption: a child role could be created under this role between the pre-flight read and the delete commit. What to verify: whether roleFactory.delete() has an FK/child guard that would fail such a delete — if not, a concurrently-added child would be orphaned pointing at a deleted parent. Low probability and matches legacy behavior, so non-blocking. Fix this →

Notes (non-issues, for confirmation)

• usersAffected uses findUserIdsForRole(role) (direct assignments only), which matches both the AbstractRoleDeletionView doc and the removal set delete() acts on. Test testDeleteRole_withUsersAssigned_cascades pins == 1. ✅
• Endpoint returns ResponseEntityRoleDeletionView directly (not Response.ok(...)), consistent with updateRole; the payload is the standard {entity: {...}} envelope, not a bare {deleted, roleId, usersAffected} as the PR description implies — worth confirming the FE ([Task] Roles and Tools: Angular implementation + backend wiring #36930) expects the envelope. ✅
• New workflow lambda role.getId().equals(action.getNextAssign()) is null-safe, unlike legacy's action.getNextAssign().equals(role.getId()). ✅

No blocking issues. The two Medium items are worth a look but do not block merge.

_{• 36939-roles-api-delete-role}