From 01a8f069eb5990301f94e2d08ff504318f0c9b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Tue, 12 May 2020 09:22:31 +0100 Subject: [PATCH] fix(ui): rewrite SilenceSubmitProgress with hooks --- .../SilenceSubmit/SilenceSubmitProgress.js | 231 ++++++++-------- .../SilenceSubmitProgress.test.js | 257 ++++++++++++++---- 2 files changed, 320 insertions(+), 168 deletions(-) diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js index e8b51eac9..9648910ba 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js +++ b/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 ; - } - if (stateValue === SubmitState.Failed) { - return ( +const SubmitIcon = ({ stateValue }) => { + return useObserver(() => + stateValue === SubmitState.Done ? ( + + ) : stateValue === SubmitState.Failed ? ( - ); - } - return ; -}); + ) : ( + + ) + ); +}; const SilenceLink = ({ uri, silenceID }) => ( { + 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 = ; - 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 ( -
-
- -
-
- {cluster} -
-
- {this.submitState.result} -
-
- ); - } - } -); + return useObserver(() => ( +
+
+ +
+
+ {cluster} +
+
+ {submitState.result} +
+
+ )); +}; +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 }; diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js index eddb03889..7d74fe4a5 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js +++ b/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("", () => { 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("", () => { 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( + + ); + 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("", () => { 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("", () => { ], }; - const tree = mount( + mount( ", () => { 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( + + ); + 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("", () => { ], }; - const tree = mount( + mount( ", () => { 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( + + ); + 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("", () => { }); 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);