From 822f5503938add9699ac86077622f60dfe607f07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Mon, 12 Jul 2021 21:06:15 +0100 Subject: [PATCH] fix(ui): use POST requests for /alerts.json endpoint --- ui/src/Components/Fetcher/index.test.tsx | 24 ++++---- ui/src/Components/Fetcher/index.tsx | 2 +- ui/src/Models/APITypes.ts | 10 ++++ ui/src/Stores/AlertStore.test.ts | 71 +++++++++++++++--------- ui/src/Stores/AlertStore.ts | 32 ++++++----- 5 files changed, 86 insertions(+), 53 deletions(-) diff --git a/ui/src/Components/Fetcher/index.test.tsx b/ui/src/Components/Fetcher/index.test.tsx index d0c2fa3a6..d91cd99b8 100644 --- a/ui/src/Components/Fetcher/index.test.tsx +++ b/ui/src/Components/Fetcher/index.test.tsx @@ -149,7 +149,7 @@ describe("", () => { const fetchSpy = jest.spyOn(alertStore, "fetchWithThrottle"); settingsStore.gridConfig.setSortOrder("default"); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "", "", "", {}); + expect(fetchSpy).toHaveBeenCalledWith("", false, "", "", false, {}); }); it("calls alertStore.fetchWithThrottle with correct sort arguments when sortOrder=disabled reverseSort=false", () => { @@ -158,7 +158,7 @@ describe("", () => { settingsStore.gridConfig.setSortOrder("disabled"); settingsStore.gridConfig.setSortReverse(false); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "disabled", "", "", {}); + expect(fetchSpy).toHaveBeenCalledWith("", false, "disabled", "", false, {}); }); it("calls alertStore.fetchWithThrottle with correct sort arguments when sortOrder=disabled reverseSort=true", () => { @@ -167,7 +167,7 @@ describe("", () => { settingsStore.gridConfig.setSortOrder("disabled"); settingsStore.gridConfig.setSortReverse(true); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "disabled", "", "", {}); + expect(fetchSpy).toHaveBeenCalledWith("", false, "disabled", "", false, {}); }); it("calls alertStore.fetchWithThrottle with correct sort arguments when sortOrder=startsAt reverseSort=false", () => { @@ -176,7 +176,7 @@ describe("", () => { settingsStore.gridConfig.setSortOrder("startsAt"); settingsStore.gridConfig.setSortReverse(false); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "startsAt", "", "0", {}); + expect(fetchSpy).toHaveBeenCalledWith("", false, "startsAt", "", false, {}); }); it("calls alertStore.fetchWithThrottle with correct sort arguments when sortOrder=startsAt reverseSort=true", () => { @@ -185,7 +185,7 @@ describe("", () => { settingsStore.gridConfig.setSortOrder("startsAt"); settingsStore.gridConfig.setSortReverse(true); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "startsAt", "", "1", {}); + expect(fetchSpy).toHaveBeenCalledWith("", false, "startsAt", "", true, {}); }); it("calls alertStore.fetchWithThrottle with correct sort arguments when sortOrder=label sortLabel=cluster reverseSort=false", () => { @@ -200,7 +200,7 @@ describe("", () => { false, "label", "cluster", - "0", + false, {} ); }); @@ -212,7 +212,7 @@ describe("", () => { settingsStore.gridConfig.setSortLabel("job"); settingsStore.gridConfig.setSortReverse(true); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "label", "job", "1", {}); + expect(fetchSpy).toHaveBeenCalledWith("", false, "label", "job", true, {}); }); it("calls alertStore.fetchWithThrottle with correct sort arguments when sortOrder=label sortLabel=instance reverseSort=null", () => { @@ -227,7 +227,7 @@ describe("", () => { false, "label", "instance", - "", + false, {} ); }); @@ -239,7 +239,7 @@ describe("", () => { settingsStore.multiGridConfig.setGridLabel("cluster"); settingsStore.multiGridConfig.setGridSortReverse(false); mount(); - expect(fetchSpy).toHaveBeenCalledWith("cluster", false, "", "", "", {}); + expect(fetchSpy).toHaveBeenCalledWith("cluster", false, "", "", false, {}); }); it("calls alertStore.fetchWithThrottle with gridLabel=cluster gridSortReverse=true", () => { @@ -249,7 +249,7 @@ describe("", () => { settingsStore.multiGridConfig.setGridLabel("cluster"); settingsStore.multiGridConfig.setGridSortReverse(true); mount(); - expect(fetchSpy).toHaveBeenCalledWith("cluster", true, "", "", "", {}); + expect(fetchSpy).toHaveBeenCalledWith("cluster", true, "", "", false, {}); }); it("calls alertStore.fetchWithThrottle with gridLabel= gridSortReverse=true", () => { @@ -259,7 +259,7 @@ describe("", () => { settingsStore.multiGridConfig.setGridLabel(""); settingsStore.multiGridConfig.setGridSortReverse(true); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", true, "", "", "", {}); + expect(fetchSpy).toHaveBeenCalledWith("", true, "", "", false, {}); }); it("calls alertStore.fetchWithThrottle with limits set", () => { @@ -271,7 +271,7 @@ describe("", () => { alertStore.ui.setLimit("old", "bar", 10); alertStore.ui.setLimit("foo", "bar", 5); mount(); - expect(fetchSpy).toHaveBeenCalledWith("", false, "", "", "", { bar: 5 }); + expect(fetchSpy).toHaveBeenCalledWith("", false, "", "", false, { bar: 5 }); }); it("internal timer is null after unmount", () => { diff --git a/ui/src/Components/Fetcher/index.tsx b/ui/src/Components/Fetcher/index.tsx index 642c9bc87..21f1cf893 100644 --- a/ui/src/Components/Fetcher/index.tsx +++ b/ui/src/Components/Fetcher/index.tsx @@ -175,7 +175,7 @@ const Fetcher: FC<{ settingsStore.multiGridConfig.config.gridSortReverse, sortSettings.sortOrder, sortSettings.sortLabel, - sortSettings.sortReverse, + sortSettings.sortReverse === "1", Object.values(alertStore.ui.limits).length > 0 ? toJS(Object.values(alertStore.ui.limits)[0]) : {} diff --git a/ui/src/Models/APITypes.ts b/ui/src/Models/APITypes.ts index 6577ea4b3..a1a7e0c2f 100644 --- a/ui/src/Models/APITypes.ts +++ b/ui/src/Models/APITypes.ts @@ -229,3 +229,13 @@ export interface CountersResponseT { total: number; counters: APILabelCounterT[]; } + +export interface AlertsRequestT { + filters: string[]; + gridLabel: string; + gridLimits: { [key: string]: number }; + gridSortReverse: boolean; + sortOrder: string; + sortLabel: string; + sortReverse: boolean; +} diff --git a/ui/src/Stores/AlertStore.test.ts b/ui/src/Stores/AlertStore.test.ts index 2d80c72fb..0a7ffbccb 100644 --- a/ui/src/Stores/AlertStore.test.ts +++ b/ui/src/Stores/AlertStore.test.ts @@ -545,7 +545,7 @@ describe("AlertStore.fetch", () => { const store = new AlertStore(["label=value"]); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(fetchMock.calls()).toHaveLength(1); @@ -561,7 +561,7 @@ describe("AlertStore.fetch", () => { const store = new AlertStore([]); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(fetchMock.calls()).toHaveLength(1); @@ -580,7 +580,7 @@ describe("AlertStore.fetch", () => { const store = new AlertStore([]); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toHaveProperty("error"); expect(fetchMock.calls()).toHaveLength(10); @@ -600,7 +600,7 @@ describe("AlertStore.fetch", () => { }); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toHaveProperty("error"); expect(fetchMock.calls()).toHaveLength(10); }); @@ -615,7 +615,7 @@ describe("AlertStore.fetch", () => { }); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toHaveProperty("error"); expect(fetchMock.calls()).toHaveLength(10); @@ -626,7 +626,7 @@ describe("AlertStore.fetch", () => { }); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(fetchMock.calls()).toHaveLength(1); @@ -636,7 +636,7 @@ describe("AlertStore.fetch", () => { }); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toHaveProperty("error"); expect(fetchMock.calls()).toHaveLength(10); }); @@ -656,7 +656,7 @@ describe("AlertStore.fetch", () => { ); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(store.info.reloadNeeded).toBe(true); @@ -673,7 +673,7 @@ describe("AlertStore.fetch", () => { }); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toHaveProperty("error"); expect(store.filters.values[0].applied).toBe(true); }); @@ -690,7 +690,7 @@ describe("AlertStore.fetch", () => { // initial fetch, should update settings store.settings.setValues({ foo: "bar" } as any); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(store.settings.values).toMatchObject({ staticColorLabels: ["job"], @@ -702,7 +702,7 @@ describe("AlertStore.fetch", () => { // second fetch, should keep same settings await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(store.settings.values).toMatchObject({ staticColorLabels: ["job"], @@ -721,7 +721,7 @@ describe("AlertStore.fetch", () => { }); const store = new AlertStore(["label=value"]); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(store.info.upgradeReady).toBe(false); @@ -731,7 +731,7 @@ describe("AlertStore.fetch", () => { body: JSON.stringify(response), }); await expect( - store.fetch("", false, "", "", "", {}) + store.fetch("", false, "", "", false, {}) ).resolves.toBeUndefined(); expect(store.info.upgradeReady).toBe(true); }); @@ -797,12 +797,19 @@ describe("AlertStore.fetch", () => { }); const store = new AlertStore(["label=value"]); await expect( - store.fetch("", false, "sortOrder", "sortLabel", "sortReverse", {}) + store.fetch("", false, "sortOrder", "sortLabel", false, {}) ).resolves.toBeUndefined(); expect(fetchMock.calls().length).toEqual(1); - expect(fetchMock.calls()[0][0]).toBe( - "/alerts.json?&gridLabel=&gridSortReverse=0&sortOrder=sortOrder&sortLabel=sortLabel&sortReverse=sortReverse&q=label%3Dvalue" - ); + expect(fetchMock.calls()[0][0]).toBe("/alerts.json"); + expect(JSON.parse(fetchMock.calls()[0][1]?.body as string)).toStrictEqual({ + filters: ["label=value"], + gridLabel: "", + gridLimits: {}, + gridSortReverse: false, + sortOrder: "sortOrder", + sortLabel: "sortLabel", + sortReverse: false, + }); }); it("uses correct query args with gridSortReverse=true", async () => { @@ -813,12 +820,19 @@ describe("AlertStore.fetch", () => { }); const store = new AlertStore(["label=value"]); await expect( - store.fetch("cluster", true, "sortOrder", "sortLabel", "sortReverse", {}) + store.fetch("cluster", true, "sortOrder", "sortLabel", true, {}) ).resolves.toBeUndefined(); expect(fetchMock.calls().length).toEqual(1); - expect(fetchMock.calls()[0][0]).toBe( - "/alerts.json?&gridLabel=cluster&gridSortReverse=1&sortOrder=sortOrder&sortLabel=sortLabel&sortReverse=sortReverse&q=label%3Dvalue" - ); + expect(fetchMock.calls()[0][0]).toBe("/alerts.json"); + expect(JSON.parse(fetchMock.calls()[0][1]?.body as string)).toStrictEqual({ + filters: ["label=value"], + gridLabel: "cluster", + gridLimits: {}, + gridSortReverse: true, + sortOrder: "sortOrder", + sortLabel: "sortLabel", + sortReverse: true, + }); }); it("uses correct query args with limits", async () => { @@ -830,13 +844,20 @@ describe("AlertStore.fetch", () => { const store = new AlertStore(["label=value"]); store.ui.setLimit("foo", "bar", 7); await expect( - store.fetch("cluster", true, "sortOrder", "sortLabel", "sortReverse", { + store.fetch("cluster", true, "sortOrder", "sortLabel", false, { bar: 7, }) ).resolves.toBeUndefined(); expect(fetchMock.calls().length).toEqual(1); - expect(fetchMock.calls()[0][0]).toBe( - "/alerts.json?&gridLabel=cluster&gridSortReverse=1&sortOrder=sortOrder&sortLabel=sortLabel&sortReverse=sortReverse&limit=bar=7&q=label%3Dvalue" - ); + expect(fetchMock.calls()[0][0]).toBe("/alerts.json"); + expect(JSON.parse(fetchMock.calls()[0][1]?.body as string)).toStrictEqual({ + filters: ["label=value"], + gridLabel: "cluster", + gridLimits: { bar: 7 }, + gridSortReverse: true, + sortOrder: "sortOrder", + sortLabel: "sortLabel", + sortReverse: false, + }); }); }); diff --git a/ui/src/Stores/AlertStore.ts b/ui/src/Stores/AlertStore.ts index b1ef4bece..a22a20fb3 100644 --- a/ui/src/Stores/AlertStore.ts +++ b/ui/src/Stores/AlertStore.ts @@ -17,6 +17,7 @@ import type { APIAlertsResponseUpstreamsT, APIAlertsResponseUpstreamsClusterMapT, APISettingsT, + AlertsRequestT, } from "Models/APITypes"; const QueryStringEncodeOptions = { @@ -577,27 +578,28 @@ class AlertStore { gridSortReverse: boolean, sortOrder: string, sortLabel: string, - sortReverse: string, + sortReverse: boolean, limits: { [key: string]: number } ) => { this.status.setFetching(); - const args: string[] = [ - `gridLabel=${gridLabel}`, - `gridSortReverse=${gridSortReverse ? "1" : "0"}`, - `sortOrder=${sortOrder}`, - `sortLabel=${sortLabel}`, - `sortReverse=${sortReverse}`, - Object.entries(limits) - .map(([key, value]) => `limit=${key}=${value}`) - .join("&"), - ].filter((arg) => arg !== ""); + const payload: AlertsRequestT = { + filters: this.filters.values.map((f) => f.raw), + gridLabel: gridLabel, + gridSortReverse: gridSortReverse, + gridLimits: limits, + sortOrder: sortOrder, + sortLabel: sortLabel, + sortReverse: sortReverse, + }; - const alertsURI = - FormatBackendURI(`alerts.json?&${args.join("&")}&`) + - FormatAPIFilterQuery(this.filters.values.map((f) => f.raw)); + const alertsURI = FormatBackendURI("alerts.json"); - return await FetchGet(alertsURI, {}, this.info.setIsRetrying) + return await FetchGet( + alertsURI, + { method: "POST", body: JSON.stringify(payload) }, + this.info.setIsRetrying + ) .then((result) => { // we're sending requests with mode=cors so the response should also be type=cors // after a few failures in the retry loop we will switch to no-cors