Fix product export with 'Current search' filter option#6254
Fix product export with 'Current search' filter option#6254witoszekdev merged 14 commits intosaleor:mainfrom
Conversation
- Add search and channel to export filter builder - Fall back to ALL scope when no filters are active - Resolves REQUIRED filter error on export Closes saleor#5985
🦋 Changeset detectedLatest commit: b54b91f The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
|
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where exporting products with the "Current search" filter option failed when no active filters were present. The backend's exportProducts mutation requires either a non-empty filter when using scope: FILTER, ids array when using scope: IDS, or just scope: ALL without filters. The dashboard was incorrectly sending an empty filter object {} with scope: FILTER, causing validation errors.
Changes:
- Added
getExportProductFilter()function to build proper export filters from query parameters - Modified export submission logic to intelligently fall back to
scope: ALLwhen no filters are active - Ensured search terms, channel, and other active filters are properly included in export mutations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/products/views/ProductList/filters.ts | Implements new getExportProductFilter() function to construct ProductFilterInput from URL query parameters |
| src/products/views/ProductList/ProductList.tsx | Updates export submission logic to use the new filter function and fall back to ALL scope when appropriate |
| .changeset/fuzzy-coins-arrive.md | Adds changeset documenting the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…' scope - Add getExportProductFilter() to extract active filters from URL params - Fall back to ALL scope when FILTER scope has no active criteria - Use ExportScope enum for type safety - Reduce code duplication, improve TypeScript typing - Add 21 comprehensive test cases (27 total tests passing) Fixes saleor#5985
witoszekdev
left a comment
witoszekdev
left a comment
There was a problem hiding this comment.
Thanks @iharshyadav for your contribution!
I think we can merge this PR, but I would like us to check if we can re-use existing logic for building query variables. This will make maintenance easier for us, since we will need to update our logic in only once place :)
There was a problem hiding this comment.
issue: We're duplicating here logic to create ProductFilterInput which our codebase already has in ConditionalFilter. This is problematic because we already have quite extensive logic for building query variables based on filter selection. It also supports building both FILTER and WHERE APIs (export still uses legacy FILTER API).
We could replace this method with:
export const createProductQueryVariablesLegacyInput = (filterContainer: FilterContainer): ProductFilterInput => {
const builder = new FiltersQueryBuilder<ProductQueryVars, "channel">({
apiType: QueryApiType.FILTER,
filterContainer,
});
const { filters } = builder.build();
return { ...filters };
};We would need to get data from filter container though, not query variables (should be somewhere on product list page).
This should work, but if it doesn't please let me know since it might mean we have bug somewhere else 🙏🏻
There was a problem hiding this comment.
Thanks for the suggestion! Refactored to use FiltersQueryBuilder with FILTER API type as you recommended.
I added a normalization step to handle field name conversions (singular to plural) and price format handling, since the FILTER API output differs slightly from what the export endpoint expects. All tests are passing now.
Let me know if this approach looks good or if there's a better way to handle the normalization!🙏🏻
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix type safety by replacing 'any' with Partial<ProductFilterInput>
- Handle price = 0 edge case with strict equality checks
- Add null type guard in ProductList.tsx
- Return null instead of {} for consistency with InputMaybe type
- Strengthen test assertions and add edge case coverage (18 tests total)
- Improve JSDoc documentation for FILTER vs WHERE API differences
...mponents/ConditionalFilter/FiltersQueryBuilder/queryVarsBuilders/ProductExportFieldMapper.ts
Show resolved
Hide resolved
| import { ApolloClient } from "@apollo/client"; | ||
| import * as Sentry from "@sentry/react"; | ||
|
|
There was a problem hiding this comment.
This file reports mapping errors via @sentry/react directly, while other ConditionalFilter code reports errors through the errorTracker abstraction (e.g. src/components/ConditionalFilter/FilterElement/Condition.ts:1,119). To keep error reporting consistent and configurable, prefer using errorTracker.captureException(...) here as well and avoid importing Sentry directly in new code.
|
|
||
| let filterValue: string[]; | ||
|
|
||
| if (typeof value === "object" && value !== null) { |
There was a problem hiding this comment.
we need to do this because export input has special type
witoszekdev
left a comment
witoszekdev
left a comment
There was a problem hiding this comment.
I did some changes from my side, so that we re-use existing queryVariables logic; @lkostrowski please take a look as well :)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6254 +/- ##
==========================================
+ Coverage 42.68% 42.76% +0.07%
==========================================
Files 2505 2507 +2
Lines 43411 43508 +97
Branches 10247 10277 +30
==========================================
+ Hits 18532 18607 +75
- Misses 23562 23583 +21
- Partials 1317 1318 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| const builder = new FiltersQueryBuilder<ProductFilterInput>({ | ||
| apiType: QUERY_API_TYPES.PRODUCT_EXPORT, | ||
| filterContainer, | ||
| filterDefinitionResolver: productExportFilterResolver, | ||
| }); | ||
| const { filters } = builder.build(); | ||
|
|
||
| // Return null if no filters remain (backend requirement) | ||
| return Object.keys(filters).length === 0 ? null : filters; |
There was a problem hiding this comment.
createProductExportQueryVariables relies on the default query-var builders for most fields. For the product price expression filter, the default mapping produces string values (UI number inputs emit strings) and for the is operator it yields a primitive (e.g. price: "200") rather than a PriceRangeInput ({ gte?: Float; lte?: Float }) required by ProductFilterInput. This will likely make exportProducts fail with GraphQL variable coercion errors when a price filter is active. Consider adding a dedicated query vars builder for product-export price that (1) parses values to numbers and (2) maps the is operator to { gte: value, lte: value }, and register it in productExportFilterResolver before the default builders.
witoszekdev
added
the
test deployment
4 participants
Description
Fixes #5985 - The export mutation was failing with "You must provide filter input" error when using the "Current search" export scope without active filters.
Root Cause
The backend's
exportProductsmutation requires either:filterwhenscope: FILTERidsarray whenscope: IDSscope: ALL(no filter needed)The dashboard was sending
scope: FILTERwith an empty filter{}, causing the backend to reject it with a REQUIRED validation error.Changes
getExportProductFilter()function insrc/products/views/ProductList/filters.tsto build proper export filter from current view's search, channel, and other active filterssrc/products/views/ProductList/ProductList.tsxto intelligently fall back toscope: ALLwhen no filters are activeFILTERScreenshots
Before (Bug)
After (Fixed)
What Didn't Work
What Works Now
Scope of the change
Testing
Checklist
pnpm run lintandpnpm run check-types