Redesign manage roles page - #765
Open
timja wants to merge 5 commits into
Open
Conversation
Member
Author
|
cc @janfaracik |
There was a problem hiding this comment.
Pull request overview
Redesigns the “Manage Roles” configuration UI from the legacy Jelly/table-based implementation to a React-driven page that boots from server-provided JSON and uses the existing REST endpoints for CRUD operations and pattern matching.
Changes:
- Replace the legacy Manage Roles Jelly/tableManage.js UI with a new React Manage Roles page (tabs, cards, add/edit dialog, matching dialog).
- Add server-side bootstrap JSON for initial render and expand frontend strategy client APIs (add/remove roles, matching endpoints, pattern validation).
- Add/adjust automated coverage (Vitest component tests + Playwright E2E tests) and update permission expectations for pattern-matching endpoints.
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Adds Vite entrypoint for the new manage-roles bundle. |
| src/test/java/org/jenkinsci/plugins/rolestrategy/playwright/pages/ManageRolesPage.java | New Playwright page object for the React Manage Roles UI. |
| src/test/java/org/jenkinsci/plugins/rolestrategy/playwright/ManageRolesUITest.java | New Playwright E2E coverage for Manage Roles flows (list/add/edit/delete/search/matching). |
| src/test/java/org/jenkinsci/plugins/rolestrategy/playwright/helpers/UITestHelper.java | Adds RBAS seeding helper for Manage Roles UI tests. |
| src/test/java/org/jenkinsci/plugins/rolestrategy/PatternMatchingApiTest.java | Updates tests/comments to reflect ITEM/AGENT_ROLES_ADMIN permissions for matching APIs. |
| src/test/java/org/jenkinsci/plugins/rolestrategy/FormSubmissionPermissionsTest.java | Removes rolesSubmit entrypoint test (endpoint removed). |
| src/test/java/com/michelin/cio/hudson/plugins/rolestrategy/ApiTest.java | Adds REST API tests for overwrite preserving assignments + invalid pattern handling. |
| src/main/webapp/js/tableManage.js | Deletes legacy table-based Manage Roles JS implementation. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles.properties | Adds description blurb for the new Manage Roles page. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles.jelly | Switches Manage Roles page to React mount + bundle + bootstrap attributes. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles_fr.properties | Removes legacy French localization for removed strings. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-project-roles.jelly | Removes legacy item roles table fragment. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-project-roles_fr.properties | Removes legacy French localization for removed fragment. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-global-roles.jelly | Removes legacy global roles table fragment. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-global-roles_fr.properties | Removes legacy French localization for removed fragment. |
| src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-agent-roles.jelly | Removes legacy agent roles table fragment. |
| src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig.java | Removes rolesSubmit endpoint and adds Manage Roles bootstrap JSON generator. |
| src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/RoleBasedAuthorizationStrategy.java | Improves addRole validation/overwrite semantics; tightens matching endpoint permissions. |
| src/main/frontend/setupTests.ts | Adds jsdom stubs for <dialog> modal APIs. |
| src/main/frontend/permission-templates/PermissionTemplatesPage.tsx | Refactors permission summary building to shared utility. |
| src/main/frontend/manage-roles/RoleDialog.tsx | New add/edit role dialog component with pattern validation + template binding. |
| src/main/frontend/manage-roles/RoleDialog.spec.tsx | New Vitest coverage for RoleDialog behavior. |
| src/main/frontend/manage-roles/MatchingDialog.tsx | New dialog to display jobs/agents matching a role pattern. |
| src/main/frontend/manage-roles/ManageRolesPage.tsx | New Manage Roles page (tabs/cards/search/filter/dialog flows). |
| src/main/frontend/manage-roles/ManageRolesPage.spec.tsx | New Vitest coverage for ManageRolesPage behavior. |
| src/main/frontend/manage-roles/index.tsx | React mount/bootstrapping for Manage Roles page. |
| src/main/frontend/common/utils/permissionSummary.ts | New shared permission indexing + summary generation helpers. |
| src/main/frontend/common/types/role.ts | New Role + RoleTypeKey frontend types. |
| src/main/frontend/common/types/bootstrap.ts | Extends bootstrap typings for role-type + manage-roles bootstrap payloads. |
| src/main/frontend/common/styles/role-strategy.css | Adds styling for tabs, pattern chips, matching dialog list, implied-permission styling. |
| src/main/frontend/common/components/Tabs.tsx | New accessible tablist component with roving focus. |
| src/main/frontend/common/components/PermissionGroups.tsx | Adds styling hook for implied permissions. |
| src/main/frontend/common/components/AppBarButton.tsx | Adds ability to toggle visibility of Jelly-rendered app-bar buttons. |
| src/main/frontend/common/api/validation.ts | New client for descriptor pattern validation endpoint. |
| src/main/frontend/common/api/strategy.ts | Extends strategy client with add/remove roles + matching endpoints. |
| src/main/frontend/common/api/client.ts | Adds GET JSON helper for query-param endpoints. |
| package.json | Adds @testing-library/user-event dependency. |
| package-lock.json | Locks @testing-library/user-event dependency. |
| docs/DEVELOPER.md | Updates UI testing guidance to reflect new automated coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+36
| export function buildPermissionSummary( | ||
| permissionIds: string[], | ||
| permissionsById: ReadonlyMap<string, PermissionRef>, | ||
| ): string | null { | ||
| if (permissionIds.length === 0) return null; | ||
| return permissionIds | ||
| .map((id) => permissionsById.get(id)) | ||
| .filter((p): p is PermissionRef => !!p) | ||
| .map((p) => `${p.groupTitle}/${p.name}`) | ||
| .sort() | ||
| .join(", "); | ||
| } |
Comment on lines
+1129
to
1132
| checkPerms(ITEM_ROLES_ADMIN); | ||
| List<String> matchingItems = new ArrayList<>(); | ||
| int itemCount = RoleMap.getMatchingItemNames(matchingItems, Pattern.compile(pattern), maxJobs); | ||
| JSONObject responseJson = new JSONObject(); |
Comment on lines
+1156
to
1159
| checkPerms(AGENT_ROLES_ADMIN); | ||
| List<String> matchingAgents = new ArrayList<>(); | ||
| int agentCount = RoleMap.getMatchingAgentNames(matchingAgents, Pattern.compile(pattern), maxAgents); | ||
| JSONObject responseJson = new JSONObject(); |
| The React pages (`Manage Roles`, `Permission Templates`) are covered by Vitest component | ||
| tests (`npm run test`) and Playwright end-to-end tests (`ManageRolesUITest`, | ||
| `PermissionTemplatesUITest`). The remaining jelly/vanilla-JS pages need manual testing. | ||
| After starting Jenkins locally via `mvn hpi:run` go to the `Manage and Assigne Roles` page. |
Member
|
Pushed some minor changes to buttons + tabs, LGTM. |
Member
Author
|
Thanks! |
Member
Author
|
@mawinter69 could you take a look please? |
timja
commented
Jul 30, 2026
| // Only react to keys aimed at the header itself. Key events from focused | ||
| // buttons inside it (badges, actions) bubble up here, and preventDefault | ||
| // would swallow the native click those buttons synthesize on Enter/Space. | ||
| if (e.target !== e.currentTarget) return; |
Member
Author
There was a problem hiding this comment.
accessibility issue I noticed that the view matching jobs couldn't be triggered via keyboard
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO:
* Performance with large sets of roles- I tried with a few 100 roles and it seems fine, can maybe paginate later if needed but it performed well for me.manage-roles.mov
AI Assisted, tested by me
Testing done
Unit, Playwright tests
Manually created global, agent and item roles
Submitter checklist