From 0d2f4772aa91a37774a32b113abfbbca25e70898 Mon Sep 17 00:00:00 2001 From: Mary McGrath Date: Mon, 6 Jan 2025 15:58:13 -0500 Subject: [PATCH] feat: reset filters button (#3085) * first pass, back-end * first pass, front-end * simplify date range query, dont need subquery * update and add tests * make filtersService file, abstract components * add test for convertDateOptionToDateRange * update date helpers, modify tests * wip, Filters * update local dev default to last year * udpate tests to reflect local dev default * else if nit * refactor: moar abstraction (#3078) * refactor: moar abstraction * tiny nits, update tests * fix test --------- Co-authored-by: angelathe * refactor Filters file to add BaseFilter file * add BaseFilter * fix todays date for test * move FiltersService to utils as date-utils * add test to check reading query will result in correct date range * Update dateRangelabels Co-authored-by: Mary McGrath * [pre-commit.ci] auto fixes from pre-commit hooks * reverting change temporarily, causing test failures * update default nits * update default nits * hardcode expected start dates in tests * Flexible widths Co-authored-by: mary.mcgrath@skylight.digital * fix small nit, tag disappeared when no conditions were selected * update snapshot test for tag, turn btnClass to functional prop isActive * refactor dateRangeLabels * move default date range const to date-utils because forgot to import in page * update docstrings * refactor updateQueryParam * update today test fixture to add time * feat: only allow one filter open at a time, easy closing * Update containers/ecr-viewer/src/app/components/BaseFilter.tsx * fix: reset trigger * fix: cleanup * fix: only listen when a filter is open * fix: consolidate trigger logic, add tests * docs: comment * fix: comments, cleanup * fix: moar = * test: focus handling * feat: wip - reset filters button * test: cajole mocks * test: better check * fix: merge cleanup * test: make reset button test more robust * fix: reset button gap * test: update snapshot * fix: gap naming convention * refactor: readability --------- Co-authored-by: angelathe Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../src/app/components/BaseFilter.tsx | 16 +- .../ecr-viewer/src/app/components/Filters.tsx | 54 +++-- .../src/app/tests/HomePage.test.tsx | 1 + .../src/app/tests/components/Filters.test.tsx | 199 +++++++++++++++++- .../__snapshots__/Filters.test.tsx.snap | 28 ++- .../src/app/tests/date-utils.test.ts | 1 - .../ecr-viewer/src/styles/utilities.scss | 4 + 7 files changed, 272 insertions(+), 31 deletions(-) diff --git a/containers/ecr-viewer/src/app/components/BaseFilter.tsx b/containers/ecr-viewer/src/app/components/BaseFilter.tsx index b13ddbf97..0fdcf0957 100644 --- a/containers/ecr-viewer/src/app/components/BaseFilter.tsx +++ b/containers/ecr-viewer/src/app/components/BaseFilter.tsx @@ -44,6 +44,12 @@ export const useQueryParam = () => { [searchParams], ); + const pushQueryUpdate = (params: URLSearchParams, keys: string[]) => { + if (keys.some((key) => searchParams.get(key) !== params.get(key))) { + router.push(pathname + "?" + params.toString()); + } + }; + // Update a specific query param (set or delete if default) const updateQueryParam = ( key: string, @@ -60,12 +66,10 @@ export const useQueryParam = () => { : setQueryParam(key, value); // Update query params - if (searchParams.get(key) !== updatedParams.get(key)) { - router.push(pathname + "?" + updatedParams.toString()); - } + pushQueryUpdate(updatedParams, [key]); }; - return { searchParams, updateQueryParam }; + return { searchParams, updateQueryParam, pushQueryUpdate }; }; /** @@ -105,6 +109,7 @@ export const Filter = ({ const { filterBoxOpen, setFilterBoxOpen, lastOpenButtonRef } = useContext(FilterOpenContext); const openBtnRef = useRef(null); + const searchParams = useSearchParams(); const isFilterBoxOpen = filterBoxOpen === type; const setIsFilterBoxOpen = useCallback((open: boolean) => { @@ -127,6 +132,9 @@ export const Filter = ({ } }, [filterBoxOpen]); + // Force a reset if the search params update + useEffect(resetHandler, [searchParams]); + return (
e.stopPropagation()}>
diff --git a/containers/ecr-viewer/src/app/components/Filters.tsx b/containers/ecr-viewer/src/app/components/Filters.tsx index 1a9e47274..c5a220f1e 100644 --- a/containers/ecr-viewer/src/app/components/Filters.tsx +++ b/containers/ecr-viewer/src/app/components/Filters.tsx @@ -7,7 +7,7 @@ import React, { useRef, useState, } from "react"; -import { Icon } from "@trussworks/react-uswds"; +import { Button, Icon } from "@trussworks/react-uswds"; import { useQueryParam, Filter } from "./BaseFilter"; import { DEFAULT_DATE_RANGE, @@ -42,6 +42,7 @@ export const FilterOpenContext = createContext({ const Filters = () => { const [filterBoxOpen, setFilterBoxOpen] = useState(FILTER_CLOSED); const lastOpenButtonRef = useRef(null); + const { searchParams, pushQueryUpdate } = useQueryParam(); const filterOpenContextValue = { filterBoxOpen, @@ -76,6 +77,16 @@ const Filters = () => { } }, [filterBoxOpen]); + const paramKeys = ["condition", "dateRange"]; + const resetToDefault = () => { + const params = new URLSearchParams(searchParams.toString()); + params.set("page", "1"); + for (const key of paramKeys) { + params.delete(key); + } + pushQueryUpdate(params, paramKeys); + }; + return (
@@ -85,6 +96,21 @@ const Filters = () => { + + {paramKeys.some((k) => searchParams.get(k) !== null) && ( + + )}
); @@ -158,14 +184,20 @@ const FilterReportableConditions = () => { return conditionsToTrue.has(c); } }; + let changed = false; const prevFilterConditions = conditions.reduce( (dict: { [key: string]: boolean }, condition: string) => { dict[condition] = conditionValue(condition); + if (dict[condition] != filterConditions[condition]) { + changed = true; + } return dict; }, {} as { [key: string]: boolean }, ); - setFilterConditions(prevFilterConditions); + if (changed) { + setFilterConditions(prevFilterConditions); + } }; return ( @@ -241,21 +273,10 @@ const FilterReportableConditions = () => { const FilterByDate = () => { const { searchParams, updateQueryParam } = useQueryParam(); - const [filterDateOption, setFilterDateOption] = useState(""); + const [filterDateOption, setFilterDateOption] = + useState(DEFAULT_DATE_RANGE); const isFilterDateDefault = filterDateOption === DEFAULT_DATE_RANGE; - useEffect(() => { - const fetchDateRange = async () => { - try { - resetFilterDate(); - } catch (error) { - console.error("Error fetching date range:", error); - } - }; - - fetchDateRange(); - }, []); - const handleDateOptionChange = ( event: React.ChangeEvent, ) => { @@ -273,6 +294,9 @@ const FilterByDate = () => { } }; + // on mount, make sure the filters match the search params + useEffect(resetFilterDate, []); + return ( ({ })); describe("Filter by Reportable Conditions Component", () => { - const mockUseSearchParams = useSearchParams as jest.Mock; beforeEach(() => { jest.clearAllMocks(); - mockUseSearchParams.mockImplementation( - () => new URLSearchParams("condition=Condition1|Condition2"), + const mockSearchParams = { + current: new URLSearchParams("condition=Condition1|Condition2"), + }; + (useSearchParams as jest.Mock).mockImplementation( + () => mockSearchParams.current, ); + const mockPush = jest.fn().mockImplementation((path: string) => { + const url = new URL(path, "https://example.com"); + mockSearchParams.current = new URLSearchParams(url.search); + }); + (useRouter as jest.Mock).mockImplementation(() => { + return { push: mockPush }; + }); + global.fetch = jest.fn().mockImplementation(() => Promise.resolve({ ok: true, @@ -44,7 +54,7 @@ describe("Filter by Reportable Conditions Component", () => { it("Toggles filter by conditions combo box visibility", async () => { render(); - const toggleButton = screen.getByRole("button", { + const toggleButton = await screen.findByRole("button", { name: /Filter by reportable condition/i, }); @@ -223,7 +233,7 @@ describe("Filter by Reportable Conditions Component", () => { }); it("Query should persist over a reload", async () => { - render(); + const { rerender } = render(); const toggleFilterButton = screen.getByRole("button", { name: /Filter by reportable condition/i, }); @@ -237,7 +247,7 @@ describe("Filter by Reportable Conditions Component", () => { const applyButton = screen.getByRole("button", { name: /Apply Filter/i }); fireEvent.click(applyButton); - window.location.reload(); + rerender(); fireEvent.click(toggleFilterButton); await waitFor(() => screen.getByText("Condition1")); @@ -276,12 +286,32 @@ describe("Filter by Date Component", () => { // NOTE: Tests look for Last Year = local dev default. The prod default is Last 24 hours. beforeEach(() => { jest.clearAllMocks(); + + const mockSearchParams = { current: new URLSearchParams("") }; + (useSearchParams as jest.Mock).mockImplementation( + () => mockSearchParams.current, + ); + + const mockPush = jest.fn().mockImplementation((path: string) => { + const url = new URL(path, "https://example.com"); + mockSearchParams.current = new URLSearchParams(url.search); + }); + (useRouter as jest.Mock).mockImplementation(() => { + return { push: mockPush }; + }); + + global.fetch = jest.fn().mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve(["Condition1", "Condition2"]), + } as unknown as Response), + ); }); it("Renders correctly after opening Filter by Date box", async () => { const { container } = render(); - const toggleFilterButton = screen.getByRole("button", { + const toggleFilterButton = await screen.findByRole("button", { name: /Filter by Received Date/i, }); fireEvent.click(toggleFilterButton); @@ -290,7 +320,7 @@ describe("Filter by Date Component", () => { }); it("Toggles Filter by Date combo box visibility", async () => { render(); - const toggleButton = screen.getByRole("button", { + const toggleButton = await screen.findByRole("button", { name: /Filter by Received Date/i, }); @@ -311,7 +341,7 @@ describe("Filter by Date Component", () => { expect(screen.queryByText(/Filter by Received Date/)).toBeNull(); }); it("Updates filter date range when selection is made", async () => { - render(); + const { rerender } = render(); const toggleButton = screen.getByRole("button", { name: /Filter by Received Date/i, }); @@ -356,7 +386,7 @@ describe("Filter by Date Component", () => { ).toHaveTextContent("Last 7 days"); // Query should persist over a reload - window.location.reload(); + rerender(); expect( screen.getByRole("button", { name: /Filter by Received Date/i, @@ -426,6 +456,30 @@ describe("Filter by Date Component", () => { }); describe("Filter Opening/Closing Controls", () => { + beforeEach(() => { + jest.clearAllMocks(); + + const mockSearchParams = { current: new URLSearchParams("") }; + (useSearchParams as jest.Mock).mockImplementation( + () => mockSearchParams.current, + ); + + const mockPush = jest.fn().mockImplementation((path: string) => { + const url = new URL(path, "https://example.com"); + mockSearchParams.current = new URLSearchParams(url.search); + }); + (useRouter as jest.Mock).mockImplementation(() => { + return { push: mockPush }; + }); + + global.fetch = jest.fn().mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve(["Condition1", "Condition2"]), + } as unknown as Response), + ); + }); + it("If a date range is checked but escape is hit without applying filter, filters should reset", async () => { render(); const toggleButton = screen.getByRole("button", { @@ -548,3 +602,128 @@ describe("Filter Opening/Closing Controls", () => { expect(screen.queryByText("Select all")).not.toBeInTheDocument(); }); }); + +describe("Reset button", () => { + let mockPush: jest.Mock; + const SearchParamContext = React.createContext({} as any); + beforeEach(() => { + jest.clearAllMocks(); + + (useSearchParams as jest.Mock).mockImplementation(() => { + const { searchParams } = React.useContext(SearchParamContext); + return searchParams; + }); + + mockPush = jest.fn().mockImplementation((path: string, setSearchParams) => { + const url = new URL(path, "https://example.com"); + setSearchParams(new URLSearchParams(url.search)); + }); + (useRouter as jest.Mock).mockImplementation(() => { + const { setSearchParams } = React.useContext(SearchParamContext); + return { push: (path: string) => mockPush(path, setSearchParams) }; + }); + + global.fetch = jest.fn().mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve(["Condition1", "Condition2"]), + } as unknown as Response), + ); + }); + + it("changes back to original defaults", async () => { + const SearchParamWrapper = ({ + children, + }: { + children: React.ReactNode; + }) => { + const [searchParams, setSearchParams] = React.useState( + new URLSearchParams(""), + ); + return ( + + {children} + + ); + }; + render( + + + , + ); + + // reset button not visible by default + expect( + screen.queryByRole("button", { name: /Reset Filters to Defaults/i }), + ).not.toBeInTheDocument(); + + const dateToggleButton = screen.getByRole("button", { + name: /Filter by Received Date/i, + }); + fireEvent.click(dateToggleButton); + + // Click different date option and submit + const radio = screen.getByRole("radio", { + name: "Last 7 days", + }); + fireEvent.click(radio); + expect(radio).toBeChecked(); + + fireEvent.submit(radio); + await waitFor(() => screen.findByText("Last 7 days")); + + // should be closed + expect(screen.queryByRole("radio")).not.toBeInTheDocument(); + + // Selection should persist because filter was applied + expect(screen.getByText("Last 7 days")).toBeInTheDocument(); + + // Should have other condition in search param + expect(mockPush).toHaveBeenCalledWith( + expect.stringContaining("dateRange=last-7-days"), + expect.anything(), + ); + + // reset button should be visible now that something has changed + expect( + screen.getByRole("button", { name: /Reset Filters to Defaults/i }), + ).toBeInTheDocument(); + + // Update condition selection + const conditionToggleButton = screen.getByRole("button", { + name: /Filter by reportable condition/i, + }); + fireEvent.click(conditionToggleButton); + + await waitFor(() => screen.getByText("Condition1")); + const checkbox = screen.getByLabelText("Condition1"); + fireEvent.click(checkbox); + expect(checkbox).not.toBeChecked(); + + fireEvent.submit(checkbox); + + // Should have other condition in search param + expect(mockPush).toHaveBeenCalledWith( + expect.stringContaining("condition=Condition2"), + expect.anything(), + ); + + const resetButton = screen.getByRole("button", { + name: /Reset Filters to Defaults/i, + }); + fireEvent.click(resetButton); + expect(mockPush).toHaveBeenCalledWith( + expect.not.stringContaining("dateRange="), + expect.anything(), + ); + expect(mockPush).toHaveBeenCalledWith( + expect.not.stringContaining("condition="), + expect.anything(), + ); + // default title + await waitFor(() => screen.findByText("Last year")); + expect(screen.getByText("Last year")).toBeInTheDocument(); + // not active + expect(conditionToggleButton).toHaveClass("filter-button"); + }); +}); diff --git a/containers/ecr-viewer/src/app/tests/components/__snapshots__/Filters.test.tsx.snap b/containers/ecr-viewer/src/app/tests/components/__snapshots__/Filters.test.tsx.snap index 27167411b..f19680ceb 100644 --- a/containers/ecr-viewer/src/app/tests/components/__snapshots__/Filters.test.tsx.snap +++ b/containers/ecr-viewer/src/app/tests/components/__snapshots__/Filters.test.tsx.snap @@ -232,7 +232,7 @@ exports[`Filter by Date Component Renders correctly after opening Filter by Date class="usa-tag padding-05 bg-base-darker radius-md" data-testid="filter-tag" > - 0 + 2
@@ -431,6 +431,32 @@ exports[`Filter by Reportable Conditions Component renders correctly after openi + diff --git a/containers/ecr-viewer/src/app/tests/date-utils.test.ts b/containers/ecr-viewer/src/app/tests/date-utils.test.ts index 514f2b5cb..0159b7491 100644 --- a/containers/ecr-viewer/src/app/tests/date-utils.test.ts +++ b/containers/ecr-viewer/src/app/tests/date-utils.test.ts @@ -5,7 +5,6 @@ import { describe("convertDateOptionToDateRange", () => { const today = new Date("2024-04-02T12:34:01.009"); - console.log(today); it("should return last 24 hours range", () => { const expectedStart = new Date("2024-04-01T16:34:01.009Z"); diff --git a/containers/ecr-viewer/src/styles/utilities.scss b/containers/ecr-viewer/src/styles/utilities.scss index c9b118d8d..282db7320 100644 --- a/containers/ecr-viewer/src/styles/utilities.scss +++ b/containers/ecr-viewer/src/styles/utilities.scss @@ -89,6 +89,10 @@ margin-top: 1.5rem; } +.gap-05 { + gap: 0.25rem; +} + .gap-1 { gap: 0.5rem; }