Conversation
3bcf136 to
9b7a4d8
Compare
|
e2e test failures seem unrelated. The order of the faces is swapped? |
|
|
|
@YarosMallorca I don't think it's required actually (but best of course). The rating could already be undefined for 'not set' which translates to null in the app right? And when updating the rating from the app, the request to PUT /api/assets/:id will auto transform the |
|
Oh yes, correct. Missed that it converts 0 to null. Then it's all fine! Actually, the mobile app can't send nulls apparently anyway. |
danieldietzler
left a comment
There was a problem hiding this comment.
This looks great actually, thanks!
| "rating": "Star rating", | ||
| "rating_clear": "Clear rating", | ||
| "rating_count": "{count, plural, one {# star} other {# stars}}", | ||
| "rating_count": "{count, plural, =-1 {Rejected} =0 {Unrated} one {# star} other {# stars}}", |
There was a problem hiding this comment.
What does "Rejected" even mean in this context? Usually it's rather bad to use a magic number like -1 to report such a state
There was a problem hiding this comment.
It was introduced as being a valid exif rating in #15699. I can imagine using workflows or scripts you would use it to auto-archive assets, or when using an external tool to browse your library, etc. Unless you search for a rating and then explicitly change the url to use rating=-1, you'll never see that string.
There was a problem hiding this comment.
We don't really see that use case. Could you update the PR to only support 1-5, and everything else becomes invalid (unrated)
Description
Small refactor of the star rating system:
nullto mean 'unrated'. 0 is not used anymore, metadata extract/write jobs convert from/to 0 to ensure proper values are saved.rating=0torating=nullFixes #26139
Changes to mobile app can be done separately because the server migrates to null and further changes with
{ rating: 0 }are auto transformed to null as well.How Has This Been Tested?
API Changes
nullas a rating filternullas a valid rating to writerating: 0is transformed tonullnumber | nullas the rating data type)Please describe to which degree, if any, an LLM was used in creating this pull request.
None