From d3936ca80b4d4c0d3e84ce26e10ec06968ef0043 Mon Sep 17 00:00:00 2001 From: doug-s-nava <92806979+doug-s-nava@users.noreply.github.com> Date: Thu, 13 Feb 2025 15:40:33 -0500 Subject: [PATCH] Agencies error boundary (#3878) * removes global state * removes search error page * instead of a boundary based approach, which is limited to client components, handles API request based search errors where they occur --- frontend/package.json | 3 +- frontend/src/app/[locale]/search/error.tsx | 136 ------------------ frontend/src/components/Layout.tsx | 25 ++-- .../src/components/search/SearchError.tsx | 39 +++++ .../AgencyFilterAccordion.tsx | 9 +- .../SearchFilterAccordion.tsx | 12 +- .../src/components/search/SearchResults.tsx | 10 +- .../globalState/GlobalStateProvider.tsx | 71 --------- frontend/src/types/generalTypes.ts | 10 ++ .../AgencyFilterAccordion.test.tsx | 6 - .../SearchFilterAccordion.test.tsx | 6 - .../components/search/SearchFilters.test.tsx | 6 - frontend/tests/errors.test.ts | 3 +- frontend/tests/pages/search/page.test.tsx | 6 - .../globalState/GlobalStateProvider.test.tsx | 55 ------- 15 files changed, 81 insertions(+), 316 deletions(-) delete mode 100644 frontend/src/app/[locale]/search/error.tsx create mode 100644 frontend/src/components/search/SearchError.tsx delete mode 100644 frontend/src/services/globalState/GlobalStateProvider.tsx delete mode 100644 frontend/tests/services/globalState/GlobalStateProvider.test.tsx diff --git a/frontend/package.json b/frontend/package.json index 9dc966ef1..afeafc5e9 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -49,8 +49,7 @@ "react-dom": "^19.0.0", "server-only": "^0.0.1", "sharp": "^0.33.5", - "zod": "^3.23.8", - "zustand": "^5.0.3" + "zod": "^3.23.8" }, "devDependencies": { "@chromatic-com/storybook": "^1.9.0", diff --git a/frontend/src/app/[locale]/search/error.tsx b/frontend/src/app/[locale]/search/error.tsx deleted file mode 100644 index d23860792..000000000 --- a/frontend/src/app/[locale]/search/error.tsx +++ /dev/null @@ -1,136 +0,0 @@ -"use client"; - -import QueryProvider from "src/app/[locale]/search/QueryProvider"; -import { usePrevious } from "src/hooks/usePrevious"; -import { useGlobalState } from "src/services/globalState/GlobalStateProvider"; -import { FrontendErrorDetails } from "src/types/apiResponseTypes"; -import { OptionalStringDict } from "src/types/generalTypes"; -import { Breakpoints, ErrorProps } from "src/types/uiTypes"; -import { convertSearchParamsToProperTypes } from "src/utils/search/convertSearchParamsToProperTypes"; - -import { useTranslations } from "next-intl"; -import { ReadonlyURLSearchParams, useSearchParams } from "next/navigation"; -import { useEffect } from "react"; -import { Alert } from "@trussworks/react-uswds"; - -import ContentDisplayToggle from "src/components/ContentDisplayToggle"; -import SearchBar from "src/components/search/SearchBar"; -import { AgencyFilterAccordion } from "src/components/search/SearchFilterAccordion/AgencyFilterAccordion"; -import SearchFilterAccordion from "src/components/search/SearchFilterAccordion/SearchFilterAccordion"; -import { - categoryOptions, - eligibilityOptions, - fundingOptions, -} from "src/components/search/SearchFilterAccordion/SearchFilterOptions"; -import SearchOpportunityStatus from "src/components/search/SearchOpportunityStatus"; -import ServerErrorAlert from "src/components/ServerErrorAlert"; - -export interface ParsedError { - message?: string; - searchInputs?: OptionalStringDict; - status?: number; - type?: string; - details?: FrontendErrorDetails; -} - -function isValidJSON(str: string) { - try { - JSON.parse(str); - return true; - } catch (e) { - return false; // String is not valid JSON - } -} - -// note that the SearchFilters component is not used here since that is a server component -// we work around that by including the rendered components from SearchFilters, but manually -// passing through the agency options as received from global state rather than fetching from API -export default function SearchError({ error, reset }: ErrorProps) { - const t = useTranslations("Search"); - const searchParams = useSearchParams(); - const previousSearchParams = - usePrevious(searchParams); - useEffect(() => { - console.error(error); - }, [error]); - - const parsedErrorData = isValidJSON(error.message) - ? (JSON.parse(error.message) as ParsedError) - : {}; - - const { agencyOptions } = useGlobalState(({ agencyOptions }) => ({ - agencyOptions, - })); - - const convertedSearchParams = convertSearchParamsToProperTypes( - Object.fromEntries(searchParams.entries().toArray()), - ); - - useEffect(() => { - if ( - reset && - previousSearchParams && - searchParams.toString() !== previousSearchParams?.toString() - ) { - reset(); - } - }, [searchParams, reset, previousSearchParams]); - - const { agency, category, eligibility, fundingInstrument, query, status } = - convertedSearchParams; - - // note that the validation error will contain untranslated strings - // and will only appear in development, prod builds will not include user facing error details - const ErrorAlert = - parsedErrorData.details && parsedErrorData.type === "ValidationError" ? ( - - {`Error in ${parsedErrorData.details.field || "a search field"}: ${parsedErrorData.details.message || "adjust your search and try again"}`} - - ) : ( - - ); - - return ( - -
-
- -
-
-
- - - - - - - -
-
{ErrorAlert}
-
-
- - ); -} diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index 4378675b7..4425f1f39 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -1,5 +1,4 @@ import UserProvider from "src/services/auth/UserProvider"; -import { GlobalStateProvider } from "src/services/globalState/GlobalStateProvider"; import { useTranslations } from "next-intl"; import { setRequestLocale } from "next-intl/server"; @@ -21,19 +20,17 @@ export default function Layout({ children, locale }: Props) { return ( // Stick the footer to the bottom of the page - -
- - {t("Layout.skip_to_main")} - -
-
- {children} -
-
- -
- +
+ + {t("Layout.skip_to_main")} + +
+
+ {children} +
+
+ +
); } diff --git a/frontend/src/components/search/SearchError.tsx b/frontend/src/components/search/SearchError.tsx new file mode 100644 index 000000000..95df06cdf --- /dev/null +++ b/frontend/src/components/search/SearchError.tsx @@ -0,0 +1,39 @@ +"use client"; + +import { ParsedError } from "src/types/generalTypes"; +import { ErrorProps } from "src/types/uiTypes"; + +import { useTranslations } from "next-intl"; +import { Alert } from "@trussworks/react-uswds"; + +import ServerErrorAlert from "src/components/ServerErrorAlert"; + +function isValidJSON(str: string) { + try { + JSON.parse(str); + return true; + } catch (e) { + return false; // String is not valid JSON + } +} + +export function SearchError({ error }: ErrorProps) { + const t = useTranslations("Search"); + + const parsedErrorData = isValidJSON(error.message) + ? (JSON.parse(error.message) as ParsedError) + : {}; + + // note that the validation error will contain untranslated strings + // and will only appear in development, prod builds will not include user facing error details + const ErrorAlert = + parsedErrorData.details && parsedErrorData.type === "ValidationError" ? ( + + {`Error in ${parsedErrorData.details.field || "a search field"}: ${parsedErrorData.details.message || "adjust your search and try again"}`} + + ) : ( + + ); + + return
{ErrorAlert}
; +} diff --git a/frontend/src/components/search/SearchFilterAccordion/AgencyFilterAccordion.tsx b/frontend/src/components/search/SearchFilterAccordion/AgencyFilterAccordion.tsx index f3fc4237a..617b611ea 100644 --- a/frontend/src/components/search/SearchFilterAccordion/AgencyFilterAccordion.tsx +++ b/frontend/src/components/search/SearchFilterAccordion/AgencyFilterAccordion.tsx @@ -15,7 +15,14 @@ async function AgencyFilterAccordionWithFetchedOptions({ agenciesPromise: Promise; title: string; }) { - const agencies = await agenciesPromise; + let agencies: FilterOption[]; + try { + agencies = await agenciesPromise; + } catch (e) { + // Come back to this to show the user an error + console.error("Unable to fetch agencies for filter list", e); + agencies = []; + } return ( ({ - setAgencyOptions, - })); - - useEffect(() => { - if (queryParamKey === "agency" && filterOptions && setAgencyOptions) { - setAgencyOptions(filterOptions); - } - }, [queryParamKey, filterOptions, setAgencyOptions]); const totalCheckedCount = query.size; // all top level selectable filter options diff --git a/frontend/src/components/search/SearchResults.tsx b/frontend/src/components/search/SearchResults.tsx index f99944495..786f74cfe 100644 --- a/frontend/src/components/search/SearchResults.tsx +++ b/frontend/src/components/search/SearchResults.tsx @@ -7,6 +7,7 @@ import { Suspense } from "react"; import { ClientSideUrlUpdater } from "src/components/ClientSideUrlUpdater"; import Loading from "src/components/Loading"; import { ExportSearchResultsButton } from "./ExportSearchResultsButton"; +import { SearchError } from "./SearchError"; import SearchPagination from "./SearchPagination"; import SearchResultsHeader from "./SearchResultsHeader"; import SearchResultsList from "./SearchResultsList"; @@ -47,7 +48,14 @@ const ResolvedSearchResults = async ({ query?: string | null; searchResultsPromise: Promise; }) => { - const searchResults = await searchResultsPromise; + let searchResults: SearchAPIResponse; + + try { + searchResults = await searchResultsPromise; + } catch (e) { + const error = e as Error; + return ; + } // if there are no results because we've requested a page beyond the number of total pages // update page to the last page to trigger a new search diff --git a/frontend/src/services/globalState/GlobalStateProvider.tsx b/frontend/src/services/globalState/GlobalStateProvider.tsx deleted file mode 100644 index 044b7c4b3..000000000 --- a/frontend/src/services/globalState/GlobalStateProvider.tsx +++ /dev/null @@ -1,71 +0,0 @@ -"use client"; - -import { useStore } from "zustand"; -import { useShallow } from "zustand/react/shallow"; -import { createStore, StoreApi } from "zustand/vanilla"; - -import { createContext, useContext, useRef, type ReactNode } from "react"; - -import { FilterOption } from "src/components/search/SearchFilterAccordion/SearchFilterAccordion"; - -type GlobalStateItems = { - agencyOptions: FilterOption[]; -}; - -type GlobalStateActions = { - setAgencyOptions: (options: FilterOption[]) => void; -}; - -type GlobalState = GlobalStateItems & GlobalStateActions; - -type GlobalStore = StoreApi; - -interface GlobalStateProviderProps { - children: ReactNode; -} - -const defaultInitState: GlobalStateItems = { - agencyOptions: [], -}; - -const createGlobalStore = (initState = defaultInitState) => { - return createStore()((set) => ({ - ...initState, - setAgencyOptions: (agencyOptions: FilterOption[]) => - set(() => ({ - agencyOptions, - })), - })); -}; - -const GlobalStateContext = createContext(undefined); - -// ref usage works around Next JS weirdness - see https://zustand.docs.pmnd.rs/guides/nextjs -export const GlobalStateProvider = ({ children }: GlobalStateProviderProps) => { - const storeRef = useRef(undefined); - if (!storeRef.current) { - storeRef.current = createGlobalStore(); - } - - return ( - - {children} - - ); -}; - -// selector here is a generic function that will take the store as an argument and return -// whatever you want from that store -export const useGlobalState = >( - selector: (store: GlobalState) => T, -): T => { - // references the store created and passed down as value in the provider - const globalStateStore = useContext(GlobalStateContext); - - if (!globalStateStore) { - throw new Error("useGlobalState must be used within GlobalStateProvider"); - } - - // not sure what useShallow is doing here but it's necessary :shrug: - return useStore(globalStateStore, useShallow(selector)); -}; diff --git a/frontend/src/types/generalTypes.ts b/frontend/src/types/generalTypes.ts index 695c7cf97..4b49293b7 100644 --- a/frontend/src/types/generalTypes.ts +++ b/frontend/src/types/generalTypes.ts @@ -1,3 +1,5 @@ +import { FrontendErrorDetails } from "src/types/apiResponseTypes"; + export interface LayoutProps { children: React.ReactNode; params: Promise<{ @@ -8,3 +10,11 @@ export interface LayoutProps { export interface OptionalStringDict { [key: string]: string | undefined; } + +export interface ParsedError { + message?: string; + searchInputs?: OptionalStringDict; + status?: number; + type?: string; + details?: FrontendErrorDetails; +} diff --git a/frontend/tests/components/search/SearchFilterAccordion/AgencyFilterAccordion.test.tsx b/frontend/tests/components/search/SearchFilterAccordion/AgencyFilterAccordion.test.tsx index 61fbb767c..91a20b9c2 100644 --- a/frontend/tests/components/search/SearchFilterAccordion/AgencyFilterAccordion.test.tsx +++ b/frontend/tests/components/search/SearchFilterAccordion/AgencyFilterAccordion.test.tsx @@ -18,12 +18,6 @@ jest.mock("src/hooks/useSearchParamUpdater", () => ({ }), })); -jest.mock("src/services/globalState/GlobalStateProvider", () => ({ - useGlobalState: () => ({ - setAgencyOptions: () => undefined, - }), -})); - jest.mock("react", () => ({ ...jest.requireActual("react"), Suspense: ({ fallback }: { fallback: React.Component }) => fallback, diff --git a/frontend/tests/components/search/SearchFilterAccordion/SearchFilterAccordion.test.tsx b/frontend/tests/components/search/SearchFilterAccordion/SearchFilterAccordion.test.tsx index 01577c01d..dd5b9f945 100644 --- a/frontend/tests/components/search/SearchFilterAccordion/SearchFilterAccordion.test.tsx +++ b/frontend/tests/components/search/SearchFilterAccordion/SearchFilterAccordion.test.tsx @@ -41,12 +41,6 @@ jest.mock("src/hooks/useSearchParamUpdater", () => ({ }), })); -jest.mock("src/services/globalState/GlobalStateProvider", () => ({ - useGlobalState: () => ({ - setAgencyOptions: () => undefined, - }), -})); - describe("SearchFilterAccordion", () => { const title = "Test Accordion"; const queryParamKey = "fundingInstrument"; diff --git a/frontend/tests/components/search/SearchFilters.test.tsx b/frontend/tests/components/search/SearchFilters.test.tsx index f6e4392d6..89b6b22b3 100644 --- a/frontend/tests/components/search/SearchFilters.test.tsx +++ b/frontend/tests/components/search/SearchFilters.test.tsx @@ -23,12 +23,6 @@ jest.mock("next-intl", () => ({ useTranslations: () => useTranslationsMock(), })); -jest.mock("src/services/globalState/GlobalStateProvider", () => ({ - useGlobalState: () => ({ - setAgencyOptions: () => undefined, - }), -})); - // otherwise we have to deal with mocking suspense jest.mock( "src/components/search/SearchFilterAccordion/AgencyFilterAccordion", diff --git a/frontend/tests/errors.test.ts b/frontend/tests/errors.test.ts index 11eb95d34..c509e68c9 100644 --- a/frontend/tests/errors.test.ts +++ b/frontend/tests/errors.test.ts @@ -1,5 +1,5 @@ -import { ParsedError } from "src/app/[locale]/search/error"; import { BadRequestError } from "src/errors"; +import { ParsedError } from "src/types/generalTypes"; import { QueryParamData } from "src/types/search/searchRequestTypes"; describe("BadRequestError (as an example of other error types)", () => { @@ -49,6 +49,7 @@ describe("BadRequestError (as an example of other error types)", () => { const { cause } = error as Error; const errorData = cause as ParsedError; + expect(errorData.details).toBeTruthy(); expect(errorData.details?.field).toEqual("fieldName"); expect(errorData.details?.message).toEqual("a more detailed message"); expect(errorData.details?.type).toEqual("a subtype"); diff --git a/frontend/tests/pages/search/page.test.tsx b/frontend/tests/pages/search/page.test.tsx index d296ec1d1..338fd8af4 100644 --- a/frontend/tests/pages/search/page.test.tsx +++ b/frontend/tests/pages/search/page.test.tsx @@ -57,12 +57,6 @@ jest.mock("react", () => ({ use: jest.fn((e: { [key: string]: string }) => e), })); -jest.mock("src/services/globalState/GlobalStateProvider", () => ({ - useGlobalState: () => ({ - agencyOptions: [], - }), -})); - const fetchMock = jest.fn().mockResolvedValue({ json: jest.fn().mockResolvedValue({ data: [], errors: [], warnings: [] }), ok: true, diff --git a/frontend/tests/services/globalState/GlobalStateProvider.test.tsx b/frontend/tests/services/globalState/GlobalStateProvider.test.tsx deleted file mode 100644 index ca7bb3ef4..000000000 --- a/frontend/tests/services/globalState/GlobalStateProvider.test.tsx +++ /dev/null @@ -1,55 +0,0 @@ -import { act, renderHook } from "@testing-library/react"; -import { - GlobalStateProvider, - useGlobalState, -} from "src/services/globalState/GlobalStateProvider"; - -import { ReactNode } from "react"; - -const wrapper = ({ children }: { children: ReactNode }) => ( - {children} -); - -describe("useGlobalState", () => { - afterEach(() => { - jest.resetAllMocks(); - }); - test("returns default global state", () => { - const { result } = renderHook(() => useGlobalState((state) => state), { - wrapper, - }); - - const { agencyOptions } = result.current; - - expect(agencyOptions).toEqual([]); - }); - test("allows updating global state", () => { - const { result, rerender } = renderHook( - () => useGlobalState((state) => state), - { wrapper }, - ); - - const { setAgencyOptions } = result.current; - - act(() => { - setAgencyOptions([ - { - id: "MOCK-NIST", - label: "Mational Institute", - value: "MOCK-NIST", - }, - ]); - }); - - rerender(); - - const { agencyOptions } = result.current; - expect(agencyOptions).toEqual([ - { - id: "MOCK-NIST", - label: "Mational Institute", - value: "MOCK-NIST", - }, - ]); - }); -});