Skip to content
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

Inconsistency in rating sort & filter query parameters #9190

Closed
ziir opened this issue May 27, 2023 · 2 comments · Fixed by mozilla/addons-server#20770
Closed

Inconsistency in rating sort & filter query parameters #9190

ziir opened this issue May 27, 2023 · 2 comments · Fixed by mozilla/addons-server#20770
Labels
migration:2024 repository:addons-server Issue relating to addons-server

Comments

@ziir
Copy link

ziir commented May 27, 2023

Describe the problem and steps to reproduce it:

On the /api/v5/addons/search API endpoint, one can filter & sort add-ons by rating.
For example, a query used for searching add-ons with an average rating of 4 and sorting the results by rating would like this:

  • /api/v5/addons/search/?ratings__gte=4&sort=rating

What happened?

The query parameters for filtering & sorting add-ons by ratings aren't named consistently which can be confusing & error-prone, especially since, in the endpoint documentation, sort parameters & threshold style filter parameters are presented next to each other.

View docs screenshot

Screenshot from 2023-05-27 23-53-47

What did you expect to happen?

The ratings related object property & sort/filter query parameters should be named consistently.

Anything else we should know?

ratings being the property actually returned in the addon details object, it might be preferable to:

  • support the sort=ratings query parameter.
  • keep the sort=rating query parameter as an alias.
  • update documentation to mention only sort=ratings query parameter for consistency / DX.
  • make the "sorting parameters" & "threshold style filtering parameters" sections in the endpoint documentation stand out more from each other.

Additionally, glancing at src/olympa/search/filters.py, it appears the filtering & sorting logic currently do not leverage the same ratings field: the ratings filter relies on the ratings.average field, whereas the rating sort relies on bayesian_rating, which might or might not be intentional, may lead to odd behavior.

┆Issue is synchronized with this Jira Task

@diox
Copy link
Member

diox commented May 29, 2023

Additionally, glancing at src/olympa/search/filters.py, it appears the filtering & sorting logic currently do not leverage the same ratings field: the ratings filter relies on the ratings.average field, whereas the rating sort relies on bayesian_rating, which might or might not be intentional, may lead to odd behavior.

This is somewhat confusing if someone ends up comparing the 2 fields values yes, but this was done on purpose... 15 years ago. The idea is to solve the classic issue with ratings sort of something with a single 5 star rating being ranked before a super popular 4.9 one. To combat this problem, we use a bayesian rating to weigh the rating (See https://web.archive.org/web/20060423210706/http://www.thebroth.com/blog/118/bayesian-rating).

Only sorting is affected though, for filtering and displaying purposes we use the actual average, mostly out of concern that some users would do the math themselves and be confused that it's not a straight average.

We could revisit all of that, but that is a product decision to make, not an engineering call.

ziir referenced this issue in ziir/addons-server May 29, 2023
- Ensure "sort=ratings" is supported, preferred
- Ensure "sort=rating" is still supported, for backwards compatibility
- Update existing tests using "sort=rating" to use the new preferred parameter

See https://github.com/mozilla/addons-server/issues/20763
ziir referenced this issue in ziir/addons-server May 29, 2023
ziir referenced this issue in ziir/addons-server Jun 1, 2023
Mention sort=ratings parameter being preferred.
Mention sort=rating parameter being still supported for backwards-compatibility but deprecated.
See https://github.com/mozilla/addons-server/issues/20763
diox referenced this issue in mozilla/addons-server Jun 1, 2023
* Add sort=ratings sorting param for consistency

See https://github.com/mozilla/addons-server/issues/20763

* Add test_sort_ratings search filters test

- Ensure "sort=ratings" is supported, preferred
- Ensure "sort=rating" is still supported, for backwards compatibility
- Update existing tests using "sort=rating" to use the new preferred parameter

See https://github.com/mozilla/addons-server/issues/20763

* Update addons search endpoint ratings sort documentation

Mention sort=ratings parameter being preferred.
Mention sort=rating parameter being still supported for backwards-compatibility but deprecated.
See https://github.com/mozilla/addons-server/issues/20763

* Update v5 API changelog for Addons search endpoint sort=ratings param renaming
@KevinMind
Copy link
Contributor

@KevinMind KevinMind transferred this issue from mozilla/addons-server May 4, 2024
@KevinMind KevinMind added repository:addons-server Issue relating to addons-server migration:2024 labels May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration:2024 repository:addons-server Issue relating to addons-server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants