From a473878bef5b0efa6d523542165419725e627d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Castillo?= Date: Wed, 15 Jul 2026 01:14:19 -0300 Subject: [PATCH 1/5] fix: add isSaving to state, guard submit and catch errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Castillo --- .../__tests__/badge-settings-form.test.js | 61 +++++++++++++++++++ src/components/forms/badge-settings-form.js | 40 +++++++----- 2 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 src/components/forms/__tests__/badge-settings-form.test.js diff --git a/src/components/forms/__tests__/badge-settings-form.test.js b/src/components/forms/__tests__/badge-settings-form.test.js new file mode 100644 index 000000000..c6e5ae19d --- /dev/null +++ b/src/components/forms/__tests__/badge-settings-form.test.js @@ -0,0 +1,61 @@ +import React from "react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import "@testing-library/jest-dom"; +import BadgeSettingsForm from "../badge-settings-form"; + +jest.mock("i18n-react/dist/i18n-react", () => ({ + __esModule: true, + default: { translate: (key) => key } +})); + +jest.mock("sweetalert2", () => ({ + __esModule: true, + default: { fire: jest.fn() } +})); + +const mockSummit = { id: 1, badge_features_types: [], badge_types: [] }; + +const renderForm = (onSubmit) => + render( + + ); + +it("should call onSubmit only once when Save is clicked twice while saving", async () => { + const pendingPromise = new Promise(() => {}); + const onSubmit = jest.fn(() => pendingPromise); + const { container } = renderForm(onSubmit); + + fireEvent.change(container.querySelector("#BADGE_TEMPLATE_WIDTH"), { + target: { value: "100" } + }); + + const saveButton = screen.getByRole("button", { name: "general.save" }); + fireEvent.click(saveButton); + fireEvent.click(saveButton); + + expect(onSubmit).toHaveBeenCalledTimes(1); +}); + +it("should re-enable Save and not throw an unhandled rejection when onSubmit rejects", async () => { + const onSubmit = jest.fn(() => Promise.reject(new Error("412"))); + const { container } = renderForm(onSubmit); + + fireEvent.change(container.querySelector("#BADGE_TEMPLATE_WIDTH"), { + target: { value: "100" } + }); + + fireEvent.click(screen.getByRole("button", { name: "general.save" })); + + await waitFor(() => { + expect( + screen.getByRole("button", { name: "general.save" }) + ).not.toBeDisabled(); + }); +}); diff --git a/src/components/forms/badge-settings-form.js b/src/components/forms/badge-settings-form.js index 27a190999..d9c88eff4 100644 --- a/src/components/forms/badge-settings-form.js +++ b/src/components/forms/badge-settings-form.js @@ -12,10 +12,10 @@ * */ import React from "react"; import T from "i18n-react/dist/i18n-react"; -import UploadInput from "openstack-uicore-foundation/lib/components/inputs/upload-input" -import Input from "openstack-uicore-foundation/lib/components/inputs/text-input" -import TextArea from "openstack-uicore-foundation/lib/components/inputs/textarea-input" -import Panel from "openstack-uicore-foundation/lib/components/sections/panel" +import UploadInput from "openstack-uicore-foundation/lib/components/inputs/upload-input"; +import Input from "openstack-uicore-foundation/lib/components/inputs/text-input"; +import TextArea from "openstack-uicore-foundation/lib/components/inputs/textarea-input"; +import Panel from "openstack-uicore-foundation/lib/components/sections/panel"; import Dropdown from "openstack-uicore-foundation/lib/components/inputs/dropdown"; import Switch from "react-switch"; import Swal from "sweetalert2"; @@ -30,7 +30,8 @@ class BadgeSettingsForm extends React.Component { this.state = { entity: { ...props.entity }, errors: props.errors, - showSection: null + showSection: null, + isSaving: false }; this.handleChange = this.handleChange.bind(this); @@ -159,6 +160,8 @@ class BadgeSettingsForm extends React.Component { handleSubmit(ev) { ev.preventDefault(); + if (this.state.isSaving) return; + // save only the settings with the following conditions const settingsToSave = Object.fromEntries( Object.entries(this.state.entity).filter( @@ -166,19 +169,27 @@ class BadgeSettingsForm extends React.Component { ) ); - this.props.onSubmit(settingsToSave).then(() => { - const success_message = { - title: T.translate("general.done"), - html: T.translate("badge_settings.badge_template_settings_updated"), - type: "success" - }; + this.setState({ isSaving: true }); - Swal.fire(success_message); - }); + this.props + .onSubmit(settingsToSave) + .then(() => { + const success_message = { + title: T.translate("general.done"), + html: T.translate("badge_settings.badge_template_settings_updated"), + type: "success" + }; + + Swal.fire(success_message); + }) + .catch(() => {}) + .finally(() => { + this.setState({ isSaving: false }); + }); } render() { - const { entity, showSection } = this.state; + const { entity, showSection, isSaving } = this.state; const { currentSummit } = this.props; const ddlAlignOptions = [ @@ -1580,6 +1591,7 @@ class BadgeSettingsForm extends React.Component { onClick={this.handleSubmit} className="btn btn-primary pull-right" value={T.translate("general.save")} + disabled={isSaving} /> From bd3b6d9bef2bf14689ad3cf4b241bc1e6f4c4f5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Castillo?= Date: Sat, 1 Aug 2026 03:13:28 -0300 Subject: [PATCH 2/5] fix: adjust catch no not swallow errors, add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Castillo --- .../__tests__/badge-settings-form.test.js | 26 +++++++++++++++++++ src/components/forms/badge-settings-form.js | 22 +++++++++------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/components/forms/__tests__/badge-settings-form.test.js b/src/components/forms/__tests__/badge-settings-form.test.js index c6e5ae19d..00955d4d5 100644 --- a/src/components/forms/__tests__/badge-settings-form.test.js +++ b/src/components/forms/__tests__/badge-settings-form.test.js @@ -1,6 +1,7 @@ import React from "react"; import { render, screen, fireEvent, waitFor } from "@testing-library/react"; import "@testing-library/jest-dom"; +import Swal from "sweetalert2"; import BadgeSettingsForm from "../badge-settings-form"; jest.mock("i18n-react/dist/i18n-react", () => ({ @@ -59,3 +60,28 @@ it("should re-enable Save and not throw an unhandled rejection when onSubmit rej ).not.toBeDisabled(); }); }); + +it("should not let a success-handler error be swallowed by onSubmit's rejection handler", () => { + const then = jest.fn(() => ({ finally: jest.fn() })); + const onSubmit = jest.fn(() => ({ then })); + const { container } = renderForm(onSubmit); + + fireEvent.change(container.querySelector("#BADGE_TEMPLATE_WIDTH"), { + target: { value: "100" } + }); + fireEvent.click(screen.getByRole("button", { name: "general.save" })); + + // .then must receive two distinct handlers - a single-argument + // .then(success).catch(fail) would let fail also catch success's own errors. + expect(then).toHaveBeenCalledWith(expect.any(Function), expect.any(Function)); + const [onSuccess, onRejected] = then.mock.calls[0]; + expect(onSuccess).not.toBe(onRejected); + + Swal.fire.mockImplementationOnce(() => { + throw new Error("Swal render error"); + }); + + // invoking the success handler directly proves its own error is not + // pre-caught before it would reach onRejected + expect(onSuccess).toThrow("Swal render error"); +}); diff --git a/src/components/forms/badge-settings-form.js b/src/components/forms/badge-settings-form.js index d9c88eff4..4185b288a 100644 --- a/src/components/forms/badge-settings-form.js +++ b/src/components/forms/badge-settings-form.js @@ -173,16 +173,18 @@ class BadgeSettingsForm extends React.Component { this.props .onSubmit(settingsToSave) - .then(() => { - const success_message = { - title: T.translate("general.done"), - html: T.translate("badge_settings.badge_template_settings_updated"), - type: "success" - }; - - Swal.fire(success_message); - }) - .catch(() => {}) + .then( + () => { + const success_message = { + title: T.translate("general.done"), + html: T.translate("badge_settings.badge_template_settings_updated"), + type: "success" + }; + + Swal.fire(success_message); + }, + () => {} // only swallows onSubmit's own rejection + ) .finally(() => { this.setState({ isSaving: false }); }); From 727eda13e548e96d11fb44684a767ef4b83c8ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Castillo?= Date: Mon, 3 Aug 2026 17:50:12 -0300 Subject: [PATCH 3/5] fix: badge settings promise allSettled, avoid state change when unmounted, add test for success path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Castillo --- src/actions/badge-actions.js | 7 ++++-- .../__tests__/badge-settings-form.test.js | 22 +++++++++++++++++++ src/components/forms/badge-settings-form.js | 10 ++++++++- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/actions/badge-actions.js b/src/actions/badge-actions.js index 5d3ef093e..bb8226030 100644 --- a/src/actions/badge-actions.js +++ b/src/actions/badge-actions.js @@ -30,7 +30,7 @@ import { } from "openstack-uicore-foundation/lib/utils/actions"; import URI from "urijs"; import pLimit from "p-limit"; -import debounce from "lodash/debounce" +import debounce from "lodash/debounce"; import history from "../history"; import { saveMarketingSetting } from "./marketing-actions"; import { getAccessTokenSafely } from "../utils/methods"; @@ -150,7 +150,10 @@ export const saveBadgeSettings = (badgeSettings) => async (dispatch) => { }) ); - return Promise.all(input); + const results = await Promise.allSettled(input); + const failed = results.find((r) => r.status === "rejected"); + if (failed) throw failed.reason; + return results.map((r) => r.value); }; /** ********************* BADGE *********************************************** */ diff --git a/src/components/forms/__tests__/badge-settings-form.test.js b/src/components/forms/__tests__/badge-settings-form.test.js index 00955d4d5..0986466e8 100644 --- a/src/components/forms/__tests__/badge-settings-form.test.js +++ b/src/components/forms/__tests__/badge-settings-form.test.js @@ -85,3 +85,25 @@ it("should not let a success-handler error be swallowed by onSubmit's rejection // pre-caught before it would reach onRejected expect(onSuccess).toThrow("Swal render error"); }); + +it("should show the success message and re-enable Save when onSubmit resolves", async () => { + const onSubmit = jest.fn(() => Promise.resolve()); + const { container } = renderForm(onSubmit); + + fireEvent.change(container.querySelector("#BADGE_TEMPLATE_WIDTH"), { + target: { value: "100" } + }); + fireEvent.click(screen.getByRole("button", { name: "general.save" })); + + await waitFor(() => + expect(Swal.fire).toHaveBeenCalledWith( + expect.objectContaining({ + html: "badge_settings.badge_template_settings_updated", + type: "success" + }) + ) + ); + expect( + screen.getByRole("button", { name: "general.save" }) + ).not.toBeDisabled(); +}); diff --git a/src/components/forms/badge-settings-form.js b/src/components/forms/badge-settings-form.js index 4185b288a..0ce9cb4da 100644 --- a/src/components/forms/badge-settings-form.js +++ b/src/components/forms/badge-settings-form.js @@ -41,6 +41,14 @@ class BadgeSettingsForm extends React.Component { this.handleRemoveBadgeTypeFile = this.handleRemoveBadgeTypeFile.bind(this); } + componentDidMount() { + this._isMounted = true; + } + + componentWillUnmount() { + this._isMounted = false; + } + componentDidUpdate(prevProps) { const state = {}; scrollToError(this.props.errors); @@ -186,7 +194,7 @@ class BadgeSettingsForm extends React.Component { () => {} // only swallows onSubmit's own rejection ) .finally(() => { - this.setState({ isSaving: false }); + if (this._isMounted) this.setState({ isSaving: false }); }); } From dc08235e95c7e5cefce9a157d10180f80d9cca8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Castillo?= Date: Mon, 3 Aug 2026 18:14:56 -0300 Subject: [PATCH 4/5] fix: add unit test to check saveBadgeSettings with new promise allSettled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Castillo --- src/actions/__tests__/badge-actions.test.js | 85 +++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 src/actions/__tests__/badge-actions.test.js diff --git a/src/actions/__tests__/badge-actions.test.js b/src/actions/__tests__/badge-actions.test.js new file mode 100644 index 000000000..bdb905266 --- /dev/null +++ b/src/actions/__tests__/badge-actions.test.js @@ -0,0 +1,85 @@ +/** + * @jest-environment jsdom + */ +import configureStore from "redux-mock-store"; +import thunk from "redux-thunk"; +import flushPromises from "flush-promises"; +import { saveBadgeSettings } from "../badge-actions"; +import { saveMarketingSetting } from "../marketing-actions"; + +jest.mock("../marketing-actions", () => ({ + __esModule: true, + saveMarketingSetting: jest.fn() +})); + +const deferred = () => { + let resolve; + let reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +describe("saveBadgeSettings", () => { + const middlewares = [thunk]; + const mockStore = configureStore(middlewares); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it("does not settle until every fanned-out setting request has settled, then rejects with the failure", async () => { + const early = deferred(); + const late = deferred(); + + saveMarketingSetting.mockImplementation((entity) => () => { + if (entity.key === "A") return early.promise; + if (entity.key === "B") return late.promise; + return Promise.resolve(); + }); + + const store = mockStore({}); + let settled = false; + const resultPromise = store.dispatch( + saveBadgeSettings({ + a: { id: 1, type: "TEXT", value: "x", updated: true }, + b: { id: 2, type: "TEXT", value: "y", updated: true } + }) + ); + resultPromise.then( + () => { + settled = true; + }, + () => { + settled = true; + } + ); + + early.reject(new Error("early failure")); + await flushPromises(); + + expect(settled).toBe(false); + + late.resolve({ response: {} }); + await flushPromises(); + + expect(settled).toBe(true); + await expect(resultPromise).rejects.toThrow("early failure"); + }); + + it("resolves once every setting request resolves", async () => { + saveMarketingSetting.mockImplementation(() => () => Promise.resolve()); + + const store = mockStore({}); + await expect( + store.dispatch( + saveBadgeSettings({ + a: { id: 1, type: "TEXT", value: "x", updated: true }, + b: { id: 2, type: "TEXT", value: "y", updated: true } + }) + ) + ).resolves.toBeDefined(); + }); +}); From af7f97755307050962521749e528367c597f82f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Castillo?= Date: Mon, 3 Aug 2026 18:20:56 -0300 Subject: [PATCH 5/5] fix: adjust test mock promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Castillo --- src/actions/__tests__/badge-actions.test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/actions/__tests__/badge-actions.test.js b/src/actions/__tests__/badge-actions.test.js index bdb905266..81c777301 100644 --- a/src/actions/__tests__/badge-actions.test.js +++ b/src/actions/__tests__/badge-actions.test.js @@ -70,8 +70,9 @@ describe("saveBadgeSettings", () => { }); it("resolves once every setting request resolves", async () => { - saveMarketingSetting.mockImplementation(() => () => Promise.resolve()); - + saveMarketingSetting + .mockImplementationOnce(() => () => Promise.resolve({ id: "first" })) + .mockImplementationOnce(() => () => Promise.resolve({ id: "second" })); const store = mockStore({}); await expect( store.dispatch( @@ -80,6 +81,6 @@ describe("saveBadgeSettings", () => { b: { id: 2, type: "TEXT", value: "y", updated: true } }) ) - ).resolves.toBeDefined(); + ).resolves.toEqual([{ id: "first" }, { id: "second" }]); }); });