From b982f04ac65e1b8c7b68b084bcb0399bb7858b2b Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Mon, 9 Mar 2026 16:20:03 +0000 Subject: [PATCH] fix(ui): fix history rewrites --- ui/package-lock.json | 81 ------------------- .../NavBar/FilterInput/History.test.tsx | 74 +++++++++++++++++ .../Components/NavBar/FilterInput/History.tsx | 16 ++-- 3 files changed, 80 insertions(+), 91 deletions(-) diff --git a/ui/package-lock.json b/ui/package-lock.json index d15871f9c..c8bad1098 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -3795,9 +3795,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3819,9 +3816,6 @@ "arm" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3843,9 +3837,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3867,9 +3858,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3891,9 +3879,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3915,9 +3900,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4158,9 +4140,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4175,9 +4154,6 @@ "arm" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4192,9 +4168,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4209,9 +4182,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4226,9 +4196,6 @@ "loong64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4243,9 +4210,6 @@ "loong64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4260,9 +4224,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4277,9 +4238,6 @@ "ppc64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4294,9 +4252,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4311,9 +4266,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4328,9 +4280,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4345,9 +4294,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -4362,9 +4308,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5267,9 +5210,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5284,9 +5224,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5301,9 +5238,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5318,9 +5252,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5335,9 +5266,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5352,9 +5280,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5369,9 +5294,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -5386,9 +5308,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ diff --git a/ui/src/Components/NavBar/FilterInput/History.test.tsx b/ui/src/Components/NavBar/FilterInput/History.test.tsx index 96c58e33d..25f5be415 100644 --- a/ui/src/Components/NavBar/FilterInput/History.test.tsx +++ b/ui/src/Components/NavBar/FilterInput/History.test.tsx @@ -459,6 +459,80 @@ describe("History localStorage", () => { await act(() => promise); }); + // Verifies that re-applying the exact same filter set does not cause + // a redundant write to history storage. + it("does not write to history when filters have not changed", async () => { + const promise = Promise.resolve(); + const { container } = renderHistory(); + + // Apply filter set A + act(() => { + alertStore.filters.setFilterValues([AppliedFilter("foo", "=", "bar")]); + jest.runOnlyPendingTimers(); + }); + + const toggle = container.querySelector("button.cursor-pointer"); + fireEvent.click(toggle!); + expect(container.querySelectorAll("button.dropdown-item")).toHaveLength(1); + fireEvent.click(toggle!); + act(() => { + jest.runOnlyPendingTimers(); + }); + + // Read localStorage to get the current persisted state + const historyBefore = localStorage.getItem("filters"); + + // Re-apply the exact same filter set A + act(() => { + alertStore.filters.setFilterValues([AppliedFilter("foo", "=", "bar")]); + jest.runOnlyPendingTimers(); + }); + + // History in localStorage should be identical — no redundant write + const historyAfter = localStorage.getItem("filters"); + expect(historyAfter).toBe(historyBefore); + await act(() => promise); + }); + + // Verifies that re-applying a filter set that already exists in history + // does not reorder the history entries. + it("does not reorder history when re-applying a filter set already on top", async () => { + const promise = Promise.resolve(); + const { container } = renderHistory(); + + // Apply filter set A then B, so history is [B, A] + act(() => { + alertStore.filters.setFilterValues([ + AppliedFilter("cluster", "=", "prod"), + ]); + jest.runOnlyPendingTimers(); + }); + act(() => { + alertStore.filters.setFilterValues([ + AppliedFilter("env", "=", "staging"), + ]); + jest.runOnlyPendingTimers(); + }); + + // Re-apply filter set A — it should NOT jump to the top + act(() => { + alertStore.filters.setFilterValues([ + AppliedFilter("cluster", "=", "prod"), + ]); + jest.runOnlyPendingTimers(); + }); + + const toggle = container.querySelector("button.cursor-pointer"); + fireEvent.click(toggle!); + + const items = container.querySelectorAll("button.dropdown-item"); + expect(items).toHaveLength(2); + // Order should remain [B, A] — "env=staging" on top + expect(items[0].textContent).toBe("env=staging"); + expect(items[1].textContent).toBe("cluster=prod"); + await act(() => promise); + }); + // Demonstrates the cross-tab race condition. When tab B receives a // StorageEvent with tab A's history and then tab B's own alertStore // filters change, the History component's autorun rebuilds history diff --git a/ui/src/Components/NavBar/FilterInput/History.tsx b/ui/src/Components/NavBar/FilterInput/History.tsx index c92768a03..4df409073 100644 --- a/ui/src/Components/NavBar/FilterInput/History.tsx +++ b/ui/src/Components/NavBar/FilterInput/History.tsx @@ -228,19 +228,15 @@ const History: FC<{ // make a JSON dump for comparing later with what's already stored const filtersJSON = JSON.stringify(validAppliedFilters); - // rewrite history putting current filter set on top, this will move - // it up if user selects a filter set that was already in history + // don't rewrite history if filters haven't changed + if (history.config.filters.some((f) => JSON.stringify(f) === filtersJSON)) + return; + const newHistory = [ ...[validAppliedFilters], - ...history.config.filters.filter( - (f) => JSON.stringify(f) !== filtersJSON, - ), + ...history.config.filters, ].slice(0, 8); - if ( - JSON.stringify(newHistory) !== JSON.stringify(history.config.filters) - ) { - history.setFilters(newHistory); - } + history.setFilters(newHistory); }); return () => { disposeAutorun();