Allow multiple sort fields in smart playlists #4214
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @deluan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the smart playlist functionality by allowing users to specify multiple fields for sorting. It introduces a new parsing logic for the Sort criteria field to handle comma-separated values and optional direction prefixes (+/-), while also implementing a specific behavior for the global Order="desc" that reverses the direction of all specified sort fields.
Highlights
- Multi-field Sorting: The
Sortcriteria field now supports a comma-separated list of fields, allowing smart playlists to be sorted by multiple criteria. - Field-specific Direction: Individual sort fields can now include a
+prefix for ascending or-prefix for descending order, overriding the default ascending order for that field. - Global Order Reversal: The global
Order="desc"setting now reverses the effective direction of all specified sort fields, including those with explicit+or-prefixes. - Robustness: Invalid sort fields are now logged and default to 'title', and invalid global
Ordervalues are logged and ignored. - Testing: New tests have been added to verify multi-field sorting and the global order reversal logic.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully implements support for multiple sort fields in smart playlists, including per-field direction using + and - prefixes. The code handles invalid sort fields and global order values gracefully. The new test cases adequately cover the added functionality, including multi-field sorting and the interaction between global and per-field order directions. Consider adding documentation to clarify the behavior when both global order and per-field direction prefixes are used.
model/criteria/criteria.go
Outdated
| if f == nil { | ||
| log.Error("Invalid field in 'sort' field. Using 'title'", "sort", sortField) | ||
| f = fieldMap["title"] |
There was a problem hiding this comment.
When an invalid field name is encountered in the sort string, it's logged and defaults to sorting by title. This is good error handling. The loop continues processing other valid fields in the comma-separated list, which is also reasonable behavior.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for multi-field sorting in smart playlists and updates tests to verify the new behavior.
- Implement sorting by multiple fields with direction flips based on a global order modifier.
- Add tests for multiple sort fields, order reversal, and handling of invalid sort fields.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| model/criteria/criteria_test.go | Added tests for multi-field sorting and order reversal. |
| model/criteria/criteria.go | Updated OrderBy to support multiple fields and global order reversal logic. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
OrderByTesting
make test PKG=./model/criteriamake testhttps://chatgpt.com/codex/tasks/task_b_68498e2d9d2c832ead3b27e698195e14