#36936: feat(roles): add PUT /v1/roles/{roleId} for role update

[hassandotcms]

Proposed Changes

• PUT /api/v1/roles/{roleId} — update role name, key, description, can-grant flags,
  and parent. Replaces DWR RoleAjax#updateRole (Angular portlet migration, Dojo to Angular: Roles and Tools Portlet #36909)
• parentRoleId: null → role becomes root (DWR parity)
• 404 missing role/parent, 403 system/locked, 400 invalid name, 409 duplicate key/name
• 400 on reparent cycles — new guard, legacy had none server-side
• Auth: backend user + roles portlet + CMS admin (same gate as POST /v1/roles; stricter
  than DWR's users-portlet check — intentional). Gate extracted, shared with POST
• Response = ResponseEntityRoleDetailView, same shape as GET /v1/roles/{roleid}
• RoleHelper → @ApplicationScoped CDI; update logic @WrapInTransaction
• Regenerated openapi.yaml

Checklist

• 15 integration tests in RoleResourceIntegrationTest (MainSuite3a): all field updates,
  reparent to other/root/self/cycle, 403/404/409 cases, POST create regression
• No breaking changes: additive endpoint; DWR and POST behavior unchanged

Fixes #36936

[claude]

Claude finished @hassandotcms's task in 3m 1s —— View job

---

Code Review: PUT /v1/roles/{roleId}

I reviewed the diff (RoleHelper, RoleResource, openapi.yaml, tests) against origin/main, and traced the supporting APIs (RoleAPIImpl.save, findRoleHierarchy, ByteBuddyFactory).

Verified correct:

• Cycle guard — findRoleHierarchy(parentRole) walks getParent() up to root and includes the node itself (RoleAPIImpl:108-121), so checking whether roleId is in that set correctly detects "target parent is a descendant of the role." Logic is sound; self-parent is caught first.
• @WrapInTransaction works despite new RoleHelper() — transactions are woven at the bytecode level by ByteBuddy over any class in the com.dotcms/com.dotmarketing whitelist (ByteBuddyFactory:71-75,196-210), independent of how the instance is constructed. No missing-transaction bug.
• Exception mapping matches source — RoleAPIImpl.save throws the com.dotmarketing.business variants of DuplicateRoleKeyException/DuplicateRoleException and RoleNameException (RoleAPIImpl:277-295), which are exactly the types imported and caught in RoleHelper.updateRole. The 409/400 mapping fires correctly.
• System/locked pre-check mirrors save's own guard and surfaces a clean 403 instead of a raw DotStateException.
• Strong test coverage — 15 integration tests including reparent/cycle/self, 403/404/409, non-admin, roles-portlet-without-admin, and full-replace reset.

New Issues

• 🟡 Medium: RoleResource.java:97 / RoleHelper.java:37-49 — RoleHelper is annotated @ApplicationScoped with an @Inject/@VisibleForTesting constructor, but RoleResource still holds it as private final RoleHelper roleHelper = new RoleHelper();. The CDI scope and @Inject are never exercised for this instance — the injected RoleAPI seam only works in unit tests that construct the helper directly, and at runtime it silently falls back to the no-arg APILocator constructor. This works, but the annotations are misleading dead metadata. Either @Inject the helper into RoleResource (consistent with the CDI intent) or drop @ApplicationScoped and keep it a plain helper. Non-blocking.

Existing

• 🟡 Medium: RoleResource.java:296-297 (POST create path) — roleAPI.loadRoleById(roleForm.getParentRoleId()) is dereferenced via parentRole.getId() with no null/existence check, so a POST with a nonexistent parentRoleId yields a 500 (NPE) rather than the clean 404 that the new PUT path returns for the same case. This is pre-existing behavior (the logic only shifted indentation in this diff, not changed), so it does not block this PR — but since PUT now establishes the correct pattern via RoleHelper, it would be worth aligning POST. Fix this →

Overall this is a clean, well-tested addition. No blocking issues — the two items above are non-blocking polish.
· 36936-roles-api-put-role-update