fix: add isSaving to state, guard submit and catch errors

src/actions/__tests__/badge-actions.test.js

@@ -0,0 +1,86 @@
+/**
+ * @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
+      .mockImplementationOnce(() => () => Promise.resolve({ id: "first" }))
+      .mockImplementationOnce(() => () => Promise.resolve({ id: "second" }));
+    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.toEqual([{ id: "first" }, { id: "second" }]);
+  });
+});

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  *********************************************** */

src/components/forms/__tests__/badge-settings-form.test.js

@@ -0,0 +1,109 @@
+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", () => ({
+  __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(
+    <BadgeSettingsForm
+      entity={{}}
+      currentSummit={mockSummit}
+      errors={{}}
+      onSubmit={onSubmit}
+      onDeleteImage={jest.fn()}
+      onDeleteBadgeTypeImage={jest.fn()}
+    />
+  );
+
+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();
+  });
+});
+
+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");
+});
+
+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();
+});

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);
@@ -40,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);
@@ -159,26 +168,38 @@ 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(
         ([, values]) => values.updated === true
       )
     );
 
-    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 });
+
+    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);
-    });
+          Swal.fire(success_message);
+        },
+        () => {} // only swallows onSubmit's own rejection
+      )
+      .finally(() => {
+        if (this._isMounted) this.setState({ isSaving: false });
+      });
   }
 
   render() {
-    const { entity, showSection } = this.state;
+    const { entity, showSection, isSaving } = this.state;
     const { currentSummit } = this.props;
 
     const ddlAlignOptions = [
@@ -1580,6 +1601,7 @@ class BadgeSettingsForm extends React.Component {
               onClick={this.handleSubmit}
               className="btn btn-primary pull-right"
               value={T.translate("general.save")}
+              disabled={isSaving}
             />
           </div>
         </div>