diff --git a/src/layouts/__tests__/selection-plan-id-layout.test.js b/src/layouts/__tests__/selection-plan-id-layout.test.js new file mode 100644 index 000000000..ec1186061 --- /dev/null +++ b/src/layouts/__tests__/selection-plan-id-layout.test.js @@ -0,0 +1,183 @@ +/** + * Copyright 2026 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 { screen, act } from "@testing-library/react"; +import { Router, Route, Switch } from "react-router-dom"; +import { createMemoryHistory } from "history"; +import flushPromises from "flush-promises"; +import { renderWithRedux } from "../../utils/test-utils"; +import { getSelectionPlan } from "../../actions/selection-plan-actions"; +import { getMarketingSettingsBySelectionPlan } from "../../actions/marketing-actions"; +import SelectionPlanIdLayout from "../selection-plan-id-layout"; + +jest.mock("i18n-react", () => ({ + __esModule: true, + default: { translate: (k) => k } +})); + +// The page renders null until it's ready, so the breadcrumb's presence is our signal. +jest.mock("react-breadcrumbs", () => ({ + Breadcrumb: () =>
+})); + +jest.mock("../../actions/selection-plan-actions", () => ({ + __esModule: true, + ...jest.requireActual("../../actions/selection-plan-actions"), + getSelectionPlan: jest.fn(), + resetSelectionPlanForm: jest.fn(() => ({ type: "RESET_SELECTION_PLAN_FORM" })) +})); + +jest.mock("../../actions/marketing-actions", () => ({ + __esModule: true, + ...jest.requireActual("../../actions/marketing-actions"), + getMarketingSettingsBySelectionPlan: jest.fn() +})); + +// Stub the page: the real form needs a fuller marketing-settings shape than set up here. +jest.mock("../../pages/selection-plans/edit-selection-plan-page", () => ({ + __esModule: true, + default: () =>
+})); + +const renderAt = (path, currentSelectionPlan) => { + const history = createMemoryHistory({ initialEntries: [path] }); + return renderWithRedux( + + + , + { + initialState: { + currentSelectionPlanState: { entity: currentSelectionPlan }, + currentSummitState: { currentSummit: { id: 1 } } + } + } + ); +}; + +const settle = () => act(async () => flushPromises()); + +const isPageRendered = () => screen.queryByTestId("breadcrumb") !== null; + +// Mirrors the sibling /new and /:id(\d+) routes in selection-plan-layout.js. +const NewOrEditHarness = ({ history }) => ( + + + + + + +); + +describe("SelectionPlanIdLayout load guard", () => { + beforeEach(() => { + getSelectionPlan.mockReset(); + getMarketingSettingsBySelectionPlan.mockReset(); + getSelectionPlan.mockImplementation(() => () => Promise.resolve()); + getMarketingSettingsBySelectionPlan.mockImplementation( + () => () => Promise.resolve() + ); + }); + + it("does not render on direct load until the matching plan finishes fetching", async () => { + getSelectionPlan.mockImplementation(() => () => new Promise(() => {})); + renderAt("/app/summits/1/selection-plans/5", { id: 5 }); + expect(isPageRendered()).toBe(false); + }); + + it("stops rendering when switching to a different plan id until the store catches up", async () => { + const history = createMemoryHistory({ + initialEntries: ["/app/summits/1/selection-plans/5"] + }); + renderWithRedux( + + + , + { + initialState: { + currentSelectionPlanState: { entity: { id: 5 } }, + currentSummitState: { currentSummit: { id: 1 } } + } + } + ); + await settle(); + expect(isPageRendered()).toBe(true); + + act(() => { + history.push("/app/summits/1/selection-plans/8"); + }); + expect(isPageRendered()).toBe(false); + expect(getSelectionPlan).toHaveBeenCalledWith("8"); + + // Fetch settles, but the store's entity.id is still "5" — must stay unrendered. + await settle(); + expect(isPageRendered()).toBe(false); + }); + + it("stops rendering when navigating from an existing plan to /new until the store reflects the reset", async () => { + const history = createMemoryHistory({ + initialEntries: ["/app/summits/1/selection-plans/5"] + }); + renderWithRedux(, { + initialState: { + currentSelectionPlanState: { entity: { id: 5 } }, + currentSummitState: { currentSummit: { id: 1 } } + } + }); + await settle(); + expect(isPageRendered()).toBe(true); + + // Store still holds plan 5's entity (reset hasn't landed) — must not render. + act(() => { + history.push("/app/summits/1/selection-plans/new"); + }); + expect(isPageRendered()).toBe(false); + }); + + it("renders on /new once the store reflects the reset (default) entity", async () => { + const history = createMemoryHistory({ + initialEntries: ["/app/summits/1/selection-plans/new"] + }); + renderWithRedux(, { + initialState: { + currentSelectionPlanState: { entity: { id: 0 } }, + currentSummitState: { currentSummit: { id: 1 } } + } + }); + await settle(); + expect(isPageRendered()).toBe(true); + }); + + it("does not render or throw when the fetch rejects", async () => { + getSelectionPlan.mockImplementation( + () => () => Promise.reject(new Error("fail")) + ); + renderAt("/app/summits/1/selection-plans/5", { id: 0 }); + await expect(settle()).resolves.not.toThrow(); + expect(isPageRendered()).toBe(false); + }); +}); diff --git a/src/layouts/selection-plan-id-layout.js b/src/layouts/selection-plan-id-layout.js index 04e41a5ed..b0d73b510 100644 --- a/src/layouts/selection-plan-id-layout.js +++ b/src/layouts/selection-plan-id-layout.js @@ -1,4 +1,4 @@ -import React, { Suspense, useEffect } from "react"; +import React, { Suspense, useEffect, useState } from "react"; import { connect } from "react-redux"; import { Redirect, Route, Switch } from "react-router-dom"; import { Breadcrumb } from "react-breadcrumbs"; @@ -11,6 +11,9 @@ import { import { getMarketingSettingsBySelectionPlan } from "../actions/marketing-actions"; import { MAX_PER_PAGE } from "../utils/constants"; +const EditSelectionPlanPage = React.lazy(() => + import("../pages/selection-plans/edit-selection-plan-page") +); const SelectionPlanExtraQuestionsLayout = React.lazy(() => import("./selection-plan-extra-questions-layout") ); @@ -26,31 +29,47 @@ const SelectionPlanIdLayout = ({ resetSelectionPlanForm, getMarketingSettingsBySelectionPlan }) => { + const [hasLoaded, setHasLoaded] = useState(false); const selectionPlanId = match.params.selection_plan_id; const breadcrumb = selectionPlanId ? currentSelectionPlan.name : T.translate("general.new"); useEffect(() => { + setHasLoaded(false); if (!selectionPlanId) { resetSelectionPlanForm(); + setHasLoaded(true); } else { - getSelectionPlan(selectionPlanId).then(() => - getMarketingSettingsBySelectionPlan( - selectionPlanId, - null, - 1, - MAX_PER_PAGE + getSelectionPlan(selectionPlanId) + .then(() => + getMarketingSettingsBySelectionPlan( + selectionPlanId, + null, + 1, + MAX_PER_PAGE + ) ) - ); + .then(() => setHasLoaded(true)) + .catch(() => {}); } }, [selectionPlanId]); + if (!hasLoaded || currentSelectionPlan.id !== Number(selectionPlanId || 0)) { + return null; + } + return (
}> + - +
diff --git a/src/layouts/selection-plan-layout.js b/src/layouts/selection-plan-layout.js index 3a4043345..57d103b6d 100644 --- a/src/layouts/selection-plan-layout.js +++ b/src/layouts/selection-plan-layout.js @@ -38,7 +38,7 @@ const SelectionPlanLayout = ({ match, currentSummit }) => ( strict exact path={`${match.url}/new`} - render={() => } + component={SelectionPlanIdLayout} /> ({ getSelectionPlans: jest.fn(), - getSelectionPlan: jest.fn(), - deleteSelectionPlan: jest.fn(), - resetSelectionPlanForm: jest.fn(), - saveSelectionPlan: jest.fn(), - saveSelectionPlanSettings: jest.fn() -})); - -jest.mock("../../../actions/marketing-actions", () => ({ - getMarketingSettingsBySelectionPlan: jest.fn() + deleteSelectionPlan: jest.fn() })); jest.mock("openstack-uicore-foundation/lib/components/mui/table", () => ({ @@ -50,23 +37,12 @@ jest.mock( }) ); -jest.mock("../edit-selection-plan-page", () => ({ - __esModule: true, - default: ({ onSave }) => ( -
- -
- ) -})); - jest.mock("i18n-react/dist/i18n-react", () => ({ __esModule: true, default: { translate: (key) => key } })); -const mockHistory = { replace: jest.fn() }; +const mockHistory = { push: jest.fn(), replace: jest.fn() }; const mockMatch = { params: {} }; const initialState = { @@ -83,10 +59,6 @@ const initialState = { term: "", order: "id", orderDir: 1 - }, - currentSelectionPlanState: { - entity: { id: 0, name: "" }, - errors: {} } }; @@ -94,39 +66,37 @@ describe("SelectionPlanListPage", () => { beforeEach(() => { jest.clearAllMocks(); getSelectionPlans.mockReturnValue(() => Promise.resolve()); - getSelectionPlan.mockReturnValue(() => Promise.resolve()); deleteSelectionPlan.mockReturnValue(() => Promise.resolve()); - resetSelectionPlanForm.mockReturnValue({ - type: "RESET_SELECTION_PLAN_FORM" - }); - getMarketingSettingsBySelectionPlan.mockReturnValue(() => - Promise.resolve() - ); - saveSelectionPlan.mockReturnValue(() => Promise.resolve({ id: 1 })); - saveSelectionPlanSettings.mockReturnValue(() => Promise.resolve()); }); - it("reloads the list after a successful save", async () => { + it("navigates to the new selection plan route", async () => { renderWithRedux( , { initialState } ); - // Open dialog await userEvent.click( screen.getByRole("button", { name: "selection_plan_list.add_selection_plan" }) ); - expect(screen.getByTestId("edit-selection-plan")).toBeInTheDocument(); - await act(async () => { - await userEvent.click(screen.getByRole("button", { name: "popup-save" })); - await flushPromises(); - }); + expect(mockHistory.push).toHaveBeenCalledWith( + "/app/summits/1/selection-plans/new" + ); + }); - // Call 1: useEffect on mount; call 2: handleSave → refreshSelectionPlans - expect(getSelectionPlans).toHaveBeenCalledTimes(2); + it("navigates to the selection plan edit route", async () => { + renderWithRedux( + , + { initialState } + ); + + await userEvent.click(screen.getByRole("button", { name: "edit-row" })); + + expect(mockHistory.push).toHaveBeenCalledWith( + "/app/summits/1/selection-plans/1" + ); }); it("reloads the list after a successful delete", async () => { diff --git a/src/pages/selection-plans/edit-selection-plan-page.js b/src/pages/selection-plans/edit-selection-plan-page.js index 7eb80ef62..fc5b09856 100644 --- a/src/pages/selection-plans/edit-selection-plan-page.js +++ b/src/pages/selection-plans/edit-selection-plan-page.js @@ -10,10 +10,12 @@ * See the License for the specific language governing permissions and * limitations under the License. * */ -import React from "react"; +import React, { useState } from "react"; import { connect } from "react-redux"; import T from "i18n-react/dist/i18n-react"; import Swal from "sweetalert2"; +import { Button, Grid2 } from "@mui/material"; +import AddIcon from "@mui/icons-material/Add"; import SelectionPlanForm from "../../components/forms/selection-plan-form"; import { addAllowedMemberToSelectionPlan, @@ -28,6 +30,8 @@ import { importAllowedMembersCSV, removeAllowedMemberFromSelectionPlan, removeTrackGroupFromSelectionPlan, + saveSelectionPlan, + saveSelectionPlanSettings, unassignProgressFlagFromSelectionPlan, updateProgressFlagOrder, updateRatingTypeOrder, @@ -39,10 +43,11 @@ const EditSelectionPlanPage = ({ entity, allowedMembers, errors, - onSave, history, extraQuestionsOrder, extraQuestionsOrderDir, + saveSelectionPlan, + saveSelectionPlanSettings, updateSelectionPlanExtraQuestionOrder, unassignProgressFlagFromSelectionPlan, deleteSelectionPlanExtraQuestion, @@ -60,6 +65,31 @@ const EditSelectionPlanPage = ({ importAllowedMembersCSV, removeAllowedMemberFromSelectionPlan }) => { + const [isSaving, setIsSaving] = useState(false); + + const onSave = (values) => { + if (isSaving) return Promise.resolve(); + setIsSaving(true); + return saveSelectionPlan(values) + .then((savedEntity) => { + if (!savedEntity?.id) return null; + return saveSelectionPlanSettings( + values.marketing_settings ?? {}, + savedEntity.id + ) + .catch(() => {}) + .then(() => { + if (!values.id) { + history.push( + `/app/summits/${currentSummit.id}/selection-plans/${savedEntity.id}` + ); + } + }); + }) + .catch(() => {}) + .finally(() => setIsSaving(false)); + }; + const onDeleteExtraQuestion = (questionId) => { const extraQuestion = entity.extra_questions.find( (t) => t.id === questionId @@ -175,38 +205,83 @@ const EditSelectionPlanPage = ({ }); }; + const title = entity?.id + ? T.translate("general.edit") + : T.translate("general.add"); + return ( - +
+ + +

+ {title} {T.translate("edit_selection_plan.selection_plan")} +

+ + {entity?.id > 0 && ( + + + + )} + +
+ + + + +
); }; @@ -219,6 +294,8 @@ const mapStateToProps = ({ }); export default connect(mapStateToProps, { + saveSelectionPlan, + saveSelectionPlanSettings, addTrackGroupToSelectionPlan, removeTrackGroupFromSelectionPlan, addEventTypeSelectionPlan, diff --git a/src/pages/selection-plans/selection-plan-list-page.js b/src/pages/selection-plans/selection-plan-list-page.js index d6aaa79ce..64eaa2e57 100644 --- a/src/pages/selection-plans/selection-plan-list-page.js +++ b/src/pages/selection-plans/selection-plan-list-page.js @@ -11,7 +11,7 @@ * limitations under the License. * */ -import React, { useCallback, useEffect, useState } from "react"; +import React, { useEffect } from "react"; import { connect } from "react-redux"; import T from "i18n-react/dist/i18n-react"; import Box from "@mui/material/Box"; @@ -21,55 +21,23 @@ import MuiTable from "openstack-uicore-foundation/lib/components/mui/table"; import GridToolbar from "../../components/mui/grid-toolbar"; import { deleteSelectionPlan, - getSelectionPlan, - getSelectionPlans, - resetSelectionPlanForm, - saveSelectionPlan, - saveSelectionPlanSettings + getSelectionPlans } from "../../actions/selection-plan-actions"; -import { getMarketingSettingsBySelectionPlan } from "../../actions/marketing-actions"; -import { DEFAULT_CURRENT_PAGE, MAX_PER_PAGE } from "../../utils/constants"; -import SelectionPlanPopup from "./selection-plan-popup"; +import { DEFAULT_CURRENT_PAGE } from "../../utils/constants"; const SelectionPlanListPage = ({ currentSummit, history, selectionPlans, - currentSelectionPlan, totalSelectionPlans, perPage, term, order, orderDir, currentPage, - getSelectionPlan, getSelectionPlans, - resetSelectionPlanForm, - getMarketingSettingsBySelectionPlan, - deleteSelectionPlan, - saveSelectionPlan, - saveSelectionPlanSettings + deleteSelectionPlan }) => { - const [openSelectionPlanPopup, setOpenSelectionPlanPopup] = useState(false); - - const openEditModal = useCallback( - (selectionPlanId) => { - if (!selectionPlanId) return; - - getSelectionPlan(selectionPlanId) - .then(() => - getMarketingSettingsBySelectionPlan( - selectionPlanId, - null, - DEFAULT_CURRENT_PAGE, - MAX_PER_PAGE - ) - ) - .then(() => setOpenSelectionPlanPopup(true)); - }, - [getMarketingSettingsBySelectionPlan, getSelectionPlan] - ); - useEffect(() => { if (currentSummit?.id) { getSelectionPlans(term, DEFAULT_CURRENT_PAGE, perPage, order, orderDir); @@ -81,7 +49,9 @@ const SelectionPlanListPage = ({ const handleEdit = (selectionPlan) => { if (!selectionPlan?.id) return; - openEditModal(selectionPlan.id); + history.push( + `/app/summits/${currentSummit.id}/selection-plans/${selectionPlan.id}` + ); }; const handleDelete = (id) => { @@ -93,26 +63,9 @@ const SelectionPlanListPage = ({ }; const handleNew = () => { - resetSelectionPlanForm(); - setOpenSelectionPlanPopup(true); + history.push(`/app/summits/${currentSummit.id}/selection-plans/new`); }; - const handleClosePopup = () => { - resetSelectionPlanForm(); - setOpenSelectionPlanPopup(false); - }; - - const handleSave = (entity) => - saveSelectionPlan(entity) - .then((savedEntity) => { - if (!savedEntity?.id) return null; - return saveSelectionPlanSettings( - entity.marketing_settings ?? {}, - savedEntity.id - ); - }) - .then(() => refreshSelectionPlans()); - const handleSort = (key, dir) => { getSelectionPlans(term, currentPage, perPage, key, dir); }; @@ -209,35 +162,19 @@ const SelectionPlanListPage = ({ />
)} - - {openSelectionPlanPopup && ( - - )}
); }; const mapStateToProps = ({ currentSummitState, - currentSelectionPlanListState, - currentSelectionPlanState + currentSelectionPlanListState }) => ({ currentSummit: currentSummitState.currentSummit, - ...currentSelectionPlanListState, - currentSelectionPlan: currentSelectionPlanState.entity + ...currentSelectionPlanListState }); export default connect(mapStateToProps, { getSelectionPlans, - getSelectionPlan, - resetSelectionPlanForm, - getMarketingSettingsBySelectionPlan, - deleteSelectionPlan, - saveSelectionPlan, - saveSelectionPlanSettings + deleteSelectionPlan })(SelectionPlanListPage); diff --git a/src/pages/selection-plans/selection-plan-popup.js b/src/pages/selection-plans/selection-plan-popup.js deleted file mode 100644 index 879b97290..000000000 --- a/src/pages/selection-plans/selection-plan-popup.js +++ /dev/null @@ -1,98 +0,0 @@ -/** - * Copyright 2019 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, { useState } from "react"; -import PropTypes from "prop-types"; -import T from "i18n-react/dist/i18n-react"; -import Button from "@mui/material/Button"; -import Dialog from "@mui/material/Dialog"; -import DialogActions from "@mui/material/DialogActions"; -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 EditSelectionPlanPage from "./edit-selection-plan-page"; - -const SelectionPlanPopup = ({ isEditing, onClose, onSave, history }) => { - const [isSaving, setIsSaving] = useState(false); - - const handleClose = () => { - if (isSaving) return; - onClose(); - }; - - const handleSave = (values) => { - if (isSaving) return Promise.resolve(); - setIsSaving(true); - return Promise.resolve(onSave(values)) - .then(() => onClose()) - .catch(() => {}) - .finally(() => setIsSaving(false)); - }; - - return ( - - - {isEditing ? T.translate("general.edit") : T.translate("general.add")}{" "} - {T.translate("edit_selection_plan.selection_plan")} - - - - - - - - - - - - - - ); -}; - -SelectionPlanPopup.propTypes = { - isEditing: PropTypes.bool, - onClose: PropTypes.func.isRequired, - onSave: PropTypes.func.isRequired, - history: PropTypes.shape({ push: PropTypes.func }).isRequired -}; - -SelectionPlanPopup.defaultProps = { - isEditing: false -}; - -export default SelectionPlanPopup;