Feature/dr 4029/facet filters#537
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
8ff45db to
bd390a1
Compare
This manifest method is unused and was previously removed.
15a57ca to
809eb4b
Compare
a160546 to
3fb29ed
Compare
3fb29ed to
c7bd976
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for dynamic “facet filter” option loading by introducing a new Next.js API route that proxies facet-option requests to the Collections API, and updating the filter modal to fetch its option list based on the current search query params.
Changes:
- Added
CollectionsApi.getFacetOptionsfor retrieving facet option lists from the Collections API. - Added
/api/search/facets/[facet]route handler to expose facet options to the client. - Updated
SelectFilterModalto fetch facet options on open and filter/paginate them client-side (plus related test mocks).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package-lock.json | Large dependency/lockfile update included in the PR. |
| app/src/utils/fetchApi/fetchApi.ts | Replaced logger usage with console.error in error handling. |
| app/src/utils/apiClients/apiClients.tsx | Added Collections API client method for facet option retrieval. |
| app/src/components/search/filters/selectFilterModal.tsx | Fetches facet options via new API route; client-side filtering/pagination. |
| app/src/components/search/filters/selectFilterModal.test.tsx | Added fetch mocking for newly introduced facet-option requests. |
| app/src/components/search/filters/selectFilterGrid.test.tsx | Updated navigation mocks to include useSearchParams. |
| app/src/components/search/filters/selectFilter.test.tsx | Updated navigation mocks to include useSearchParams. |
| app/api/search/facets/[facet]/route.tsx | New route handler to proxy facet-option requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const GET = async ( | ||
| request: NextRequest, | ||
| { params }: { params: { facet: string } } | ||
| ) => { | ||
| const searchParams = request.nextUrl.searchParams; | ||
| const q = searchParams.get("q") || ""; | ||
| const filters = searchParams.get("filters") || ""; | ||
| try { | ||
| const response = await CollectionsApi.getFacetOptions( | ||
| params.facet, | ||
| q, | ||
| filters | ||
| ); | ||
| return NextResponse.json(response, { status: 200 }); | ||
| } catch (error: any) { | ||
| return NextResponse.json( | ||
| { error: error?.message || "Unknown error" }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
There are Jest API tests for other route handlers under __tests__/api/*, but this new /api/search/facets/[facet] route doesn’t have coverage. Please add tests for the 200 path (returns facet options) and an error path (upstream failure → 500) to keep API handler behavior stable.
- Remove unused state - pass query string from params a dependency - handle non-ok response from API
Ticket:
This PR does the following:
Open questions
How has this been tested? How should a reviewer test this?
Accessibility concerns or updates
Checklist: