From 96a781967eca36524b1d07de62f7918ced9cc496 Mon Sep 17 00:00:00 2001 From: Priscila Moneo Date: Mon, 4 May 2026 13:07:43 -0300 Subject: [PATCH 1/4] feat: move admin access group grid to mui --- .../__tests__/admin-access-actions.test.js | 131 ++++++ src/actions/admin-access-actions.js | 102 ++--- src/components/forms/admin-access-form.js | 228 +++++----- src/i18n/en.json | 4 +- src/layouts/admin-access-layout.js | 67 ++- .../__tests__/admin-access-list-page.test.js | 415 ++++++++++++++++++ .../admin_access/admin-access-form-popup.js | 82 ++++ .../admin_access/admin-access-list-page.js | 339 ++++++++------ .../admin_access/edit-admin-access-page.js | 89 ---- .../admin-access-list-reducer.test.js | 49 +++ .../admin_access/admin-access-list-reducer.js | 44 +- 11 files changed, 1110 insertions(+), 440 deletions(-) create mode 100644 src/actions/__tests__/admin-access-actions.test.js create mode 100644 src/pages/admin_access/__tests__/admin-access-list-page.test.js create mode 100644 src/pages/admin_access/admin-access-form-popup.js delete mode 100644 src/pages/admin_access/edit-admin-access-page.js create mode 100644 src/reducers/admin_access/__tests__/admin-access-list-reducer.test.js diff --git a/src/actions/__tests__/admin-access-actions.test.js b/src/actions/__tests__/admin-access-actions.test.js new file mode 100644 index 000000000..e7d000167 --- /dev/null +++ b/src/actions/__tests__/admin-access-actions.test.js @@ -0,0 +1,131 @@ +/** + * @jest-environment jsdom + */ +import configureStore from "redux-mock-store"; +import thunk from "redux-thunk"; +import flushPromises from "flush-promises"; +import { + postRequest, + putRequest +} from "openstack-uicore-foundation/lib/utils/actions"; +import { saveAdminAccess } from "../admin-access-actions"; +import * as methods from "../../utils/methods"; + +jest.mock("openstack-uicore-foundation/lib/utils/actions", () => ({ + __esModule: true, + ...jest.requireActual("openstack-uicore-foundation/lib/utils/actions"), + postRequest: jest.fn(), + putRequest: jest.fn() +})); + +const requestMock = + (requestActionCreator, receiveActionCreator) => () => (dispatch) => { + if (requestActionCreator && typeof requestActionCreator === "function") { + dispatch(requestActionCreator({})); + } + return new Promise((resolve) => { + if (typeof receiveActionCreator === "function") { + dispatch(receiveActionCreator({ response: { id: 1 } })); + } else { + dispatch(receiveActionCreator); + } + resolve({ response: { id: 1 } }); + }); + }; + +const rejectMock = () => () => () => Promise.reject(new Error("API error")); + +describe("saveAdminAccess", () => { + const middlewares = [thunk]; + const mockStore = configureStore(middlewares); + + beforeEach(() => { + jest.spyOn(methods, "getAccessTokenSafely").mockResolvedValue("TOKEN"); + postRequest.mockImplementation(requestMock); + putRequest.mockImplementation(requestMock); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe("create path (entity has no id)", () => { + it("returns a Promise", async () => { + const store = mockStore({}); + const result = store.dispatch( + saveAdminAccess({ title: "Group A", members: [], summits: [] }) + ); + expect(result).toBeInstanceOf(Promise); + await expect(result).resolves.toBeUndefined(); + }); + + it("dispatches ADMIN_ACCESS_ADDED then STOP_LOADING on success", async () => { + const store = mockStore({}); + store.dispatch( + saveAdminAccess({ title: "Group A", members: [], summits: [] }) + ); + await flushPromises(); + + const actionTypes = store.getActions().map((a) => a.type); + expect(actionTypes).toContain("ADMIN_ACCESS_ADDED"); + expect(actionTypes).toContain("STOP_LOADING"); + expect(actionTypes.indexOf("STOP_LOADING")).toBeGreaterThan( + actionTypes.indexOf("ADMIN_ACCESS_ADDED") + ); + }); + + it("still dispatches STOP_LOADING when postRequest rejects", async () => { + postRequest.mockImplementation(rejectMock); + const store = mockStore({}); + await store + .dispatch( + saveAdminAccess({ title: "Group A", members: [], summits: [] }) + ) + .catch(() => {}); + await flushPromises(); + + const actionTypes = store.getActions().map((a) => a.type); + expect(actionTypes).toContain("STOP_LOADING"); + }); + }); + + describe("update path (entity has id)", () => { + it("returns a Promise", async () => { + const store = mockStore({}); + const result = store.dispatch( + saveAdminAccess({ id: 1, title: "Group A", members: [], summits: [] }) + ); + expect(result).toBeInstanceOf(Promise); + await expect(result).resolves.toBeUndefined(); + }); + + it("dispatches ADMIN_ACCESS_UPDATED then STOP_LOADING on success", async () => { + const store = mockStore({}); + store.dispatch( + saveAdminAccess({ id: 1, title: "Group A", members: [], summits: [] }) + ); + await flushPromises(); + + const actionTypes = store.getActions().map((a) => a.type); + expect(actionTypes).toContain("ADMIN_ACCESS_UPDATED"); + expect(actionTypes).toContain("STOP_LOADING"); + expect(actionTypes.indexOf("STOP_LOADING")).toBeGreaterThan( + actionTypes.indexOf("ADMIN_ACCESS_UPDATED") + ); + }); + + it("still dispatches STOP_LOADING when putRequest rejects", async () => { + putRequest.mockImplementation(rejectMock); + const store = mockStore({}); + await store + .dispatch( + saveAdminAccess({ id: 1, title: "Group A", members: [], summits: [] }) + ) + .catch(() => {}); + await flushPromises(); + + const actionTypes = store.getActions().map((a) => a.type); + expect(actionTypes).toContain("STOP_LOADING"); + }); + }); +}); diff --git a/src/actions/admin-access-actions.js b/src/actions/admin-access-actions.js index ec960e643..36c18b0ec 100644 --- a/src/actions/admin-access-actions.js +++ b/src/actions/admin-access-actions.js @@ -19,12 +19,9 @@ import { createAction, stopLoading, startLoading, - showMessage, - showSuccessMessage, - authErrorHandler, escapeFilterValue } from "openstack-uicore-foundation/lib/utils/actions"; -import history from "../history"; +import { snackbarErrorHandler, snackbarSuccessHandler } from "./base-actions"; import { getAccessTokenSafely } from "../utils/methods"; import { DEFAULT_PER_PAGE } from "../utils/constants"; @@ -94,9 +91,9 @@ export const getAdminAccesses = createAction(REQUEST_ADMIN_ACCESSES), createAction(RECEIVE_ADMIN_ACCESSES), `${window.API_BASE_URL}/api/v1/summit-administrator-groups`, - authErrorHandler, - { order, orderDir, term } - )(params)(dispatch).then(() => { + snackbarErrorHandler, + { order, orderDir, term, page, perPage } + )(params)(dispatch).finally(() => { dispatch(stopLoading()); }); }; @@ -118,8 +115,8 @@ export const getAdminAccess = (adminAccessId) => async (dispatch) => { null, createAction(RECEIVE_ADMIN_ACCESS), `${window.API_BASE_URL}/api/v1/summit-administrator-groups/${adminAccessId}`, - authErrorHandler - )(params)(dispatch).then(() => { + snackbarErrorHandler + )(params)(dispatch).finally(() => { dispatch(stopLoading()); }); }; @@ -128,52 +125,55 @@ export const resetAdminAccessForm = () => (dispatch) => { dispatch(createAction(RESET_ADMIN_ACCESS_FORM)({})); }; -export const saveAdminAccess = - (entity, noAlert = false) => - async (dispatch) => { - const accessToken = await getAccessTokenSafely(); +export const saveAdminAccess = (entity) => async (dispatch) => { + const accessToken = await getAccessTokenSafely(); - dispatch(startLoading()); + dispatch(startLoading()); - const normalizedEntity = normalizeEntity(entity); - const params = { access_token: accessToken }; - - if (entity.id) { - putRequest( - createAction(UPDATE_ADMIN_ACCESS), - createAction(ADMIN_ACCESS_UPDATED), - `${window.API_BASE_URL}/api/v1/summit-administrator-groups/${entity.id}`, - normalizedEntity, - authErrorHandler, - entity - )(params)(dispatch).then(() => { - if (!noAlert) - dispatch(showSuccessMessage(T.translate("admin_access.saved"))); - else dispatch(stopLoading()); - }); - } else { - const successMessage = { - title: T.translate("general.done"), - html: T.translate("admin_access.created"), - type: "success" - }; - - postRequest( - createAction(UPDATE_ADMIN_ACCESS), - createAction(ADMIN_ACCESS_ADDED), - `${window.API_BASE_URL}/api/v1/summit-administrator-groups`, - normalizedEntity, - authErrorHandler, - entity - )(params)(dispatch).then((payload) => { + const normalizedEntity = normalizeEntity(entity); + const params = { access_token: accessToken }; + + if (entity.id) { + return putRequest( + createAction(UPDATE_ADMIN_ACCESS), + createAction(ADMIN_ACCESS_UPDATED), + `${window.API_BASE_URL}/api/v1/summit-administrator-groups/${entity.id}`, + normalizedEntity, + snackbarErrorHandler, + entity + )(params)(dispatch) + .then(() => { dispatch( - showMessage(successMessage, () => { - history.push(`/app/admin-access/${payload.response.id}`); + snackbarSuccessHandler({ + title: T.translate("general.success"), + html: T.translate("admin_access.saved") }) ); + }) + .finally(() => { + dispatch(stopLoading()); }); - } - }; + } + return postRequest( + createAction(UPDATE_ADMIN_ACCESS), + createAction(ADMIN_ACCESS_ADDED), + `${window.API_BASE_URL}/api/v1/summit-administrator-groups`, + normalizedEntity, + snackbarErrorHandler, + entity + )(params)(dispatch) + .then(() => { + dispatch( + snackbarSuccessHandler({ + title: T.translate("general.success"), + html: T.translate("admin_access.created") + }) + ); + }) + .finally(() => { + dispatch(stopLoading()); + }); +}; export const deleteAdminAccess = (adminAccessId) => async (dispatch) => { const accessToken = await getAccessTokenSafely(); @@ -187,8 +187,8 @@ export const deleteAdminAccess = (adminAccessId) => async (dispatch) => { createAction(ADMIN_ACCESS_DELETED)({ adminAccessId }), `${window.API_BASE_URL}/api/v1/summit-administrator-groups/${adminAccessId}`, null, - authErrorHandler - )(params)(dispatch).then(() => { + snackbarErrorHandler + )(params)(dispatch).finally(() => { dispatch(stopLoading()); }); }; diff --git a/src/components/forms/admin-access-form.js b/src/components/forms/admin-access-form.js index 6207d00d1..1a0b44b59 100644 --- a/src/components/forms/admin-access-form.js +++ b/src/components/forms/admin-access-form.js @@ -9,124 +9,138 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - **/ + * */ -import React from "react"; +import React, { useEffect } from "react"; +import { FormikProvider, useFormik } from "formik"; +import * as yup from "yup"; import T from "i18n-react/dist/i18n-react"; -import "awesome-bootstrap-checkbox/awesome-bootstrap-checkbox.css"; -import Input from "openstack-uicore-foundation/lib/components/inputs/text-input" -import MemberInput from "openstack-uicore-foundation/lib/components/inputs/member-input" +import Box from "@mui/material/Box"; +import Button from "@mui/material/Button"; +import Grid2 from "@mui/material/Grid2"; +import Typography from "@mui/material/Typography"; +import MemberInput from "openstack-uicore-foundation/lib/components/inputs/member-input"; import SummitInput from "openstack-uicore-foundation/lib/components/inputs/summit-input"; -import { - scrollToError, - hasErrors, - shallowEqual, - isEmpty -} from "../../utils/methods"; - -class AdminAccessForm extends React.Component { - constructor(props) { - super(props); - - this.state = { - entity: { ...props.entity }, - errors: props.errors - }; - - this.handleChange = this.handleChange.bind(this); - this.handleSubmit = this.handleSubmit.bind(this); - } - - componentDidUpdate(prevProps, prevState, snapshot) { - const state = {}; - scrollToError(this.props.errors); - - if (!shallowEqual(prevProps.entity, this.props.entity)) { - state.entity = { ...this.props.entity }; - state.errors = {}; +import MuiFormikTextField from "../mui/formik-inputs/mui-formik-textfield"; +import { requiredStringValidation } from "../../utils/yup"; + +const validationSchema = yup.object().shape({ + title: requiredStringValidation(), + members: yup.array().min(1, T.translate("validation.required")), + summits: yup.array().min(1, T.translate("validation.required")) +}); + +const AdminAccessForm = ({ + entity, + errors: serverErrors, + onSubmit, + isSaving = false +}) => { + const formik = useFormik({ + enableReinitialize: true, + initialValues: { + id: entity.id || 0, + title: entity.title || "", + members: entity.members || [], + summits: entity.summits || [] + }, + validationSchema, + onSubmit: (values) => onSubmit(values) + }); + + useEffect(() => { + if (!serverErrors || Object.keys(serverErrors).length === 0) { + formik.setErrors({}); + formik.setTouched({}); + return; } - - if (!shallowEqual(prevProps.errors, this.props.errors)) { - state.errors = { ...this.props.errors }; - } - - if (!isEmpty(state)) { - this.setState({ ...this.state, ...state }); - } - } - - handleChange(ev) { - const entity = { ...this.state.entity }; - const errors = { ...this.state.errors }; - const { value, id } = ev.target; - - errors[id] = ""; - entity[id] = value; - - this.setState({ entity: entity, errors: errors }); - } - - handleSubmit(ev) { - const { entity } = this.state; - ev.preventDefault(); - - this.props.onSubmit(entity); - } - - render() { - const { entity, errors } = this.state; - - return ( -
- -
-
- - [k, true])) + ); + }, [serverErrors]); + + return ( + + + + + + -
-
- + + + + + {T.translate("admin_access.members")} * + { - return member.hasOwnProperty("email") + value={formik.values.members} + getOptionLabel={(member) => + "email" in member ? `${member.first_name} ${member.last_name} (${member.email})` - : `${member.first_name} ${member.last_name} (${member.id})`; - }} - onChange={this.handleChange} - multi={true} + : `${member.first_name} ${member.last_name} (${member.id})` + } + onChange={(ev) => + formik.setFieldValue("members", ev.target.value) + } + onBlur={() => formik.setFieldTouched("members", true)} + menuPortalTarget={document.body} + menuPosition="fixed" + styles={{ menuPortal: (base) => ({ ...base, zIndex: 1400 }) }} + multi /> -
-
- + {formik.touched.members && formik.errors.members && ( + + {formik.errors.members} + + )} + + + + + {T.translate("admin_access.summits")} * + + formik.setFieldValue("summits", ev.target.value) + } + onBlur={() => formik.setFieldTouched("summits", true)} + menuPortalTarget={document.body} + menuPosition="fixed" + styles={{ menuPortal: (base) => ({ ...base, zIndex: 1400 }) }} + multi /> -
-
-
-
- -
-
-
- ); - } -} + {formik.touched.summits && formik.errors.summits && ( + + {formik.errors.summits} + + )} + + + + + + + + + ); +}; export default AdminAccessForm; diff --git a/src/i18n/en.json b/src/i18n/en.json index eda7913d2..74023cd0a 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -3363,11 +3363,11 @@ "title": "Title", "members": "Members", "summits": "Events", - "delete_warning": "Are you sure you want to delete group ", + "delete_warning": "Please verify you want to delete group ", "saved": "Admin Access Group saved successfully.", "created": "Admin Access Group created successfully.", "placeholders": { - "search": "Search groups by name", + "search": "Search groups by title", "select_summits": "Select Events", "select_members": "Select Members" } diff --git a/src/layouts/admin-access-layout.js b/src/layouts/admin-access-layout.js index 738e36794..8e61a069e 100644 --- a/src/layouts/admin-access-layout.js +++ b/src/layouts/admin-access-layout.js @@ -9,52 +9,41 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - **/ + * */ import React from "react"; import { Switch, Route, Redirect } from "react-router-dom"; import T from "i18n-react/dist/i18n-react"; -import { connect } from "react-redux"; import { Breadcrumb } from "react-breadcrumbs"; import Restrict from "../routes/restrict"; import AdminAccessListPage from "../pages/admin_access/admin-access-list-page"; -import EditAdminAccessPage from "../pages/admin_access/edit-admin-access-page"; -class AdminAccessLayout extends React.Component { - render() { - const { match } = this.props; +const AdminAccessLayout = ({ match }) => ( +
+ - return ( -
- + + + + + + +
+); - - - - - - -
- ); - } -} - -export default Restrict(connect(null, {})(AdminAccessLayout), "admin-access"); +export default Restrict(AdminAccessLayout, "admin-access"); diff --git a/src/pages/admin_access/__tests__/admin-access-list-page.test.js b/src/pages/admin_access/__tests__/admin-access-list-page.test.js new file mode 100644 index 000000000..154030f90 --- /dev/null +++ b/src/pages/admin_access/__tests__/admin-access-list-page.test.js @@ -0,0 +1,415 @@ +import React from "react"; +import { Provider } from "react-redux"; +import { MemoryRouter, Route } from "react-router-dom"; +import { + act, + render, + screen, + fireEvent, + waitFor +} from "@testing-library/react"; +import "@testing-library/jest-dom"; +import flushPromises from "flush-promises"; +import configureStore from "redux-mock-store"; +import thunk from "redux-thunk"; +import AdminAccessListPage from "../admin-access-list-page"; + +const mockGetAdminAccesses = jest.fn(() => () => Promise.resolve()); +const mockDeleteAdminAccess = jest.fn(() => () => Promise.resolve()); +const mockGetAdminAccess = jest.fn(() => () => Promise.resolve()); +const mockResetAdminAccessForm = jest.fn(() => () => Promise.resolve()); +const mockSaveAdminAccess = jest.fn(() => () => Promise.resolve()); + +jest.mock("../../../actions/admin-access-actions", () => ({ + getAdminAccesses: (...args) => mockGetAdminAccesses(...args), + deleteAdminAccess: (...args) => mockDeleteAdminAccess(...args), + getAdminAccess: (...args) => mockGetAdminAccess(...args), + resetAdminAccessForm: (...args) => mockResetAdminAccessForm(...args), + saveAdminAccess: (...args) => mockSaveAdminAccess(...args) +})); + +jest.mock("openstack-uicore-foundation/lib/components/mui/table", () => { + const MockMuiTable = ({ + data = [], + onEdit, + onDelete, + onPerPageChange, + onSort + }) => ( +
+ + + {data.map((row) => ( +
+ {row.title} + + +
+ ))} +
+ ); + + return MockMuiTable; +}); + +jest.mock( + "openstack-uicore-foundation/lib/components/mui/search-input", + () => ({ + __esModule: true, + default: ({ onSearch }) => ( + onSearch(e.target.value)} /> + ) + }) +); + +jest.mock("../../../components/forms/admin-access-form", () => { + const MockAdminAccessForm = ({ onSubmit, isSaving }) => ( +
+ +
+ ); + + return MockAdminAccessForm; +}); + +jest.mock("i18n-react/dist/i18n-react", () => ({ + __esModule: true, + default: { translate: (key) => key } +})); + +const middlewares = [thunk]; +const mockStore = configureStore(middlewares); + +const baseListState = { + admin_accesses: [ + { id: 1, title: "Group A", members: "John Doe", summits: "Summit One" } + ], + totalAdminAccesses: 1, + term: "", + order: "id", + orderDir: 1, + currentPage: 1, + lastPage: 1, + perPage: 10 +}; + +const baseFormState = { + entity: { id: 0, title: "", members: [], summits: [] }, + errors: {} +}; + +const renderPage = (path = "/app/admin-access", formState = baseFormState) => { + const store = mockStore({ + adminAccessListState: baseListState, + adminAccessState: formState + }); + + render( + + + + + + ); + + return store; +}; + +describe("AdminAccessListPage", () => { + beforeEach(() => { + jest.clearAllMocks(); + mockGetAdminAccesses.mockReturnValue(() => Promise.resolve()); + mockDeleteAdminAccess.mockReturnValue(() => Promise.resolve()); + mockGetAdminAccess.mockReturnValue(() => Promise.resolve()); + mockResetAdminAccessForm.mockReturnValue(() => Promise.resolve()); + mockSaveAdminAccess.mockReturnValue(() => Promise.resolve()); + }); + + describe("rendering and navigation", () => { + it("renders grid and opens popup for new item", async () => { + renderPage("/app/admin-access"); + + expect(screen.getByTestId("mui-table")).toBeInTheDocument(); + expect(screen.queryByTestId("admin-access-form")).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /create|add/i })); + + await waitFor(() => { + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument(); + }); + }); + + it("opens popup when route has an access id", async () => { + const formState = { + entity: { id: 1, title: "Group A", members: [], summits: [] }, + errors: {} + }; + + renderPage("/app/admin-access/1", formState); + + await waitFor(() => { + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument(); + }); + }); + + it("requests data with selected rows per page", async () => { + renderPage("/app/admin-access"); + + fireEvent.click(screen.getByRole("button", { name: "per-page-25" })); + + await waitFor(() => { + expect(mockGetAdminAccesses).toHaveBeenCalledWith("", 1, 25, "id", 1); + }); + }); + + it("requests data with new sort column and direction", async () => { + renderPage("/app/admin-access"); + + fireEvent.click(screen.getByRole("button", { name: "sort-col" })); + + await waitFor(() => { + expect(mockGetAdminAccesses).toHaveBeenCalledWith( + "", + 1, + 10, + "title", + -1 + ); + }); + }); + + it("requests data with search term and resets to page 1", async () => { + renderPage("/app/admin-access"); + + fireEvent.change(screen.getByPlaceholderText("search"), { + target: { value: "admins" } + }); + + await waitFor(() => { + expect(mockGetAdminAccesses).toHaveBeenCalledWith( + "admins", + 1, + 10, + "id", + 1 + ); + }); + }); + }); + + describe("save guard", () => { + it("disables the X button and submit while save is in flight", async () => { + mockSaveAdminAccess.mockReturnValue(() => new Promise(() => {})); + + renderPage("/app/admin-access/new"); + await waitFor(() => + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() + ); + + fireEvent.click(screen.getByRole("button", { name: "submit-form" })); + + await waitFor(() => { + expect( + screen.getByTestId("CloseIcon").closest("button") + ).toBeDisabled(); + expect( + screen.getByRole("button", { name: "submit-form" }) + ).toBeDisabled(); + }); + }); + + it("keeps dialog open, re-enables buttons, and does not reload list when save rejects", async () => { + mockSaveAdminAccess.mockReturnValue(() => + Promise.reject(new Error("save failed")) + ); + + renderPage("/app/admin-access/new"); + await waitFor(() => + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() + ); + + const callsBefore = mockGetAdminAccesses.mock.calls.length; + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "submit-form" })); + await flushPromises(); + }); + + await waitFor(() => { + expect( + screen.getByTestId("CloseIcon").closest("button") + ).not.toBeDisabled(); + }); + + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument(); + expect(mockGetAdminAccesses.mock.calls.length).toBe(callsBefore); + }); + }); + + describe("route-driven open/close", () => { + it("does not open dialog and navigates back when getAdminAccess rejects", async () => { + mockGetAdminAccess.mockReturnValue(() => + Promise.reject(new Error("not found")) + ); + + renderPage("/app/admin-access/1"); + + await act(async () => { + await flushPromises(); + }); + + expect(screen.queryByTestId("admin-access-form")).not.toBeInTheDocument(); + }); + + it("closes the dialog when the URL changes from a detail route to the list route", async () => { + const store = mockStore({ + adminAccessListState: baseListState, + adminAccessState: { + entity: { id: 1, title: "Group A", members: [], summits: [] }, + errors: {} + } + }); + + let capturedHistory; + + render( + + + <> + + { + capturedHistory = history; + return null; + }} + /> + + + + ); + + await waitFor(() => + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() + ); + + await act(async () => { + capturedHistory.push("/app/admin-access"); + await flushPromises(); + }); + + await waitFor(() => + expect( + screen.queryByTestId("admin-access-form") + ).not.toBeInTheDocument() + ); + }); + }); + + describe("list management", () => { + it("reloads the list after a successful save", async () => { + renderPage("/app/admin-access/new"); + await waitFor(() => + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() + ); + + const callsBefore = mockGetAdminAccesses.mock.calls.length; + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "submit-form" })); + await flushPromises(); + }); + + expect(mockGetAdminAccesses.mock.calls.length).toBeGreaterThan( + callsBefore + ); + }); + + it("reloads the list after a successful delete", async () => { + renderPage("/app/admin-access"); + + const callsBefore = mockGetAdminAccesses.mock.calls.length; + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "delete" })); + await flushPromises(); + }); + + expect(mockGetAdminAccesses.mock.calls.length).toBeGreaterThan( + callsBefore + ); + }); + + it("re-syncs the list after a failed delete", async () => { + mockDeleteAdminAccess.mockReturnValue(() => + Promise.reject(new Error("delete failed")) + ); + + renderPage("/app/admin-access"); + + const callsBefore = mockGetAdminAccesses.mock.calls.length; + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "delete" })); + await flushPromises(); + }); + + // .finally() fires even on rejection + expect(mockGetAdminAccesses.mock.calls.length).toBeGreaterThan( + callsBefore + ); + }); + + it("decrements page when deleting the last item on a page > 1", async () => { + const store = mockStore({ + adminAccessListState: { + ...baseListState, + currentPage: 2, + admin_accesses: [{ id: 1, title: "Group A" }] + }, + adminAccessState: baseFormState + }); + + render( + + + + + + ); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "delete" })); + await flushPromises(); + }); + + const lastCall = + mockGetAdminAccesses.mock.calls[ + mockGetAdminAccesses.mock.calls.length - 1 + ]; + expect(lastCall[1]).toBe(1); + }); + }); +}); diff --git a/src/pages/admin_access/admin-access-form-popup.js b/src/pages/admin_access/admin-access-form-popup.js new file mode 100644 index 000000000..9c31ea244 --- /dev/null +++ b/src/pages/admin_access/admin-access-form-popup.js @@ -0,0 +1,82 @@ +import React, { useState } from "react"; +import { connect } from "react-redux"; +import T from "i18n-react/dist/i18n-react"; +import Dialog from "@mui/material/Dialog"; +import DialogContent from "@mui/material/DialogContent"; +import DialogTitle from "@mui/material/DialogTitle"; +import Divider from "@mui/material/Divider"; +import IconButton from "@mui/material/IconButton"; +import CloseIcon from "@mui/icons-material/Close"; +import AdminAccessForm from "../../components/forms/admin-access-form"; +import { resetAdminAccessForm } from "../../actions/admin-access-actions"; + +const AdminAccessFormPopup = ({ + entity, + errors, + onClose, + onSave, + resetAdminAccessForm +}) => { + const [isSaving, setIsSaving] = useState(false); + + const handleClose = () => { + if (isSaving) return; + resetAdminAccessForm(); + onClose(); + }; + + const handleSave = (adminAccessEntity) => { + if (isSaving) return; + setIsSaving(true); + onSave(adminAccessEntity) + .then(() => onClose()) + .catch(() => {}) + .finally(() => { + setIsSaving(false); + }); + }; + + return ( + + + + {entity.id ? T.translate("general.edit") : T.translate("general.add")}{" "} + {T.translate("admin_access.admin_access")} + + + + + + + + + + + ); +}; + +const mapStateToProps = ({ adminAccessState }) => ({ + entity: adminAccessState.entity, + errors: adminAccessState.errors +}); + +export default connect(mapStateToProps, { + resetAdminAccessForm +})(AdminAccessFormPopup); diff --git a/src/pages/admin_access/admin-access-list-page.js b/src/pages/admin_access/admin-access-list-page.js index d4c943e15..e99bab451 100644 --- a/src/pages/admin_access/admin-access-list-page.js +++ b/src/pages/admin_access/admin-access-list-page.js @@ -11,167 +11,232 @@ * limitations under the License. * */ -import React from "react"; +import React, { useEffect, useMemo, useState } from "react"; import { connect } from "react-redux"; import T from "i18n-react/dist/i18n-react"; -import Swal from "sweetalert2"; -import { Pagination } from "react-bootstrap"; -import FreeTextSearch from "openstack-uicore-foundation/lib/components/free-text-search" -import Table from "openstack-uicore-foundation/lib/components/table"; -import { getSummitById } from "../../actions/summit-actions"; +import Box from "@mui/material/Box"; +import Button from "@mui/material/Button"; +import Grid2 from "@mui/material/Grid2"; +import AddIcon from "@mui/icons-material/Add"; +import MuiTable from "openstack-uicore-foundation/lib/components/mui/table"; +import MuiSearchInput from "openstack-uicore-foundation/lib/components/mui/search-input"; import { getAdminAccesses, - deleteAdminAccess + deleteAdminAccess, + getAdminAccess, + resetAdminAccessForm, + saveAdminAccess } from "../../actions/admin-access-actions"; +import { DEFAULT_CURRENT_PAGE } from "../../utils/constants"; +import AdminAccessFormPopup from "./admin-access-form-popup"; + +const AdminAccessListPage = ({ + admin_accesses, + totalAdminAccesses, + currentPage, + perPage, + term, + order, + orderDir, + match, + history, + getAdminAccesses, + deleteAdminAccess, + getAdminAccess, + resetAdminAccessForm, + saveAdminAccess +}) => { + const [searchTerm, setSearchTerm] = useState(term || ""); + const [open, setOpen] = useState(false); + + useEffect(() => { + getAdminAccesses(term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir); + }, [getAdminAccesses]); + + useEffect(() => { + const { access_id: accessId } = match.params; + const isNew = /\/new$/.test(history.location.pathname); + + if (isNew) { + resetAdminAccessForm(); + setOpen(true); + return; + } + + if (accessId) { + getAdminAccess(accessId) + .then(() => setOpen(true)) + .catch(() => history.push("/app/admin-access")); + return; + } + + setOpen(false); + }, [match.params.access_id, history.location.pathname]); + + const handlePageChange = (page) => { + getAdminAccesses(term, page, perPage, order, orderDir); + }; + + const handlePerPageChange = (newPerPage) => { + getAdminAccesses(term, DEFAULT_CURRENT_PAGE, newPerPage, order, orderDir); + }; + + const handleSort = (key, dir) => { + getAdminAccesses(term, currentPage, perPage, key, dir); + }; + + const handleSearch = (value) => { + setSearchTerm(value); + getAdminAccesses(value, DEFAULT_CURRENT_PAGE, perPage, order, orderDir); + }; + + const handleNewAdminAccess = () => { + history.push("/app/admin-access/new"); + }; -class AdminAccessListPage extends React.Component { - constructor(props) { - super(props); - - this.handleEdit = this.handleEdit.bind(this); - this.handlePageChange = this.handlePageChange.bind(this); - this.handleSort = this.handleSort.bind(this); - this.handleSearch = this.handleSearch.bind(this); - this.handleNewAdminAccess = this.handleNewAdminAccess.bind(this); - this.handleDeleteAdminAccess = this.handleDeleteAdminAccess.bind(this); - - this.state = {}; - } + const handleEdit = (row) => { + history.push(`/app/admin-access/${row.id}`); + }; - componentDidMount() { - this.props.getAdminAccesses(); - } + const handleDeleteAdminAccess = (rowOrId) => { + const accessId = typeof rowOrId === "object" ? rowOrId?.id : rowOrId; - handleEdit(admin_access_id) { - const { history } = this.props; - history.push(`/app/admin-access/${admin_access_id}`); - } + if (!accessId) return; - handlePageChange(page) { - const { term, order, orderDir, perPage } = this.props; - this.props.getAdminAccesses(term, page, perPage, order, orderDir); - } + const nextPage = + admin_accesses.length === 1 && currentPage > 1 + ? currentPage - 1 + : currentPage; - handleSort(index, key, dir) { - const { term, page, perPage } = this.props; - this.props.getAdminAccesses(term, page, perPage, key, dir); - } + deleteAdminAccess(accessId) + .finally(() => { + getAdminAccesses(term, nextPage, perPage, order, orderDir); + }) + .catch(() => {}); + }; - handleSearch(term) { - const { order, orderDir, page, perPage } = this.props; - this.props.getAdminAccesses(term, page, perPage, order, orderDir); - } + const handleSave = (entity) => + saveAdminAccess(entity).then(() => + getAdminAccesses(term, currentPage, perPage, order, orderDir) + ); - handleNewAdminAccess(ev) { - const { history } = this.props; - ev.preventDefault(); + const closeDialog = () => { + setOpen(false); + history.push("/app/admin-access"); + }; - history.push("/app/admin-access/new"); - } - - handleDeleteAdminAccess(accessId) { - const { deleteAdminAccess, admin_accesses } = this.props; - const admin_access = admin_accesses.find((t) => t.id === accessId); - - Swal.fire({ - title: T.translate("general.are_you_sure"), - text: `${T.translate("admin_access.delete_warning")} ${ - admin_access.title - }`, - type: "warning", - showCancelButton: true, - confirmButtonColor: "#DD6B55", - confirmButtonText: T.translate("general.yes_delete") - }).then((result) => { - if (result.value) { - deleteAdminAccess(accessId); - } - }); - } - - render() { - const { admin_accesses, lastPage, currentPage, term, order, orderDir } = - this.props; - - const columns = [ - { columnKey: "id", value: T.translate("general.id"), sortable: true }, + const columns = useMemo( + () => [ + { columnKey: "id", header: T.translate("general.id"), sortable: true }, { columnKey: "title", - value: T.translate("admin_access.title"), + header: T.translate("admin_access.title"), sortable: true }, - { columnKey: "summits", value: T.translate("admin_access.summits") }, - { columnKey: "members", value: T.translate("admin_access.members") } - ]; - - const table_options = { - sortCol: order, - sortDir: orderDir, - actions: { - edit: { onClick: this.handleEdit }, - delete: { onClick: this.handleDeleteAdminAccess } - } - }; - - return ( -
-

{T.translate("admin_access.admin_access_list")}

-
-
- +

{T.translate("admin_access.admin_access_list")}

+ + + + {totalItems} {T.translate("general.items")} + + + + + -
-
- -
-
- - {admin_accesses.length === 0 && ( -
{T.translate("admin_access.no_results")}
- )} - - {admin_accesses.length > 0 && ( -
- - - - )} - - ); - } -} + + + + + + {admin_accesses.length === 0 && ( +
{T.translate("admin_access.no_results")}
+ )} + + {admin_accesses.length > 0 && ( + adminAccess?.title ?? adminAccess?.id} + deleteDialogBody={(groupName) => + `${T.translate("admin_access.delete_warning")} "${groupName}"` + } + confirmButtonColor="error" + onPageChange={handlePageChange} + onPerPageChange={handlePerPageChange} + onSort={handleSort} + onEdit={handleEdit} + onDelete={handleDeleteAdminAccess} + /> + )} + + {open && ( + + )} + + ); +}; const mapStateToProps = ({ adminAccessListState }) => ({ ...adminAccessListState }); export default connect(mapStateToProps, { - getSummitById, getAdminAccesses, - deleteAdminAccess + deleteAdminAccess, + getAdminAccess, + resetAdminAccessForm, + saveAdminAccess })(AdminAccessListPage); diff --git a/src/pages/admin_access/edit-admin-access-page.js b/src/pages/admin_access/edit-admin-access-page.js deleted file mode 100644 index 12fe488b6..000000000 --- a/src/pages/admin_access/edit-admin-access-page.js +++ /dev/null @@ -1,89 +0,0 @@ -/** - * Copyright 2020 OpenStack Foundation - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - **/ - -import React from "react"; -import { connect } from "react-redux"; -import T from "i18n-react/dist/i18n-react"; -import { Breadcrumb } from "react-breadcrumbs"; -import AdminAccessForm from "../../components/forms/admin-access-form"; -import { getSummitById } from "../../actions/summit-actions"; -import { - getAdminAccess, - resetAdminAccessForm, - saveAdminAccess -} from "../../actions/admin-access-actions"; - -//import '../../styles/edit-admin-access-page.less'; - -class EditAdminAccessPage extends React.Component { - constructor(props) { - super(props); - - this.state = {}; - - const accessId = props.match.params.access_id; - - if (!accessId) { - props.resetAdminAccessForm(); - } else { - props.getAdminAccess(accessId); - } - } - - componentDidUpdate(prevProps, prevState, snapshot) { - const oldId = prevProps.match.params.access_id; - const newId = this.props.match.params.access_id; - - if (oldId !== newId) { - if (!newId) { - this.props.resetTemplateForm(); - } else { - this.props.getAdminAccess(newId); - } - } - } - - render() { - const { entity, errors, match } = this.props; - const title = entity.id - ? T.translate("general.edit") - : T.translate("general.add"); - const breadcrumb = entity.id ? entity.title : T.translate("general.new"); - - return ( -
- -

- {title} {T.translate("admin_access.admin_access")} -

-
- -
- ); - } -} - -const mapStateToProps = ({ adminAccessState }) => ({ - ...adminAccessState -}); - -export default connect(mapStateToProps, { - getSummitById, - getAdminAccess, - resetAdminAccessForm, - saveAdminAccess -})(EditAdminAccessPage); diff --git a/src/reducers/admin_access/__tests__/admin-access-list-reducer.test.js b/src/reducers/admin_access/__tests__/admin-access-list-reducer.test.js new file mode 100644 index 000000000..76c65bae0 --- /dev/null +++ b/src/reducers/admin_access/__tests__/admin-access-list-reducer.test.js @@ -0,0 +1,49 @@ +import adminAccessListReducer from "../admin-access-list-reducer"; +import { ADMIN_ACCESS_DELETED } from "../../../actions/admin-access-actions"; + +describe("adminAccessListReducer", () => { + test("decrements totalAdminAccesses when deleting an existing row", () => { + const initialState = { + admin_accesses: [ + { id: 10, title: "Group A" }, + { id: 11, title: "Group B" } + ], + totalAdminAccesses: 2, + term: "", + order: "id", + orderDir: 1, + currentPage: 1, + lastPage: 1, + perPage: 10 + }; + + const result = adminAccessListReducer(initialState, { + type: ADMIN_ACCESS_DELETED, + payload: { adminAccessId: 10 } + }); + + expect(result.admin_accesses).toStrictEqual([{ id: 11, title: "Group B" }]); + expect(result.totalAdminAccesses).toBe(1); + }); + + test("keeps totalAdminAccesses when deleted id does not exist", () => { + const initialState = { + admin_accesses: [{ id: 10, title: "Group A" }], + totalAdminAccesses: 1, + term: "", + order: "id", + orderDir: 1, + currentPage: 1, + lastPage: 1, + perPage: 10 + }; + + const result = adminAccessListReducer(initialState, { + type: ADMIN_ACCESS_DELETED, + payload: { adminAccessId: 999 } + }); + + expect(result.admin_accesses).toStrictEqual([{ id: 10, title: "Group A" }]); + expect(result.totalAdminAccesses).toBe(1); + }); +}); diff --git a/src/reducers/admin_access/admin-access-list-reducer.js b/src/reducers/admin_access/admin-access-list-reducer.js index cbf46006f..decacd9d5 100644 --- a/src/reducers/admin_access/admin-access-list-reducer.js +++ b/src/reducers/admin_access/admin-access-list-reducer.js @@ -9,18 +9,19 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - **/ + * */ +import { LOGOUT_USER } from "openstack-uicore-foundation/lib/security/actions"; import { RECEIVE_ADMIN_ACCESSES, REQUEST_ADMIN_ACCESSES, ADMIN_ACCESS_DELETED } from "../../actions/admin-access-actions"; -import { LOGOUT_USER } from "openstack-uicore-foundation/lib/security/actions"; const DEFAULT_STATE = { admin_accesses: [], + totalAdminAccesses: 0, term: null, order: "id", orderDir: 1, @@ -36,37 +37,50 @@ const adminAccessListReducer = (state = DEFAULT_STATE, action) => { return DEFAULT_STATE; } case REQUEST_ADMIN_ACCESSES: { - let { order, orderDir, term } = payload; + const { order, orderDir, term, page, perPage } = payload; - return { ...state, order, orderDir, term }; + return { + ...state, + order, + orderDir, + term, + currentPage: page || state.currentPage, + perPage: perPage || state.perPage + }; } case RECEIVE_ADMIN_ACCESSES: { - let { total, last_page, current_page } = payload.response; - let admin_accesses = payload.response.data.map((aa) => { - return { + const { total, last_page, current_page, per_page } = payload.response; + const admin_accesses = payload.response.data.map((aa) => ({ id: aa.id, title: aa.title, members: aa.members .map((m) => `${m.first_name} ${m.last_name}`) .join(", "), summits: aa.summits.map((s) => s.name).join(", ") - }; - }); + })); return { ...state, - admin_accesses: admin_accesses, + admin_accesses, + totalAdminAccesses: total || 0, currentPage: current_page, - lastPage: last_page + lastPage: last_page, + perPage: per_page || state.perPage }; } case ADMIN_ACCESS_DELETED: { - let { adminAccessId } = payload; + const { adminAccessId } = payload; + const filteredAdminAccesses = state.admin_accesses.filter( + (aa) => aa.id !== adminAccessId + ); + return { ...state, - admin_accesses: state.admin_accesses.filter( - (aa) => aa.id !== adminAccessId - ) + admin_accesses: filteredAdminAccesses, + totalAdminAccesses: + filteredAdminAccesses.length === state.admin_accesses.length + ? state.totalAdminAccesses + : Math.max(0, (state.totalAdminAccesses ?? 0) - 1) }; } default: From ea6f5db7a58af22c93e188265fd0116536d59fa1 Mon Sep 17 00:00:00 2001 From: Priscila Moneo Date: Thu, 25 Jun 2026 13:21:26 -0300 Subject: [PATCH 2/4] fix: feedback from PR Signed-off-by: Priscila Moneo --- src/components/forms/admin-access-form.js | 10 +++++++--- src/pages/admin_access/admin-access-form-popup.js | 13 +------------ src/pages/admin_access/admin-access-list-page.js | 9 +++++++-- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/components/forms/admin-access-form.js b/src/components/forms/admin-access-form.js index 1a0b44b59..beee899a2 100644 --- a/src/components/forms/admin-access-form.js +++ b/src/components/forms/admin-access-form.js @@ -62,6 +62,10 @@ const AdminAccessForm = ({ return ( + + {entity.id ? T.translate("general.edit") : T.translate("general.add")}{" "} + {T.translate("admin_access.admin_access")} + - "email" in member - ? `${member.first_name} ${member.last_name} (${member.email})` - : `${member.first_name} ${member.last_name} (${member.id})` + `${member.first_name} ${member.last_name} (${ + "email" in member ? member.email : member.id + })` } onChange={(ev) => formik.setFieldValue("members", ev.target.value) diff --git a/src/pages/admin_access/admin-access-form-popup.js b/src/pages/admin_access/admin-access-form-popup.js index 9c31ea244..997424977 100644 --- a/src/pages/admin_access/admin-access-form-popup.js +++ b/src/pages/admin_access/admin-access-form-popup.js @@ -1,6 +1,5 @@ import React, { useState } from "react"; import { connect } from "react-redux"; -import T from "i18n-react/dist/i18n-react"; import Dialog from "@mui/material/Dialog"; import DialogContent from "@mui/material/DialogContent"; import DialogTitle from "@mui/material/DialogTitle"; @@ -44,17 +43,7 @@ const AdminAccessFormPopup = ({ maxWidth="md" fullWidth > - - - {entity.id ? T.translate("general.edit") : T.translate("general.add")}{" "} - {T.translate("admin_access.admin_access")} - + diff --git a/src/pages/admin_access/admin-access-list-page.js b/src/pages/admin_access/admin-access-list-page.js index e99bab451..efb54b9f2 100644 --- a/src/pages/admin_access/admin-access-list-page.js +++ b/src/pages/admin_access/admin-access-list-page.js @@ -64,10 +64,15 @@ const AdminAccessListPage = ({ } if (accessId) { + let ignore = false; getAdminAccess(accessId) - .then(() => setOpen(true)) + .then(() => { + if (!ignore) setOpen(true); + }) .catch(() => history.push("/app/admin-access")); - return; + return () => { + ignore = true; + }; } setOpen(false); From 4d3435f714f2eed661ff433643317bb59342488b Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 29 Jun 2026 12:43:01 -0300 Subject: [PATCH 3/4] fix(admin-access): refactor list page to popup pattern and fix post-save crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace route-driven popup (URL changes) with inventory-style modal pattern: no URL mutation on open/edit/close, uses useState(false) open flag, getAdminAccess(id).then(() => setOpen(true)) for edit flow - Simplify admin-access-layout to single route (remove /new and /:id sub-routes) - Reset to DEFAULT_CURRENT_PAGE after delete (align with codebase convention) - Add expand=members,summits to POST/PUT params so ADMIN_ACCESS_ADDED response returns full objects — fixes white screen crash from enableReinitialize reinitializing Formik with integer IDs before popup closes - Add defensive guard in getOptionLabel for non-object member values - Rewrite tests for new popup-based flow (19 passing) --- src/actions/admin-access-actions.js | 2 +- src/components/forms/admin-access-form.js | 10 +- src/layouts/admin-access-layout.js | 12 - .../__tests__/admin-access-list-page.test.js | 222 ++++++------------ .../admin_access/admin-access-list-page.js | 114 +++------ 5 files changed, 107 insertions(+), 253 deletions(-) diff --git a/src/actions/admin-access-actions.js b/src/actions/admin-access-actions.js index 36c18b0ec..141f05282 100644 --- a/src/actions/admin-access-actions.js +++ b/src/actions/admin-access-actions.js @@ -131,7 +131,7 @@ export const saveAdminAccess = (entity) => async (dispatch) => { dispatch(startLoading()); const normalizedEntity = normalizeEntity(entity); - const params = { access_token: accessToken }; + const params = { access_token: accessToken, expand: "members,summits" }; if (entity.id) { return putRequest( diff --git a/src/components/forms/admin-access-form.js b/src/components/forms/admin-access-form.js index beee899a2..e243d9146 100644 --- a/src/components/forms/admin-access-form.js +++ b/src/components/forms/admin-access-form.js @@ -89,11 +89,13 @@ const AdminAccessForm = ({ - `${member.first_name} ${member.last_name} (${ + getOptionLabel={(member) => { + if (typeof member !== "object" || member === null) + return String(member ?? ""); + return `${member.first_name} ${member.last_name} (${ "email" in member ? member.email : member.id - })` - } + })`; + }} onChange={(ev) => formik.setFieldValue("members", ev.target.value) } diff --git a/src/layouts/admin-access-layout.js b/src/layouts/admin-access-layout.js index 8e61a069e..22509016b 100644 --- a/src/layouts/admin-access-layout.js +++ b/src/layouts/admin-access-layout.js @@ -29,18 +29,6 @@ const AdminAccessLayout = ({ match }) => ( - - diff --git a/src/pages/admin_access/__tests__/admin-access-list-page.test.js b/src/pages/admin_access/__tests__/admin-access-list-page.test.js index 154030f90..c36e50439 100644 --- a/src/pages/admin_access/__tests__/admin-access-list-page.test.js +++ b/src/pages/admin_access/__tests__/admin-access-list-page.test.js @@ -1,6 +1,6 @@ import React from "react"; import { Provider } from "react-redux"; -import { MemoryRouter, Route } from "react-router-dom"; +import { MemoryRouter } from "react-router-dom"; import { act, render, @@ -56,7 +56,6 @@ jest.mock("openstack-uicore-foundation/lib/components/mui/table", () => { ))} ); - return MockMuiTable; }); @@ -82,7 +81,6 @@ jest.mock("../../../components/forms/admin-access-form", () => { ); - return MockAdminAccessForm; }); @@ -112,19 +110,16 @@ const baseFormState = { errors: {} }; -const renderPage = (path = "/app/admin-access", formState = baseFormState) => { +const renderPage = (listState = baseListState, formState = baseFormState) => { const store = mockStore({ - adminAccessListState: baseListState, + adminAccessListState: listState, adminAccessState: formState }); render( - - + + ); @@ -132,6 +127,13 @@ const renderPage = (path = "/app/admin-access", formState = baseFormState) => { return store; }; +const openNewPopup = async () => { + fireEvent.click(screen.getByRole("button", { name: "admin_access.add" })); + await waitFor(() => + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() + ); +}; + describe("AdminAccessListPage", () => { beforeEach(() => { jest.clearAllMocks(); @@ -143,47 +145,38 @@ describe("AdminAccessListPage", () => { }); describe("rendering and navigation", () => { - it("renders grid and opens popup for new item", async () => { - renderPage("/app/admin-access"); - + it("renders the table and no popup on initial load", () => { + renderPage(); expect(screen.getByTestId("mui-table")).toBeInTheDocument(); expect(screen.queryByTestId("admin-access-form")).not.toBeInTheDocument(); - - fireEvent.click(screen.getByRole("button", { name: /create|add/i })); - - await waitFor(() => { - expect(screen.getByTestId("admin-access-form")).toBeInTheDocument(); - }); }); - it("opens popup when route has an access id", async () => { - const formState = { - entity: { id: 1, title: "Group A", members: [], summits: [] }, - errors: {} - }; - - renderPage("/app/admin-access/1", formState); + it("opens popup and resets form when Add is clicked", async () => { + renderPage(); + await openNewPopup(); + expect(mockResetAdminAccessForm).toHaveBeenCalled(); + }); - await waitFor(() => { - expect(screen.getByTestId("admin-access-form")).toBeInTheDocument(); - }); + it("fetches the item and opens popup when edit row is clicked", async () => { + renderPage(); + fireEvent.click(screen.getByRole("button", { name: "edit" })); + await waitFor(() => + expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() + ); + expect(mockGetAdminAccess).toHaveBeenCalledWith(1); }); it("requests data with selected rows per page", async () => { - renderPage("/app/admin-access"); - + renderPage(); fireEvent.click(screen.getByRole("button", { name: "per-page-25" })); - await waitFor(() => { expect(mockGetAdminAccesses).toHaveBeenCalledWith("", 1, 25, "id", 1); }); }); it("requests data with new sort column and direction", async () => { - renderPage("/app/admin-access"); - + renderPage(); fireEvent.click(screen.getByRole("button", { name: "sort-col" })); - await waitFor(() => { expect(mockGetAdminAccesses).toHaveBeenCalledWith( "", @@ -196,12 +189,10 @@ describe("AdminAccessListPage", () => { }); it("requests data with search term and resets to page 1", async () => { - renderPage("/app/admin-access"); - + renderPage(); fireEvent.change(screen.getByPlaceholderText("search"), { target: { value: "admins" } }); - await waitFor(() => { expect(mockGetAdminAccesses).toHaveBeenCalledWith( "admins", @@ -214,14 +205,43 @@ describe("AdminAccessListPage", () => { }); }); + describe("popup open/close", () => { + it("closes popup and resets form when close button is clicked", async () => { + renderPage(); + await openNewPopup(); + + fireEvent.click(screen.getByTestId("CloseIcon").closest("button")); + + await waitFor(() => + expect( + screen.queryByTestId("admin-access-form") + ).not.toBeInTheDocument() + ); + expect(mockResetAdminAccessForm).toHaveBeenCalled(); + }); + + it("does not open popup when getAdminAccess rejects", async () => { + mockGetAdminAccess.mockReturnValue(() => + Promise.reject(new Error("not found")) + ); + + renderPage(); + fireEvent.click(screen.getByRole("button", { name: "edit" })); + + await act(async () => { + await flushPromises(); + }); + + expect(screen.queryByTestId("admin-access-form")).not.toBeInTheDocument(); + }); + }); + describe("save guard", () => { it("disables the X button and submit while save is in flight", async () => { mockSaveAdminAccess.mockReturnValue(() => new Promise(() => {})); - renderPage("/app/admin-access/new"); - await waitFor(() => - expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() - ); + renderPage(); + await openNewPopup(); fireEvent.click(screen.getByRole("button", { name: "submit-form" })); @@ -240,10 +260,8 @@ describe("AdminAccessListPage", () => { Promise.reject(new Error("save failed")) ); - renderPage("/app/admin-access/new"); - await waitFor(() => - expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() - ); + renderPage(); + await openNewPopup(); const callsBefore = mockGetAdminAccesses.mock.calls.length; @@ -263,74 +281,10 @@ describe("AdminAccessListPage", () => { }); }); - describe("route-driven open/close", () => { - it("does not open dialog and navigates back when getAdminAccess rejects", async () => { - mockGetAdminAccess.mockReturnValue(() => - Promise.reject(new Error("not found")) - ); - - renderPage("/app/admin-access/1"); - - await act(async () => { - await flushPromises(); - }); - - expect(screen.queryByTestId("admin-access-form")).not.toBeInTheDocument(); - }); - - it("closes the dialog when the URL changes from a detail route to the list route", async () => { - const store = mockStore({ - adminAccessListState: baseListState, - adminAccessState: { - entity: { id: 1, title: "Group A", members: [], summits: [] }, - errors: {} - } - }); - - let capturedHistory; - - render( - - - <> - - { - capturedHistory = history; - return null; - }} - /> - - - - ); - - await waitFor(() => - expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() - ); - - await act(async () => { - capturedHistory.push("/app/admin-access"); - await flushPromises(); - }); - - await waitFor(() => - expect( - screen.queryByTestId("admin-access-form") - ).not.toBeInTheDocument() - ); - }); - }); - describe("list management", () => { it("reloads the list after a successful save", async () => { - renderPage("/app/admin-access/new"); - await waitFor(() => - expect(screen.getByTestId("admin-access-form")).toBeInTheDocument() - ); + renderPage(); + await openNewPopup(); const callsBefore = mockGetAdminAccesses.mock.calls.length; @@ -345,26 +299,7 @@ describe("AdminAccessListPage", () => { }); it("reloads the list after a successful delete", async () => { - renderPage("/app/admin-access"); - - const callsBefore = mockGetAdminAccesses.mock.calls.length; - - await act(async () => { - fireEvent.click(screen.getByRole("button", { name: "delete" })); - await flushPromises(); - }); - - expect(mockGetAdminAccesses.mock.calls.length).toBeGreaterThan( - callsBefore - ); - }); - - it("re-syncs the list after a failed delete", async () => { - mockDeleteAdminAccess.mockReturnValue(() => - Promise.reject(new Error("delete failed")) - ); - - renderPage("/app/admin-access"); + renderPage(); const callsBefore = mockGetAdminAccesses.mock.calls.length; @@ -373,32 +308,13 @@ describe("AdminAccessListPage", () => { await flushPromises(); }); - // .finally() fires even on rejection expect(mockGetAdminAccesses.mock.calls.length).toBeGreaterThan( callsBefore ); }); - it("decrements page when deleting the last item on a page > 1", async () => { - const store = mockStore({ - adminAccessListState: { - ...baseListState, - currentPage: 2, - admin_accesses: [{ id: 1, title: "Group A" }] - }, - adminAccessState: baseFormState - }); - - render( - - - - - - ); + it("reloads from page 1 after a successful delete", async () => { + renderPage({ ...baseListState, currentPage: 3 }); await act(async () => { fireEvent.click(screen.getByRole("button", { name: "delete" })); diff --git a/src/pages/admin_access/admin-access-list-page.js b/src/pages/admin_access/admin-access-list-page.js index efb54b9f2..5be41f527 100644 --- a/src/pages/admin_access/admin-access-list-page.js +++ b/src/pages/admin_access/admin-access-list-page.js @@ -11,7 +11,7 @@ * limitations under the License. * */ -import React, { useEffect, useMemo, useState } from "react"; +import React, { useEffect, useState } from "react"; import { connect } from "react-redux"; import T from "i18n-react/dist/i18n-react"; import Box from "@mui/material/Box"; @@ -30,6 +30,17 @@ import { import { DEFAULT_CURRENT_PAGE } from "../../utils/constants"; import AdminAccessFormPopup from "./admin-access-form-popup"; +const columns = [ + { columnKey: "id", header: T.translate("general.id"), sortable: true }, + { + columnKey: "title", + header: T.translate("admin_access.title"), + sortable: true + }, + { columnKey: "summits", header: T.translate("admin_access.summits") }, + { columnKey: "members", header: T.translate("admin_access.members") } +]; + const AdminAccessListPage = ({ admin_accesses, totalAdminAccesses, @@ -38,46 +49,18 @@ const AdminAccessListPage = ({ term, order, orderDir, - match, - history, getAdminAccesses, deleteAdminAccess, getAdminAccess, resetAdminAccessForm, saveAdminAccess }) => { - const [searchTerm, setSearchTerm] = useState(term || ""); const [open, setOpen] = useState(false); useEffect(() => { getAdminAccesses(term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir); }, [getAdminAccesses]); - useEffect(() => { - const { access_id: accessId } = match.params; - const isNew = /\/new$/.test(history.location.pathname); - - if (isNew) { - resetAdminAccessForm(); - setOpen(true); - return; - } - - if (accessId) { - let ignore = false; - getAdminAccess(accessId) - .then(() => { - if (!ignore) setOpen(true); - }) - .catch(() => history.push("/app/admin-access")); - return () => { - ignore = true; - }; - } - - setOpen(false); - }, [match.params.access_id, history.location.pathname]); - const handlePageChange = (page) => { getAdminAccesses(term, page, perPage, order, orderDir); }; @@ -91,33 +74,24 @@ const AdminAccessListPage = ({ }; const handleSearch = (value) => { - setSearchTerm(value); getAdminAccesses(value, DEFAULT_CURRENT_PAGE, perPage, order, orderDir); }; const handleNewAdminAccess = () => { - history.push("/app/admin-access/new"); + resetAdminAccessForm(); + setOpen(true); }; const handleEdit = (row) => { - history.push(`/app/admin-access/${row.id}`); + getAdminAccess(row.id) + .then(() => setOpen(true)) + .catch(() => {}); }; - const handleDeleteAdminAccess = (rowOrId) => { - const accessId = typeof rowOrId === "object" ? rowOrId?.id : rowOrId; - - if (!accessId) return; - - const nextPage = - admin_accesses.length === 1 && currentPage > 1 - ? currentPage - 1 - : currentPage; - - deleteAdminAccess(accessId) - .finally(() => { - getAdminAccesses(term, nextPage, perPage, order, orderDir); - }) - .catch(() => {}); + const handleDelete = (accessId) => { + deleteAdminAccess(accessId).then(() => + getAdminAccesses(term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir) + ); }; const handleSave = (entity) => @@ -125,31 +99,12 @@ const AdminAccessListPage = ({ getAdminAccesses(term, currentPage, perPage, order, orderDir) ); - const closeDialog = () => { + const handleClose = () => { + resetAdminAccessForm(); setOpen(false); - history.push("/app/admin-access"); - }; - - const columns = useMemo( - () => [ - { columnKey: "id", header: T.translate("general.id"), sortable: true }, - { - columnKey: "title", - header: T.translate("admin_access.title"), - sortable: true - }, - { columnKey: "summits", header: T.translate("admin_access.summits") }, - { columnKey: "members", header: T.translate("admin_access.members") } - ], - [] - ); - - const tableOptions = { - sortCol: order, - sortDir: orderDir }; - const totalItems = totalAdminAccesses; + const tableOptions = { sortCol: order, sortDir: orderDir }; return ( @@ -157,15 +112,11 @@ const AdminAccessListPage = ({ - {totalItems} {T.translate("general.items")} + {totalAdminAccesses} {T.translate("general.items")} @@ -213,7 +161,7 @@ const AdminAccessListPage = ({ options={tableOptions} perPage={perPage} currentPage={currentPage} - totalRows={totalItems} + totalRows={totalAdminAccesses} getName={(adminAccess) => adminAccess?.title ?? adminAccess?.id} deleteDialogBody={(groupName) => `${T.translate("admin_access.delete_warning")} "${groupName}"` @@ -223,12 +171,12 @@ const AdminAccessListPage = ({ onPerPageChange={handlePerPageChange} onSort={handleSort} onEdit={handleEdit} - onDelete={handleDeleteAdminAccess} + onDelete={handleDelete} /> )} {open && ( - + )} ); From 6fa265636cdda46fd36f714dc29be8fc37339d47 Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 29 Jun 2026 13:00:05 -0300 Subject: [PATCH 4/4] fix(admin-access): remove double reset in popup, add missing test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove resetAdminAccessForm from popup handleClose — reset responsibility belongs to the parent per popup-dialog-pattern (was dispatched twice on close) - Add expand=members,summits assertion in saveAdminAccess tests (POST and PUT) to lock in the crash-prevention fix against future regressions - Add handlePageChange test and expose onPageChange in MuiTable mock --- .../__tests__/admin-access-actions.test.js | 32 +++++++++++++++++++ .../__tests__/admin-access-list-page.test.js | 12 +++++++ .../admin_access/admin-access-form-popup.js | 14 ++------ 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/actions/__tests__/admin-access-actions.test.js b/src/actions/__tests__/admin-access-actions.test.js index e7d000167..bdd920af8 100644 --- a/src/actions/__tests__/admin-access-actions.test.js +++ b/src/actions/__tests__/admin-access-actions.test.js @@ -50,6 +50,22 @@ describe("saveAdminAccess", () => { }); describe("create path (entity has no id)", () => { + it("includes expand=members,summits in request params", async () => { + let capturedParams; + postRequest.mockImplementation((req, res) => (params) => (dispatch) => { + capturedParams = params; + return requestMock(req, res)(params)(dispatch); + }); + + const store = mockStore({}); + await store.dispatch( + saveAdminAccess({ title: "Group A", members: [], summits: [] }) + ); + await flushPromises(); + + expect(capturedParams).toMatchObject({ expand: "members,summits" }); + }); + it("returns a Promise", async () => { const store = mockStore({}); const result = store.dispatch( @@ -90,6 +106,22 @@ describe("saveAdminAccess", () => { }); describe("update path (entity has id)", () => { + it("includes expand=members,summits in request params", async () => { + let capturedParams; + putRequest.mockImplementation((req, res) => (params) => (dispatch) => { + capturedParams = params; + return requestMock(req, res)(params)(dispatch); + }); + + const store = mockStore({}); + await store.dispatch( + saveAdminAccess({ id: 1, title: "Group A", members: [], summits: [] }) + ); + await flushPromises(); + + expect(capturedParams).toMatchObject({ expand: "members,summits" }); + }); + it("returns a Promise", async () => { const store = mockStore({}); const result = store.dispatch( diff --git a/src/pages/admin_access/__tests__/admin-access-list-page.test.js b/src/pages/admin_access/__tests__/admin-access-list-page.test.js index c36e50439..e42593f22 100644 --- a/src/pages/admin_access/__tests__/admin-access-list-page.test.js +++ b/src/pages/admin_access/__tests__/admin-access-list-page.test.js @@ -33,10 +33,14 @@ jest.mock("openstack-uicore-foundation/lib/components/mui/table", () => { data = [], onEdit, onDelete, + onPageChange, onPerPageChange, onSort }) => (
+ @@ -166,6 +170,14 @@ describe("AdminAccessListPage", () => { expect(mockGetAdminAccess).toHaveBeenCalledWith(1); }); + it("requests data for the selected page", async () => { + renderPage(); + fireEvent.click(screen.getByRole("button", { name: "page-2" })); + await waitFor(() => { + expect(mockGetAdminAccesses).toHaveBeenCalledWith("", 2, 10, "id", 1); + }); + }); + it("requests data with selected rows per page", async () => { renderPage(); fireEvent.click(screen.getByRole("button", { name: "per-page-25" })); diff --git a/src/pages/admin_access/admin-access-form-popup.js b/src/pages/admin_access/admin-access-form-popup.js index 997424977..686e6a21f 100644 --- a/src/pages/admin_access/admin-access-form-popup.js +++ b/src/pages/admin_access/admin-access-form-popup.js @@ -7,20 +7,12 @@ import Divider from "@mui/material/Divider"; import IconButton from "@mui/material/IconButton"; import CloseIcon from "@mui/icons-material/Close"; import AdminAccessForm from "../../components/forms/admin-access-form"; -import { resetAdminAccessForm } from "../../actions/admin-access-actions"; -const AdminAccessFormPopup = ({ - entity, - errors, - onClose, - onSave, - resetAdminAccessForm -}) => { +const AdminAccessFormPopup = ({ entity, errors, onClose, onSave }) => { const [isSaving, setIsSaving] = useState(false); const handleClose = () => { if (isSaving) return; - resetAdminAccessForm(); onClose(); }; @@ -66,6 +58,4 @@ const mapStateToProps = ({ adminAccessState }) => ({ errors: adminAccessState.errors }); -export default connect(mapStateToProps, { - resetAdminAccessForm -})(AdminAccessFormPopup); +export default connect(mapStateToProps)(AdminAccessFormPopup);