[Issue #3518] download search results #3689

doug-s-nava · 2025-01-30T15:24:38Z

Summary

Time to review: 30 mins

Changes proposed

The goal here is adding a button to the search page that will allow users to download a csv file containing all (well, the first 5000) search results.

To accomplish this a few larger refactors were made:

fetch function service and helpers updated to be more flexible in order to allow a non-json response
some flaky e2e tests updated

Context for reviewers

Test steps

seed your database with a bunch more records. Update seed_local_db.py to up the size of each type by a bunch. Try updating size=5 to size=50. This is important for testing with a full width pagination. Then run make db-seed-local populate-search-opportunities
start a server on this branch with npm run dev
visit http://localhost:3000/search
VERIFY: you see an "Export results" button that matches designs
resize window to table and mobile widths
VERIFY: button placement and appearance matches expectations at each viewport width
click the button
VERIFY: a csv file downloads with filename grants-search-<timestamp>.csv)
open the file
VERIFY: the file contains the expected search result details in csv form

Additional information

…h world

doug-s-nava · 2025-01-31T15:35:39Z

frontend/src/services/fetch/fetchers/opportunityFetcher.ts

@@ -0,0 +1,11 @@
+import { OpportunityApiResponse } from "src/types/opportunity/opportunityResponseTypes";


since the bare fetcher no longer returns the json result, we'll create a basic wrapper here.

doug-s-nava · 2025-01-31T15:36:16Z

frontend/src/app/[locale]/search/error.tsx

@@ -19,7 +19,7 @@ import ServerErrorAlert from "src/components/ServerErrorAlert";

 export interface ParsedError {
  message: string;
-  searchInputs: ServerSideSearchParams;
+  searchInputs: OptionalStringDict;


just a rename to reflect that this type is generic rather than specific in any way

doug-s-nava · 2025-01-31T15:36:53Z

frontend/src/app/api/search/export/route.ts

+
+*/
+
+export async function GET(request: NextRequest) {


making API request here in order to avoid exposing the API key

Seems like a shame we have to do this. Maybe at some point we can open up some of the APIs for the general public.

Noting the API is a POST while we're using a GET, I think GET is the correct interpretation for search since a search request isn't meaningfully creating or updating server entities.

Agree it's not great, but the best option we have for now. We should maybe talk as a team about how we want to handle this sort of thing.

As far as the GET v POST, I'd be fine to make the first request as POST as well, but since it's largely internal and not sending a request body, a GET seemed fine. The search itself being a post makes sense to me since we're sending data in the body of the request.

doug-s-nava · 2025-01-31T15:38:38Z

frontend/src/services/fetch/fetcherHelpers.ts

- * Send a request and handle the response
- * @param queryParamData: note that this is only used in error handling in order to help restore original page state
- */
-export async function sendRequest<ResponseType extends APIResponse>(


since this function wasn't doing much, and what it was doing (reading a json response body) was making our implementation less flexible, I got rid of it and moved all this functionality either into the fetcher generator or endpoint specific wrappers

doug-s-nava · 2025-01-31T15:39:05Z

frontend/src/services/fetch/fetcherHelpers.ts

-// note that this will pass along filter inputs in order to maintain the state
-// of the page when relaying an error, but anything passed in the body of the request,
-// such as keyword search query will not be included
-function handleNotOkResponse(


similar to the above, this wasn't doing much, moved the behavior into the fetcher generator

doug-s-nava · 2025-01-31T15:39:51Z

frontend/src/services/fetch/fetchers/fetchers.ts


    return response;
  };
 }

 export const fetchOpportunity = cache(
-  requesterForEndpoint<OpportunityApiResponse>(fetchOpportunityEndpoint),


these all return Response now, so the generic isn't necessary any more

doug-s-nava · 2025-01-31T15:41:10Z

frontend/tests/e2e/search/search-loading.spec.ts

@@ -7,7 +7,7 @@ import {

 test.describe("Search page tests", () => {
  // Loadiing indicator resolves too quickly to reliably test in e2e.
-  skip("should show and hide loading state", async () => {
+  test.skip("should show and hide loading state", async () => {


the other implementation didn't seem to actually skip the test?

doug-s-nava · 2025-01-31T15:41:31Z

frontend/tests/e2e/search/search.spec.ts

@@ -156,12 +156,12 @@ test.describe("Search page tests", () => {
    page,
  }: PageProps) => {
    await page.goto("/search");
+    await waitForSearchResultsInitialLoad(page);


not sure why, but these tests got flaky. Adding the wait on initial page load seems to solve the problem

doug-s-nava · 2025-01-31T21:27:40Z

frontend/src/components/opportunity/OpportunityDocuments.tsx

@@ -16,9 +14,6 @@ interface OpportunityDocumentsProps {
  documents: OpportunityDocument[];
 }

-dayjs.extend(advancedFormat);
-dayjs.extend(timezone);


moved this into a central location

doug-s-nava · 2025-01-31T21:28:05Z

frontend/src/utils/dateUtil.ts

@@ -23,3 +30,5 @@ export function formatDate(dateStr: string | null): string {
  };
  return date.toLocaleDateString("en-US", options);
 }
+
+export const getConfiguredDayJs = () => dayjs;


to support the timestamp in the filename

acouch · 2025-02-03T16:59:23Z

frontend/src/components/USWDSIcon.tsx

@@ -11,6 +11,8 @@ interface IconProps {
 // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
 const sprite_uri = SpriteSVG.src as string;

+// height prop doesn't seem to work


Seems like this could be discussion rather than a code comment. The USWDS component doesn't work with SVGs as far as I recall. We could revisit that, was a while ago this was adopted.

true, I'll remove the comment. Ticket for follow up here: #3748

acouch

Looks great.

* adds a button to the search page that downloads search results in csv format * refactors fetch function service and helpers to be more flexible in order to allow a non-json response * updates some flaky e2e tests

doug-s-nava added 6 commits January 30, 2025 10:23

add button

8669ff8

wip requesting csv - handling a non json request

7900244

this works but need to take a crack at some code improvements

cdad8c0

fixing it up - removing dead stuff - moving things around in the fetc…

557cb87

…h world

testing and related fixes

3202931

add e2e test and fix some flakiness

061fe14

doug-s-nava commented Jan 31, 2025

View reviewed changes

doug-s-nava added 3 commits January 31, 2025 12:10

get download to work on safari

bb0ec06

styling fixes and filename

109dcce

skip test in safari

e33b130

doug-s-nava marked this pull request as ready for review January 31, 2025 21:26

doug-s-nava requested review from acouch, andycochran and btabaska as code owners January 31, 2025 21:26

doug-s-nava changed the title ~~Dschrashun/3518 download search results~~ [Issue #3518] download search results Jan 31, 2025

doug-s-nava commented Jan 31, 2025

View reviewed changes

acouch reviewed Feb 3, 2025

View reviewed changes

acouch previously approved these changes Feb 3, 2025

View reviewed changes

remove comment

3080d15

doug-s-nava dismissed acouch’s stale review via 3080d15 February 3, 2025 17:42

acouch approved these changes Feb 3, 2025

View reviewed changes

doug-s-nava merged commit e475e7c into main Feb 3, 2025
10 checks passed

doug-s-nava deleted the dschrashun/3518-download-search-results branch February 3, 2025 18:17

doug-s-nava mentioned this pull request Feb 3, 2025

[unticketed] fix webkit / chrome error with csv download filename #3756

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Issue #3518] download search results #3689

[Issue #3518] download search results #3689

doug-s-nava commented Jan 30, 2025 •

edited

Loading

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

acouch Feb 3, 2025

doug-s-nava Feb 3, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

doug-s-nava Jan 31, 2025

acouch Feb 3, 2025

doug-s-nava Feb 3, 2025

acouch left a comment

		@@ -0,0 +1,11 @@
		import { OpportunityApiResponse } from "src/types/opportunity/opportunityResponseTypes";

[Issue #3518] download search results #3689

[Issue #3518] download search results #3689

Conversation

doug-s-nava commented Jan 30, 2025 • edited Loading

Summary

Time to review: 30 mins

Changes proposed

Context for reviewers

Test steps

Additional information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acouch left a comment

Choose a reason for hiding this comment

doug-s-nava commented Jan 30, 2025 •

edited

Loading