-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Task 2260] refactor search error page for maintainability #2434
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
d2a68f3
some error handling refactors, can be dropped if extraneous
doug-s-nava 01ebf1c
move things around to make error handling less cumbersome
doug-s-nava 18629e2
get error page working
doug-s-nava 3f5eb8c
cleanup
doug-s-nava ff63533
move results component and add test
doug-s-nava File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,17 @@ | ||
"use client"; | ||
|
||
// Error components must be Client Components | ||
import QueryProvider from "src/app/[locale]/search/QueryProvider"; | ||
import { SEARCH_CRUMBS } from "src/constants/breadcrumbs"; | ||
import { QueryParamData } from "src/services/search/searchfetcher/SearchFetcher"; | ||
import { ServerSideSearchParams } from "src/types/searchRequestURLTypes"; | ||
import { Breakpoints } from "src/types/uiTypes"; | ||
import { convertSearchParamsToProperTypes } from "src/utils/search/convertSearchParamsToProperTypes"; | ||
|
||
import { useTranslations } from "next-intl"; | ||
import { useEffect } from "react"; | ||
|
||
import BetaAlert from "src/components/BetaAlert"; | ||
import Breadcrumbs from "src/components/Breadcrumbs"; | ||
import PageSEO from "src/components/PageSEO"; | ||
import ContentDisplayToggle from "src/components/ContentDisplayToggle"; | ||
import SearchErrorAlert from "src/components/search/error/SearchErrorAlert"; | ||
import SearchBar from "src/components/search/SearchBar"; | ||
import SearchCallToAction from "src/components/search/SearchCallToAction"; | ||
import SearchFilterAccordion from "src/components/search/SearchFilterAccordion/SearchFilterAccordion"; | ||
import { | ||
agencyOptions, | ||
categoryOptions, | ||
eligibilityOptions, | ||
fundingOptions, | ||
} from "src/components/search/SearchFilterAccordion/SearchFilterOptions"; | ||
import SearchOpportunityStatus from "src/components/search/SearchOpportunityStatus"; | ||
import SearchResultsHeader from "src/components/search/SearchResultsHeader"; | ||
import SearchFilters from "src/components/search/SearchFilters"; | ||
|
||
interface ErrorProps { | ||
// Next's error boundary also includes a reset function as a prop for retries, | ||
|
@@ -31,138 +21,91 @@ interface ErrorProps { | |
|
||
export interface ParsedError { | ||
message: string; | ||
searchInputs: QueryParamData; | ||
searchInputs: ServerSideSearchParams; | ||
status: number; | ||
type: string; | ||
} | ||
|
||
function isValidJSON(str: string) { | ||
try { | ||
JSON.parse(str); | ||
return true; | ||
} catch (e) { | ||
return false; // String is not valid JSON | ||
} | ||
} | ||
|
||
function createBlankParsedError(): ParsedError { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems like there should be a better way but I think I'd need more context into our error throwing / handling patterns to take an informed approach. For now just tried to change as little as possible to keep this working |
||
return { | ||
type: "NetworkError", | ||
searchInputs: { | ||
query: "", | ||
status: "", | ||
fundingInstrument: "", | ||
eligibility: "", | ||
agency: "", | ||
category: "", | ||
sortby: undefined, | ||
page: "1", | ||
actionType: "initialLoad", | ||
}, | ||
message: "Invalid error message JSON returned", | ||
status: -1, | ||
}; | ||
} | ||
|
||
export default function Error({ error }: ErrorProps) { | ||
const t = useTranslations("Search"); | ||
|
||
// The error message is passed as an object that's been stringified. | ||
// Parse it here. | ||
|
||
let parsedErrorData; | ||
let convertedSearchParams; | ||
|
||
if (!isValidJSON(error.message)) { | ||
// the error likely is just a string with a non-specific Server Component error when running the built app | ||
// "An error occurred in the Server Components render. The specific message is omitted in production builds..." | ||
parsedErrorData = getParsedError(); | ||
convertedSearchParams = parsedErrorData.searchInputs; | ||
parsedErrorData = createBlankParsedError(); | ||
} else { | ||
// Valid error thrown from server component | ||
parsedErrorData = JSON.parse(error.message) as ParsedError; | ||
|
||
// The error message search inputs had to be converted to arrays in order to be stringified, | ||
// convert those back to sets as we do in non-error flow. | ||
convertedSearchParams = convertSearchInputArraysToSets( | ||
parsedErrorData.searchInputs, | ||
); | ||
} | ||
const { | ||
agency, | ||
category, | ||
eligibility, | ||
fundingInstrument, | ||
query, | ||
sortby, | ||
status, | ||
} = convertedSearchParams; | ||
const convertedSearchParams = convertSearchParamsToProperTypes( | ||
parsedErrorData.searchInputs, | ||
); | ||
const { agency, category, eligibility, fundingInstrument, query, status } = | ||
convertedSearchParams; | ||
|
||
useEffect(() => { | ||
console.error(error); | ||
}, [error]); | ||
|
||
return ( | ||
<> | ||
<PageSEO | ||
title="Search Funding Opportunities" | ||
description="Try out our experimental search page." | ||
/> | ||
<BetaAlert /> | ||
<Breadcrumbs breadcrumbList={SEARCH_CRUMBS} /> | ||
<SearchCallToAction /> | ||
<QueryProvider> | ||
<div className="grid-container"> | ||
<div className="search-bar"> | ||
<SearchBar query={query} /> | ||
</div> | ||
<div className="grid-row grid-gap"> | ||
<div className="tablet:grid-col-4"> | ||
<SearchOpportunityStatus query={status} /> | ||
<SearchFilterAccordion | ||
filterOptions={fundingOptions} | ||
title="Funding instrument" | ||
queryParamKey="fundingInstrument" | ||
query={fundingInstrument} | ||
/> | ||
<SearchFilterAccordion | ||
filterOptions={eligibilityOptions} | ||
title="Eligibility" | ||
queryParamKey="eligibility" | ||
query={eligibility} | ||
/> | ||
<SearchFilterAccordion | ||
filterOptions={agencyOptions} | ||
title="Agency" | ||
queryParamKey="agency" | ||
query={agency} | ||
/> | ||
<SearchFilterAccordion | ||
filterOptions={categoryOptions} | ||
title="Category" | ||
queryParamKey="category" | ||
query={category} | ||
<QueryProvider> | ||
<div className="grid-container"> | ||
<div className="search-bar"> | ||
<SearchBar query={query} /> | ||
</div> | ||
<div className="grid-row grid-gap"> | ||
<div className="tablet:grid-col-4"> | ||
<ContentDisplayToggle | ||
showCallToAction={t("filterDisplayToggle.showFilters")} | ||
hideCallToAction={t("filterDisplayToggle.hideFilters")} | ||
breakpoint={Breakpoints.TABLET} | ||
> | ||
<SearchFilters | ||
opportunityStatus={status} | ||
eligibility={eligibility} | ||
category={category} | ||
fundingInstrument={fundingInstrument} | ||
agency={agency} | ||
/> | ||
</div> | ||
<div className="tablet:grid-col-8"> | ||
<SearchResultsHeader sortby={sortby} /> | ||
<div className="usa-prose"> | ||
<SearchErrorAlert /> | ||
</div> | ||
</div> | ||
</ContentDisplayToggle> | ||
</div> | ||
<div className="tablet:grid-col-8"> | ||
<SearchErrorAlert /> | ||
</div> | ||
</div> | ||
</QueryProvider> | ||
</> | ||
</div> | ||
</QueryProvider> | ||
); | ||
} | ||
|
||
function convertSearchInputArraysToSets( | ||
searchInputs: QueryParamData, | ||
): QueryParamData { | ||
return { | ||
...searchInputs, | ||
status: new Set(searchInputs.status || []), | ||
fundingInstrument: new Set(searchInputs.fundingInstrument || []), | ||
eligibility: new Set(searchInputs.eligibility || []), | ||
agency: new Set(searchInputs.agency || []), | ||
category: new Set(searchInputs.category || []), | ||
}; | ||
} | ||
|
||
function isValidJSON(str: string) { | ||
try { | ||
JSON.parse(str); | ||
return true; | ||
} catch (e) { | ||
return false; // String is not valid JSON | ||
} | ||
} | ||
|
||
function getParsedError() { | ||
return { | ||
type: "NetworkError", | ||
searchInputs: { | ||
status: new Set(), | ||
fundingInstrument: new Set(), | ||
eligibility: new Set(), | ||
agency: new Set(), | ||
category: new Set(), | ||
sortby: null, | ||
page: 1, | ||
actionType: "initialLoad", | ||
}, | ||
message: "Invalid JSON returned", | ||
status: -1, | ||
} as ParsedError; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { SEARCH_CRUMBS } from "src/constants/breadcrumbs"; | ||
|
||
import { useTranslations } from "next-intl"; | ||
import { unstable_setRequestLocale } from "next-intl/server"; | ||
|
||
import BetaAlert from "src/components/BetaAlert"; | ||
import Breadcrumbs from "src/components/Breadcrumbs"; | ||
import PageSEO from "src/components/PageSEO"; | ||
import SearchCallToAction from "src/components/search/SearchCallToAction"; | ||
|
||
export default function SearchLayout({ | ||
children, | ||
}: { | ||
children: React.ReactNode; | ||
}) { | ||
unstable_setRequestLocale("en"); | ||
const t = useTranslations("Search"); | ||
return ( | ||
<> | ||
<PageSEO title={t("title")} description={t("meta_description")} /> | ||
<BetaAlert /> | ||
<Breadcrumbs breadcrumbList={SEARCH_CRUMBS} /> | ||
<SearchCallToAction /> | ||
{children} | ||
</> | ||
); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably move to a util, but didn't want to cause unnecessary churn creating a new file for it today