-
Notifications
You must be signed in to change notification settings - Fork 8
fix: no results on ena picker when pre filters do not match (#778) #789
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
Conversation
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.
Pull Request Overview
This PR fixes an issue where the ENA picker returned no results when pre-filters didn't match data by refactoring the column filter validation logic. The new approach validates the entire combination of filters rather than individual values, ensuring only filter combinations that match at least one table row are applied.
- Replaced individual column/value validation with combination-based validation
- Added utility function for consistent value-to-string-array conversion
- Changed validation to only apply filters when at least one row matches all filter criteria
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ents/Step/SequencingStep/components/ENASequencingData/components/CollectionSelector/utils.ts
Show resolved
Hide resolved
...ents/Step/SequencingStep/components/ENASequencingData/components/CollectionSelector/utils.ts
Show resolved
Hide resolved
|
|
||
| return columnFiltersState; | ||
| function toStringArray(value: unknown): string[] { | ||
| if (value == null) return []; |
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.
Checking == is intentional here.
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.
LOL, no. I let Windsurf write that function 😱 .
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.
LGTM, thanks F! Just one nit for you to review.
|
|
||
| return columnFiltersState; | ||
| function toStringArray(value: unknown): string[] { | ||
| if (value == null) return []; |
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.
| if (value == null) return []; | |
| if (value === null) return []; |
Closes #778.
This pull request refactors the logic for building validated column filters in the
CollectionSelectorutility by introducing a new approach that ensures only filter combinations that match at least one row in the table are applied. The previous implementation, which filtered out invalid columns and values individually, has been replaced with a stricter validation that checks the overall combination of filter values for validity. Additionally, a new utility function for value conversion was added.Key changes:
Column Filter Validation Logic:
buildValidatedColumnFiltersimplementation with a new version that only applies preselected filters if at least one row in the table matches all filters (AND across columns, OR within a column). If no such row exists, no filters are applied. This ensures only valid filter combinations are used.Utility Functions:
toStringArrayutility function to consistently convert values to string arrays, improving type safety and code clarity.