From f95ca403eb879a1a03498e4f654e5d865d019d34 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 22 Jul 2026 21:54:03 +0100 Subject: [PATCH 1/5] Redesign manage roles page --- docs/DEVELOPER.md | 21 +- package-lock.json | 15 + package.json | 1 + src/main/frontend/common/api/client.ts | 13 + src/main/frontend/common/api/strategy.ts | 50 +- src/main/frontend/common/api/validation.ts | 39 ++ .../common/components/AppBarButton.tsx | 19 +- .../common/components/PermissionGroups.tsx | 2 +- src/main/frontend/common/components/Tabs.tsx | 63 +++ .../frontend/common/styles/role-strategy.css | 108 ++++ src/main/frontend/common/types/bootstrap.ts | 17 + src/main/frontend/common/types/role.ts | 11 + .../common/utils/permissionSummary.ts | 36 ++ .../manage-roles/ManageRolesPage.spec.tsx | 236 +++++++++ .../frontend/manage-roles/ManageRolesPage.tsx | 416 ++++++++++++++++ .../frontend/manage-roles/MatchingDialog.tsx | 87 ++++ .../frontend/manage-roles/RoleDialog.spec.tsx | 189 +++++++ src/main/frontend/manage-roles/RoleDialog.tsx | 266 ++++++++++ src/main/frontend/manage-roles/index.tsx | 27 + .../PermissionTemplatesPage.tsx | 28 +- src/main/frontend/setupTests.ts | 8 + .../RoleBasedAuthorizationStrategy.java | 45 +- .../rolestrategy/RoleStrategyConfig.java | 81 ++- .../manage-agent-roles.jelly | 71 --- .../manage-global-roles.jelly | 69 --- .../manage-global-roles_fr.properties | 25 - .../manage-project-roles.jelly | 105 ---- .../manage-project-roles_fr.properties | 26 - .../RoleStrategyConfig/manage-roles.jelly | 256 ++-------- .../manage-roles.properties | 2 + .../manage-roles_fr.properties | 26 - src/main/webapp/js/tableManage.js | 462 ------------------ .../hudson/plugins/rolestrategy/ApiTest.java | 38 ++ .../FormSubmissionPermissionsTest.java | 22 +- .../rolestrategy/PatternMatchingApiTest.java | 23 +- .../playwright/ManageRolesUITest.java | 158 ++++++ .../playwright/helpers/UITestHelper.java | 21 + .../playwright/pages/ManageRolesPage.java | 159 ++++++ vite.config.ts | 1 + 39 files changed, 2127 insertions(+), 1115 deletions(-) create mode 100644 src/main/frontend/common/api/validation.ts create mode 100644 src/main/frontend/common/components/Tabs.tsx create mode 100644 src/main/frontend/common/types/role.ts create mode 100644 src/main/frontend/common/utils/permissionSummary.ts create mode 100644 src/main/frontend/manage-roles/ManageRolesPage.spec.tsx create mode 100644 src/main/frontend/manage-roles/ManageRolesPage.tsx create mode 100644 src/main/frontend/manage-roles/MatchingDialog.tsx create mode 100644 src/main/frontend/manage-roles/RoleDialog.spec.tsx create mode 100644 src/main/frontend/manage-roles/RoleDialog.tsx create mode 100644 src/main/frontend/manage-roles/index.tsx delete mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-agent-roles.jelly delete mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-global-roles.jelly delete mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-global-roles_fr.properties delete mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-project-roles.jelly delete mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-project-roles_fr.properties create mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles.properties delete mode 100644 src/main/resources/com/michelin/cio/hudson/plugins/rolestrategy/RoleStrategyConfig/manage-roles_fr.properties delete mode 100644 src/main/webapp/js/tableManage.js create mode 100644 src/test/java/org/jenkinsci/plugins/rolestrategy/playwright/ManageRolesUITest.java create mode 100644 src/test/java/org/jenkinsci/plugins/rolestrategy/playwright/pages/ManageRolesPage.java diff --git a/docs/DEVELOPER.md b/docs/DEVELOPER.md index 9a0d8484..a1933f17 100644 --- a/docs/DEVELOPER.md +++ b/docs/DEVELOPER.md @@ -7,22 +7,11 @@ This page contains links to the information for plugin developers. See the [Developer Documentation](https://www.jenkins.io/doc/developer/). -### Manual Testing of the UI -There are no tests available for the UI part. Mainly the javascript code and the interaction between html elements in the jelly files needs manual testing. -After starting Jenkins locally via `mvn hpi:run` go to the `Manage and Assigne Roles` page. -Verify that following things work on `Manage Roles`: -1. Adding a global, item and agent role -2. Deleting a role by pressing on the red x deletes the role -3. Clicking on the pencil next to a pattern enables edit mode of the pattern (validate for both items and agents) -4. Pressing return key when in the input field of the pattern terminates edit mode and pattern has the new value -5. Pressing escape when in the input field of the pattern terminates edit mode and pattern has the old value -6. Clicking on the pencil next to the pattern disables edit mode of the pattern when it is enabled and pattern has new value -7. Clicking on the pattern opens a dialog box showing the matching items or agents -8. Hovering over the checkboxes properly highlights the row and column and shows a tooltip -9. Tooltips are properly formatted -10. Entering html as rolename is printed as plain text in the field and in the tooltips. -11. After changing the pattern, tooltips are properly updated -12. After pressing Save/Apply and page reload new data is there +### Testing the UI +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. Verify that following things work on `Assign Roles`: 1. Adding a new user to global, item and agent role diff --git a/package-lock.json b/package-lock.json index 078a7684..a0e81a21 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,6 +17,7 @@ "@biomejs/biome": "^2.5.4", "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.3.0", + "@testing-library/user-event": "^14.6.1", "@types/node": "^24.0.0", "@types/react": "^19.1.2", "@types/react-dom": "^19.1.3", @@ -887,6 +888,20 @@ } } }, + "node_modules/@testing-library/user-event": { + "version": "14.6.1", + "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-14.6.1.tgz", + "integrity": "sha512-vq7fv0rnt+QTXgPxr5Hjc210p6YKq2kmdziLgnsZGgLJ9e6VAShx1pACLuRjd/AS/sr7phAR58OIIpf0LlmQNw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12", + "npm": ">=6" + }, + "peerDependencies": { + "@testing-library/dom": ">=7.21.4" + } + }, "node_modules/@tippyjs/react": { "version": "4.2.6", "resolved": "https://registry.npmjs.org/@tippyjs/react/-/react-4.2.6.tgz", diff --git a/package.json b/package.json index 3bcc43aa..cb7dae97 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "@biomejs/biome": "^2.5.4", "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.3.0", + "@testing-library/user-event": "^14.6.1", "@types/node": "^24.0.0", "@types/react": "^19.1.2", "@types/react-dom": "^19.1.3", diff --git a/src/main/frontend/common/api/client.ts b/src/main/frontend/common/api/client.ts index 87040fa6..730240f6 100644 --- a/src/main/frontend/common/api/client.ts +++ b/src/main/frontend/common/api/client.ts @@ -23,6 +23,19 @@ async function ensureOk(response: Response): Promise { return response; } +export async function getJson(url: string, params: Params): Promise { + const query = new URLSearchParams(); + for (const [key, value] of Object.entries(params)) { + if (value === undefined || value === null) continue; + query.append(key, String(value)); + } + const response = await fetch(`${url}?${query}`, { + headers: { Accept: "application/json" }, + }); + await ensureOk(response); + return (await response.json()) as T; +} + export async function postForm(url: string, params: Params): Promise { const body = new URLSearchParams(); for (const [key, value] of Object.entries(params)) { diff --git a/src/main/frontend/common/api/strategy.ts b/src/main/frontend/common/api/strategy.ts index 33f6282e..15edb027 100644 --- a/src/main/frontend/common/api/strategy.ts +++ b/src/main/frontend/common/api/strategy.ts @@ -1,4 +1,15 @@ -import { postForm } from "./client.ts"; +import type { RoleTypeKey } from "../types/role.ts"; +import { getJson, postForm } from "./client.ts"; + +export interface MatchingJobs { + matchingJobs: string[]; + itemCount: number; +} + +export interface MatchingAgents { + matchingAgents: string[]; + agentCount: number; +} /** * Client for the role-strategy REST endpoints exposed on @@ -18,6 +29,25 @@ export interface StrategyClient { overwrite: boolean, ): Promise; removeTemplates(templateNames: string[]): Promise; + /** + * Create or update a role. Pass {@code overwrite: true} when editing an + * existing role; its assignments are kept. A bound {@code template} takes + * precedence over {@code permissionIds} (item roles only). + */ + addRole( + type: RoleTypeKey, + roleName: string, + permissionIds: string[], + overwrite: boolean, + pattern?: string, + template?: string, + ): Promise; + removeRoles(type: RoleTypeKey, roleNames: string[]): Promise; + getMatchingJobs(pattern: string, maxJobs: number): Promise; + getMatchingAgents( + pattern: string, + maxAgents: number, + ): Promise; } export function createStrategyClient(baseUrl: string): StrategyClient { @@ -34,5 +64,23 @@ export function createStrategyClient(baseUrl: string): StrategyClient { postForm(url("removeTemplates"), { names: templateNames.join(","), }), + addRole: (type, roleName, permissionIds, overwrite, pattern, template) => + postForm(url("addRole"), { + type, + roleName, + permissionIds: permissionIds.join(","), + overwrite: String(overwrite), + pattern, + template, + }), + removeRoles: (type, roleNames) => + postForm(url("removeRoles"), { + type, + roleNames: roleNames.join(","), + }), + getMatchingJobs: (pattern, maxJobs) => + getJson(url("getMatchingJobs"), { pattern, maxJobs }), + getMatchingAgents: (pattern, maxAgents) => + getJson(url("getMatchingAgents"), { pattern, maxAgents }), }; } diff --git a/src/main/frontend/common/api/validation.ts b/src/main/frontend/common/api/validation.ts new file mode 100644 index 00000000..f9495a8d --- /dev/null +++ b/src/main/frontend/common/api/validation.ts @@ -0,0 +1,39 @@ +import { ApiError } from "./client.ts"; + +export interface PatternValidation { + ok: boolean; + message?: string; +} + +/** + * Validate a role pattern against the descriptor's {@code checkPattern} + * endpoint, which responds with a Jenkins FormValidation HTML fragment. + */ +export async function checkPattern( + checkPatternUrl: string, + value: string, +): Promise { + const body = new URLSearchParams({ value }); + const headers = crumb.wrap({ + "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8", + }); + const response = await fetch(checkPatternUrl, { + method: "POST", + headers, + body, + }); + if (!response.ok) { + throw new ApiError( + `${response.status} ${response.statusText}`, + response.status, + await response.text(), + ); + } + const html = await response.text(); + const doc = new DOMParser().parseFromString(html, "text/html"); + const error = doc.querySelector(".error"); + if (error) { + return { ok: false, message: error.textContent?.trim() || undefined }; + } + return { ok: true }; +} diff --git a/src/main/frontend/common/components/AppBarButton.tsx b/src/main/frontend/common/components/AppBarButton.tsx index c0ecce5a..56609c9f 100644 --- a/src/main/frontend/common/components/AppBarButton.tsx +++ b/src/main/frontend/common/components/AppBarButton.tsx @@ -3,8 +3,16 @@ import { useEffect } from "react"; /** * Attach a click handler to a Jelly-rendered app-bar button identified by `id`. * The button lives outside the React mount node but in the same document. + * `options.visible` toggles the button, e.g. when only some tabs of a page are + * editable; Jelly still gates whether the button is rendered at all. */ -export function useAppBarButton(id: string, handler: () => void) { +export function useAppBarButton( + id: string, + handler: () => void, + options?: { visible?: boolean }, +) { + const visible = options?.visible ?? true; + useEffect(() => { const node = document.getElementById(id); if (!node) return; @@ -15,4 +23,13 @@ export function useAppBarButton(id: string, handler: () => void) { node.addEventListener("click", onClick); return () => node.removeEventListener("click", onClick); }, [id, handler]); + + useEffect(() => { + const node = document.getElementById(id); + if (!node) return; + node.hidden = !visible; + return () => { + node.hidden = false; + }; + }, [id, visible]); } diff --git a/src/main/frontend/common/components/PermissionGroups.tsx b/src/main/frontend/common/components/PermissionGroups.tsx index 2e41ba0c..84fadb67 100644 --- a/src/main/frontend/common/components/PermissionGroups.tsx +++ b/src/main/frontend/common/components/PermissionGroups.tsx @@ -51,7 +51,7 @@ export function PermissionGroups({ return (