-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add sorting for multiple attributes, improve default sorting #117
Conversation
This also refactors the orderBy logic in searchBooks, since that started to become some delicious spaghetti
This adds a publish_year SQL column which we will fill out with values once. The frontend no longer needs to extract the publish year, since the backend does it
This sorts for books with most ratings first instead of sorting by id. This is done with sorting by most ratings first, and fuzzy matching is still used when no books match a query
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.
Nice to have more sorting options!
I haven’t completely made up my mind on the rating sorting adding a filter, but I think it could be quite annoying to have a filter chosen for you automatically, like you say.
However, the toast and the badge on the filter button do make it quite obvious that a filter has been added.
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.
The sorting works well!
Whatever you decide about the rating sort adding a filter, we can document the decision, and that is probably fine.
|
What do you think, @johahor? |
|
Filter coming automatically is totally fine by me, and I think it's a good solution, but I want to adjust the toast to stay on the screen longer. Double the length of it if you ask me. |
Co-authored-by: Johannes Horn <johahor@stud.ntnu.no>
|
Good idea, added it now |
The default sorting has been improved. It now sorts for books with the most ratings instead of the book ID. If the user adds a search query, and we have no exact results, fuzzy matching is still used like before, and the most relevant books are shown first (the ones with the best word similarity).
This may be a bit confusing at first, so please feel free to ask if you have any questions.
Furthermore, the following sorting options have been added:
We also have a new column
publish_yearfor thebookstable. This is filled out once instead of using a generated column, and the value comes from pre-processing thepublish_dateand extracting the year.Important UX decision that needs feedback:
When the user sorts by "Rating: high to low", we automatically add a filter of at least 10 000+ ratings if the user doesn't have any other filter for this. An info toast is also shown.
Since we add this automatically, this may honestly be more annoying than useful. Please give me feedback on this, and we may remove this completely at any time.
Sorting by author name was explicitly not added. Please see the issue #100 for a discussion of this.
The PR size is again inflated by the PostgreSQL migration changes (approximately 5300 lines in total). The long
snapshot.jsonfiles do not require a review.Closes #64