feat: Support for multi-value filters#2411
Conversation
This change converts the existing single-value ROM filter parameters to support multiple values. Users can now filter ROMs by multiple genres, franchises, collections, companies, age ratings, regions, and languages by repeating the respective query parameters in API requests. At the moment, setting multiple values for a filter will return ROMs that match any of the provided values (logical OR). A future change can introduce parameters like `genres_all (boolean)` to allow filtering ROMs that match all specified values (logical AND). The frontend has been updated to send single-value filters as arrays to maintain compatibility with existing UI components. At the moment, the UI does not support selecting multiple values, but this change lays the groundwork for it. NOTE: Breaking API change, as the filter parameters have changed names.
|
Running Code Quality on PRs by uploading data to Trunk will soon be removed. You can still run checks on your PRs using trunk-action - see the migration guide for more information. |
Test Results678 tests 677 ✅ 1m 17s ⏱️ Results for commit 647e99f. ♻️ This comment has been updated with latest results. |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
gantoine
left a comment
gantoine
left a comment
There was a problem hiding this comment.
this looks great! 2 questions:
- is the a migration needed for existing smart collections?
- do you plan to also change the ui and filters to support multi-select? if not lmk and i can backlog it
…tates, allowing null values for all filters. Update filter logic in the store and API services to accommodate new states. Enhance UI for better visibility and interaction with filter options in the gallery app.
…re, franchise, collection, company, age rating, region, and language. Update serialization logic to maintain query parameters on refresh.
…mproved visibility and styling.
…tionality; add FilterPlatformBtn component.
…e filter reset functionality
…res, franchises, collections, companies, age ratings, regions, and languages. Update API and frontend components to support new filter states and logic. Refactor gallery filter store and API service to accommodate multi-value selections and maintain backward compatibility with single-value filters. Improve UI for filter options in the gallery app, including logic toggle buttons for better user interaction.
… API and frontend components
…c operator parameters, streamline multi-platform handling in API and frontend components.
…management to clear single platform selection
…port multi-value filtering in API and frontend components
…ss various handlers and endpoints
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant and well-implemented support for multi-value filters on ROMs, a great enhancement for the API and frontend. The backend changes correctly adapt the database queries, and the frontend UI has been impressively revamped to support multi-selection and AND/OR logic. I've identified a couple of areas for improvement in the backend concerning input sanitization for robustness and clarity, and a minor code duplication issue on the frontend. Overall, this is a solid contribution.
…thod by removing match_all parameter and redundant AND logic
…or improved code reuse
…form IDs and remove legacy conversion logic
…ate boolean filter values to use true/false strings
…handling to support null values
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-implemented feature: multi-value ROM filters. The changes span the backend API, database handlers, and frontend components, enabling users to filter by multiple genres, franchises, and other criteria. The refactoring in both the backend and frontend is excellent, particularly the move to a more data-driven approach for applying filters and the improved UI for filter selection.
My review has identified a couple of areas for improvement. There's a bug in how smart collections handle multi-platform filters, and the 'Create Smart Collection' dialog hasn't been fully updated to support the new multi-value filtering capabilities. I've also made a suggestion to improve backward compatibility for existing smart collections. Addressing these points will make this already strong feature even more robust.
frontend/src/components/common/Collection/Dialog/CreateSmartCollection.vue
Outdated
Show resolved
Hide resolved
…ections for genres, franchises, collections, companies, age ratings, regions, and languages
…duce order_by and order_dir parameters
…r platforms, genres, franchises, collections, companies, age ratings, statuses, regions, and languages
…ed states based on URL parameters
…rDrawer components
zurdi15
changed the title
…andom game selection
zurdi15
changed the title
gantoine
left a comment
gantoine
left a comment
There was a problem hiding this comment.
very clean implementation and easy to review 🤘🏼
| companies = convert_legacy_filter("companies", "selected_company") | ||
| age_ratings = convert_legacy_filter("age_ratings", "selected_age_rating") | ||
| regions = convert_legacy_filter("regions", "selected_region") | ||
| languages = convert_legacy_filter("languages", "selected_language") |
There was a problem hiding this comment.
Actually, now self revieweing again the code I removed the old parameters. Should we remove the convert_legavy_filter or do you think it's better to re-add the old single-value parameters?
There was a problem hiding this comment.
good question, not sure but do what you think is best and we'll test in staging using exisiting smart collections.
There was a problem hiding this comment.
ili'll leave itt here to not mess with already existing smart collections
| predicate = RomUser.last_played.isnot(None) | ||
| if not value: | ||
| predicate = RomUser.last_played.is_(None) | ||
| return query.filter(predicate) |
| } | ||
| }); | ||
|
|
||
| return searchParams.toString(); |
3 participants
Description
This change converts the existing single-value ROM filter parameters to support multiple values. Users can now filter ROMs by multiple genres, franchises, collections, companies, age ratings, regions, and languages by repeating the respective query parameters in API requests. It also allow to filter with
andororlogic any of those parameters.Edit from @zurdi15: Added
last_playedas filter to fix #2816NOTE: Breaking API change, as the filter parameters have changed names.
Checklist
Screenshots: