fix(ui): rewrite SilenceSubmitProgress with hooks

ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js

@@ -1,8 +1,7 @@
-import React, { Component } from "react";
+import React, { useEffect, useCallback } from "react";
 import PropTypes from "prop-types";
 
-import { action, observable } from "mobx";
-import { observer } from "mobx-react";
+import { useObserver, useLocalStore } from "mobx-react";
 
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
 import { faCircleNotch } from "@fortawesome/free-solid-svg-icons/faCircleNotch";
@@ -19,17 +18,17 @@ const SubmitState = Object.freeze({
   Failed: "Failed",
 });
 
-const SubmitIcon = observer(({ stateValue }) => {
-  if (stateValue === SubmitState.Done) {
-    return <FontAwesomeIcon icon={faCheckCircle} className="text-success" />;
-  }
-  if (stateValue === SubmitState.Failed) {
-    return (
+const SubmitIcon = ({ stateValue }) => {
+  return useObserver(() =>
+    stateValue === SubmitState.Done ? (
+      <FontAwesomeIcon icon={faCheckCircle} className="text-success" />
+    ) : stateValue === SubmitState.Failed ? (
       <FontAwesomeIcon icon={faExclamationCircle} className="text-danger" />
-    );
-  }
-  return <FontAwesomeIcon icon={faCircleNotch} spin />;
-});
+    ) : (
+      <FontAwesomeIcon icon={faCircleNotch} spin />
+    )
+  );
+};
 
 const SilenceLink = ({ uri, silenceID }) => (
   <a
@@ -45,126 +44,118 @@ SilenceLink.propTypes = {
   silenceID: PropTypes.string.isRequired,
 };
 
-const SilenceSubmitProgress = observer(
-  class SilenceSubmitProgress extends Component {
-    static propTypes = {
-      cluster: PropTypes.string.isRequired,
-      members: PropTypes.arrayOf(PropTypes.string).isRequired,
-      payload: PropTypes.exact({
-        matchers: PropTypes.arrayOf(APISilenceMatcher).isRequired,
-        startsAt: PropTypes.string.isRequired,
-        endsAt: PropTypes.string.isRequired,
-        createdBy: PropTypes.string.isRequired,
-        comment: PropTypes.string.isRequired,
-        id: PropTypes.string,
-      }).isRequired,
-      alertStore: PropTypes.instanceOf(AlertStore).isRequired,
-    };
+const SilenceSubmitProgress = ({ alertStore, cluster, members, payload }) => {
+  const submitState = useLocalStore(() => ({
+    membersToTry: [],
+    value: SubmitState.InProgress,
+    result: null,
+    markDone(result) {
+      this.result = result;
+      this.value = SubmitState.Done;
+    },
+    markFailed(result) {
+      this.result = result;
+      this.value = SubmitState.Failed;
+    },
+  }));
 
-    submitState = observable(
-      {
-        // store fetch result here, useful for testing
-        fetch: null,
-        membersToTry: [],
-        value: SubmitState.InProgress,
-        result: null,
-        markDone(result) {
-          this.result = result;
-          this.value = SubmitState.Done;
-        },
-        markFailed(result) {
-          this.result = result;
-          this.value = SubmitState.Failed;
-        },
-      },
-      { markDone: action.bound, markFailed: action.bound }
-    );
+  const handleAlertmanagerRequest = useCallback(() => {
+    const member = submitState.membersToTry.pop();
 
-    maybeTryAgainAfterError = (err) => {
-      if (this.submitState.membersToTry.length) {
-        return this.handleAlertmanagerRequest();
+    if (alertStore.data.isReadOnlyAlertmanager(member)) {
+      const err = `Alertmanager instance "${member}" is read-only`;
+      console.error(err);
+      if (submitState.membersToTry.length) {
+        return handleAlertmanagerRequest();
       } else {
-        this.submitState.markFailed(err.message);
+        submitState.markFailed(err.message);
       }
-    };
+      return;
+    }
 
-    handleAlertmanagerRequest = () => {
-      const { payload, alertStore } = this.props;
-
-      const member = this.submitState.membersToTry.pop();
-
-      if (alertStore.data.isReadOnlyAlertmanager(member)) {
-        const err = `Alertmanager instance "${member}" is read-only`;
-        console.error(err);
-        this.maybeTryAgainAfterError(err);
-        return;
+    const am = alertStore.data.getAlertmanagerByName(member);
+    if (am === undefined) {
+      const err = `Alertmanager instance "${member}" not found`;
+      console.error(err);
+      if (submitState.membersToTry.length) {
+        return handleAlertmanagerRequest();
+      } else {
+        submitState.markFailed(err.message);
       }
+      return;
+    }
 
-      const am = alertStore.data.getAlertmanagerByName(member);
-      if (am === undefined) {
-        const err = `Alertmanager instance "${member}" not found`;
-        console.error(err);
-        return this.maybeTryAgainAfterError(err);
-      }
-
-      this.submitState.fetch = FetchPost(`${am.uri}/api/v2/silences`, {
-        body: JSON.stringify(payload),
-        credentials: am.corsCredentials,
-        headers: {
-          "Content-Type": "application/json",
-          ...am.headers,
-        },
-      })
-        .then((result) => {
-          if (result.ok) {
-            return result
-              .json()
-              .then((r) => this.parseOpenAPIResponse(am.publicURI, r));
-          } else {
-            return result.text().then((text) => {
-              this.submitState.markFailed(text);
-              return text;
-            });
-          }
-        })
-        .catch(this.maybeTryAgainAfterError);
-    };
-
-    parseOpenAPIResponse = (uri, response) => {
+    const parseOpenAPIResponse = (uri, response) => {
       const link = <SilenceLink uri={uri} silenceID={response.silenceID} />;
-      this.submitState.markDone(link);
+      submitState.markDone(link);
       // return silenceID so we can assert it in tests
       return response.silenceID;
     };
 
-    componentDidMount() {
-      const { members } = this.props;
-      this.submitState.membersToTry = [...members];
-      this.handleAlertmanagerRequest();
-    }
+    FetchPost(`${am.uri}/api/v2/silences`, {
+      body: JSON.stringify(payload),
+      credentials: am.corsCredentials,
+      headers: {
+        "Content-Type": "application/json",
+        ...am.headers,
+      },
+    })
+      .then((result) => {
+        if (result.ok) {
+          return result
+            .json()
+            .then((r) => parseOpenAPIResponse(am.publicURI, r));
+        } else {
+          return result.text().then((text) => {
+            submitState.markFailed(text);
+            return text;
+          });
+        }
+      })
+      .catch((err) => {
+        if (submitState.membersToTry.length) {
+          return handleAlertmanagerRequest();
+        } else {
+          submitState.markFailed(err.message);
+        }
+      });
+  }, [alertStore.data, payload, submitState]);
 
-    render() {
-      const { cluster } = this.props;
+  useEffect(() => {
+    submitState.membersToTry = [...members];
+    handleAlertmanagerRequest();
+  }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
-      return (
-        <div className="d-flex mb-2">
-          <div className="p-2 flex-fill my-auto flex-grow-0 flex-shrink-0">
-            <SubmitIcon stateValue={this.submitState.value} />
-          </div>
-          <div className="p-2 mr-1 flex-fill my-auto flex-grow-0 flex-shrink-0">
-            {cluster}
-          </div>
-          <div
-            className={`p-2 flex-fill flex-grow-1 flex-shrink-1 rounded text-center ${
-              this.submitState.value === SubmitState.Failed ? "bg-light" : ""
-            }`}
-          >
-            {this.submitState.result}
-          </div>
-        </div>
-      );
-    }
-  }
-);
+  return useObserver(() => (
+    <div className="d-flex mb-2">
+      <div className="p-2 flex-fill my-auto flex-grow-0 flex-shrink-0">
+        <SubmitIcon stateValue={submitState.value} />
+      </div>
+      <div className="p-2 mr-1 flex-fill my-auto flex-grow-0 flex-shrink-0">
+        {cluster}
+      </div>
+      <div
+        className={`p-2 flex-fill flex-grow-1 flex-shrink-1 rounded text-center ${
+          submitState.value === SubmitState.Failed ? "bg-light" : ""
+        }`}
+      >
+        {submitState.result}
+      </div>
+    </div>
+  ));
+};
+SilenceSubmitProgress.propTypes = {
+  cluster: PropTypes.string.isRequired,
+  members: PropTypes.arrayOf(PropTypes.string).isRequired,
+  payload: PropTypes.exact({
+    matchers: PropTypes.arrayOf(APISilenceMatcher).isRequired,
+    startsAt: PropTypes.string.isRequired,
+    endsAt: PropTypes.string.isRequired,
+    createdBy: PropTypes.string.isRequired,
+    comment: PropTypes.string.isRequired,
+    id: PropTypes.string,
+  }).isRequired,
+  alertStore: PropTypes.instanceOf(AlertStore).isRequired,
+};
 
 export { SilenceSubmitProgress };

ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js

@@ -2,6 +2,8 @@ import React from "react";
 
 import { mount } from "enzyme";
 
+import fetchMock from "fetch-mock";
+
 import { AlertStore } from "Stores/AlertStore";
 import { SilenceSubmitProgress } from "./SilenceSubmitProgress";
 
@@ -26,12 +28,21 @@ beforeEach(() => {
     ],
   };
 
-  fetch.resetMocks();
+  fetchMock.resetHistory();
+  fetchMock.any(
+    {
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify({ silenceID: "123456789" }),
+    },
+    {
+      overwriteRoutes: true,
+    }
+  );
 });
 
 afterEach(() => {
   jest.restoreAllMocks();
-  fetch.resetMocks();
+  fetchMock.resetHistory();
 });
 
 const MountedSilenceSubmitProgress = () => {
@@ -53,21 +64,22 @@ const MountedSilenceSubmitProgress = () => {
 
 describe("<SilenceSubmitProgress />", () => {
   it("sends a request on mount", async () => {
-    const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBeUndefined();
-    expect(fetch.mock.calls).toHaveLength(1);
+    MountedSilenceSubmitProgress();
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()).toHaveLength(1);
   });
 
   it("appends /api/v2/silences to the passed URI", async () => {
-    const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBeUndefined();
-    const uri = fetch.mock.calls[0][0];
+    MountedSilenceSubmitProgress();
+    await fetchMock.flush(true);
+    const uri = fetchMock.calls()[0][0];
     expect(uri).toBe("http://localhost/api/v2/silences");
   });
 
-  it("sends correct JSON payload", () => {
+  it("sends correct JSON payload", async () => {
     MountedSilenceSubmitProgress();
-    const payload = fetch.mock.calls[0][1];
+    await fetchMock.flush(true);
+    const payload = fetchMock.calls()[0][1];
     expect(payload).toMatchObject({
       method: "POST",
       headers: { "Content-Type": "application/json", foo: "bar" },
@@ -84,17 +96,85 @@ describe("<SilenceSubmitProgress />", () => {
   it("uses CORS credentials from alertmanager config", async () => {
     alertStore.data.upstreams.instances[0].corsCredentials = "same-origin";
     MountedSilenceSubmitProgress();
-    expect(fetch.mock.calls[0][0]).toBe("http://localhost/api/v2/silences");
-    expect(fetch.mock.calls[0][1]).toMatchObject({
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()[0][0]).toBe("http://localhost/api/v2/silences");
+    expect(fetchMock.calls()[0][1]).toMatchObject({
       credentials: "same-origin",
       method: "POST",
     });
   });
 
   it("will retry on another cluster member after fetch failure", async () => {
-    fetch
-      .mockRejectOnce(new Error("mock error message"))
-      .mockResponseOnce(JSON.stringify({ silenceID: "123456789" }));
+    fetchMock.reset();
+    fetchMock.mock("http://am2.example.com/api/v2/silences", {
+      throws: new TypeError("failed to fetch"),
+    });
+    fetchMock.mock("http://am1.example.com/api/v2/silences", {
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify({ silenceID: "123456789" }),
+    });
+    alertStore.data.upstreams = {
+      clusters: { ha: ["am1", "am2"] },
+      instances: [
+        {
+          name: "am1",
+          uri: "http://am1.example.com",
+          publicURI: "http://am1.example.com",
+          readonly: false,
+          headers: {},
+          corsCredentials: "include",
+          error: "",
+          version: "0.17.0",
+          cluster: "ha",
+          clusterMembers: ["am1", "am2"],
+        },
+        {
+          name: "am2",
+          uri: "http://am2.example.com",
+          publicURI: "http://am2.example.com",
+          readonly: false,
+          headers: {},
+          corsCredentials: "include",
+          error: "",
+          version: "0.17.0",
+          cluster: "ha",
+          clusterMembers: ["am1", "am2"],
+        },
+      ],
+    };
+
+    mount(
+      <SilenceSubmitProgress
+        cluster="ha"
+        members={["am1", "am2"]}
+        payload={{
+          matchers: [],
+          startsAt: "now",
+          endsAt: "later",
+          createdBy: "me@example.com",
+          comment: "fake payload",
+        }}
+        alertStore={alertStore}
+      />
+    );
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()[0][0]).toBe(
+      "http://am2.example.com/api/v2/silences"
+    );
+    expect(fetchMock.calls()).toHaveLength(2);
+    expect(fetchMock.calls()[1][0]).toBe(
+      "http://am1.example.com/api/v2/silences"
+    );
+  });
+
+  it("will render error message from last failed cluster member", async () => {
+    fetchMock.reset();
+    fetchMock.mock("http://am2.example.com/api/v2/silences", {
+      throws: new TypeError("failed to fetch from am2"),
+    });
+    fetchMock.mock("http://am1.example.com/api/v2/silences", {
+      throws: new TypeError("failed to fetch from am1"),
+    });
     alertStore.data.upstreams = {
       clusters: { ha: ["am1", "am2"] },
       instances: [
@@ -139,19 +219,12 @@ describe("<SilenceSubmitProgress />", () => {
         alertStore={alertStore}
       />
     );
-    await expect(tree.instance().submitState.fetch).resolves.toBeUndefined();
-    expect(fetch.mock.calls[0][0]).toBe(
-      "http://am2.example.com/api/v2/silences"
-    );
-    await expect(tree.instance().submitState.fetch).resolves.toBe("123456789");
-    expect(fetch.mock.calls[1][0]).toBe(
-      "http://am1.example.com/api/v2/silences"
-    );
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()).toHaveLength(2);
+    expect(tree.text()).toBe("hafailed to fetch from am1");
   });
 
   it("will log an error if Alertmanager instance is missing from instances and try the next one", async () => {
-    fetch.resetMocks();
-    fetch.mockReject(new Error("mock error message"));
     const consoleSpy = jest
       .spyOn(console, "error")
       .mockImplementation(() => {});
@@ -173,7 +246,7 @@ describe("<SilenceSubmitProgress />", () => {
       ],
     };
 
-    const tree = mount(
+    mount(
       <SilenceSubmitProgress
         cluster="ha"
         members={["am1", "am2"]}
@@ -187,15 +260,42 @@ describe("<SilenceSubmitProgress />", () => {
         alertStore={alertStore}
       />
     );
-    await expect(tree.instance().submitState.fetch).resolves.toBeUndefined();
-    expect(fetch.mock.calls[0][0]).toBe(
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()[0][0]).toBe(
       "http://am1.example.com/api/v2/silences"
     );
     expect(consoleSpy).toHaveBeenCalledTimes(1);
   });
 
+  it("will log an error if all members are missing from instances", async () => {
+    const consoleSpy = jest
+      .spyOn(console, "error")
+      .mockImplementation(() => {});
+    alertStore.data.upstreams = {
+      clusters: { ha: ["am1", "am2"] },
+      instances: [],
+    };
+
+    mount(
+      <SilenceSubmitProgress
+        cluster="ha"
+        members={["am1", "am2"]}
+        payload={{
+          matchers: [],
+          startsAt: "now",
+          endsAt: "later",
+          createdBy: "me@example.com",
+          comment: "fake payload",
+        }}
+        alertStore={alertStore}
+      />
+    );
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()).toHaveLength(0);
+    expect(consoleSpy).toHaveBeenCalledTimes(2);
+  });
+
   it("will refuse to send requests to an alertmanager instance that is readonly", async () => {
-    fetch.resetMocks();
     const logs = [];
     const consoleSpy = jest
       .spyOn(console, "error")
@@ -233,7 +333,7 @@ describe("<SilenceSubmitProgress />", () => {
       ],
     };
 
-    const tree = mount(
+    mount(
       <SilenceSubmitProgress
         cluster="ha"
         members={["am1", "am2"]}
@@ -247,19 +347,78 @@ describe("<SilenceSubmitProgress />", () => {
         alertStore={alertStore}
       />
     );
-    await expect(tree.instance().submitState.fetch).resolves.toBeUndefined();
-    expect(fetch.mock.calls).toHaveLength(1);
-    expect(fetch.mock.calls[0][0]).toBe(
+    await fetchMock.flush(true);
+    expect(fetchMock.calls()).toHaveLength(1);
+    expect(fetchMock.calls()[0][0]).toBe(
       "http://am1.example.com/api/v2/silences"
     );
     expect(logs).toEqual(['Alertmanager instance "am2" is read-only']);
     expect(consoleSpy).toHaveBeenCalledTimes(1);
   });
 
+  it("will log an error if all members are readonly", async () => {
+    const logs = [];
+    const consoleSpy = jest
+      .spyOn(console, "error")
+      .mockImplementation((message, ...args) => {
+        logs.push(message);
+      });
+
+    alertStore.data.upstreams = {
+      clusters: { ha: ["am1", "am2"] },
+      instances: [
+        {
+          name: "am1",
+          uri: "http://am1.example.com",
+          publicURI: "http://am1.example.com",
+          readonly: true,
+          headers: {},
+          corsCredentials: "include",
+          error: "",
+          version: "0.17.0",
+          cluster: "ha",
+          clusterMembers: ["am1", "am2"],
+        },
+        {
+          name: "am2",
+          uri: "http://am2.example.com",
+          publicURI: "http://am2.example.com",
+          readonly: true,
+          headers: {},
+          corsCredentials: "include",
+          error: "",
+          version: "0.17.0",
+          cluster: "ha",
+          clusterMembers: ["am1", "am2"],
+        },
+      ],
+    };
+
+    mount(
+      <SilenceSubmitProgress
+        cluster="ha"
+        members={["am1", "am2"]}
+        payload={{
+          matchers: [],
+          startsAt: "now",
+          endsAt: "later",
+          createdBy: "me@example.com",
+          comment: "fake payload",
+        }}
+        alertStore={alertStore}
+      />
+    );
+    expect(fetchMock.calls()).toHaveLength(0);
+    expect(logs).toEqual([
+      'Alertmanager instance "am2" is read-only',
+      'Alertmanager instance "am1" is read-only',
+    ]);
+    expect(consoleSpy).toHaveBeenCalledTimes(2);
+  });
+
   it("renders returned silence ID on successful fetch", async () => {
-    fetch.mockResponseOnce(JSON.stringify({ silenceID: "123456789" }));
     const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBe("123456789");
+    await fetchMock.flush(true);
     // force re-render
     tree.update();
     const silenceLink = tree.find("a");
@@ -268,39 +427,41 @@ describe("<SilenceSubmitProgress />", () => {
   });
 
   it("renders returned error message on failed fetch", async () => {
-    fetch.mockResponseOnce("mock error message", { status: 500 });
+    fetchMock.reset();
+    fetchMock.any({
+      status: 500,
+      body: "mock error message",
+    });
     const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBe(
-      "mock error message"
-    );
+    await fetchMock.flush(true);
     expect(tree.text()).toBe("mockAlertmanagermock error message");
   });
 
   it("renders success icon on successful fetch", async () => {
-    fetch.mockResponseOnce(JSON.stringify({ silenceID: "123" }));
     const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBe("123");
+    await fetchMock.flush(true);
     tree.update();
     expect(tree.find("FontAwesomeIcon.text-success")).toHaveLength(1);
     expect(tree.find("FontAwesomeIcon.text-danger")).toHaveLength(0);
   });
 
   it("renders silence link on successful fetch", async () => {
-    fetch.mockResponseOnce(JSON.stringify({ silenceID: "123" }));
     const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBe("123");
+    await fetchMock.flush(true);
     tree.update();
     expect(tree.find("a").getDOMNode().getAttribute("href")).toBe(
-      "http://example.com/#/silences/123"
+      "http://example.com/#/silences/123456789"
     );
   });
 
   it("renders error icon on failed fetch", async () => {
-    fetch.mockResponseOnce("error message", { status: 500 });
+    fetchMock.reset();
+    fetchMock.any({
+      status: 500,
+      body: "error message",
+    });
     const tree = MountedSilenceSubmitProgress();
-    await expect(tree.instance().submitState.fetch).resolves.toBe(
-      "error message"
-    );
+    await fetchMock.flush(true);
     tree.update();
     expect(tree.find("FontAwesomeIcon.text-success")).toHaveLength(0);
     expect(tree.find("FontAwesomeIcon.text-danger")).toHaveLength(1);