mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
128 stars 41 forks source link

Incorrect extension star rating for extension displayed on https://addons.mozilla.org/en-US/developers/addons #8277

Closed st333v closed 2 years ago

st333v commented 3 years ago

Describe the problem and steps to reproduce it:

when i visit https://addons.mozilla.org/en-US/developers/addons it displays my extension as having a "5 out of 5 stars" rating however when i go to the product page at https://addons.mozilla.org/en-US/firefox/addon/revert-site/ the correct star rating is shown, which is 4.6

What happened?

it didnt show the correct star rating on the addons overview page

What did you expect to happen?

it should show the correct star rating on the addons overview page

Anything else we should know?

(Please include a link to the page, screenshots and any relevant files.)

diox commented 3 years ago

devhub (and reviewer tools) doesn't handle half-stars so it currently rounds to the nearest integer. The fix would need to not only display the correct tooltip text, but also correctly display half-stars so the image sprite + corresponding style needs to be updated as well.

st333v commented 3 years ago

personally i would be ok with it just showing the correct numerical value rating, the image thing is just icing on the cake. i think most people should be able to figure out 4.6 is almost 5 stars, so would let it go if its actually showing 5 star images, so maybe a partial fix would be fine, i wonder if doing the partial fix would be easy enough for a beginner to fix with a quick code change.

diox commented 3 years ago

I think we should be consistent with how we display it on the frontend pages, so we should support half-star images and display the proper numerical value in the tooltip. It's less approachable for a beginner and it's low priority for us, but someone who knows their way around CSS should be able to pick up the relevant code & image from the frontend to replace it in addons-server as well - I can give pointers to what python code would then need to be modified to stop the rounding to the next integer.

st333v commented 3 years ago

i see, thanks for the reply.

bhushan-borole commented 3 years ago

Hey @diox I would like to give it a try.

diox commented 3 years ago

The code responsible for the ratings is this helper: https://github.com/mozilla/addons-server/blob/ebff8e2f878bc3ac2012fd56a4b8e0e1cc4247c4/src/olympia/ratings/templatetags/jinja_helpers.py#L9-L19

As you can see it round()s the value to the nearest integer before rendering with this template, which generates a css class that is defined in a couple different places depending on the page. For the devhub "landing page", that is https://github.com/mozilla/addons-server/blob/ebff8e2f878bc3ac2012fd56a4b8e0e1cc4247c4/static/css/devhub/new-landing/base.less#L156-L186, for the "all submissions" page that is https://github.com/mozilla/addons-server/blob/ebff8e2f878bc3ac2012fd56a4b8e0e1cc4247c4/static/css/impala/ratings.less#L118-L148

As mentioned in comment above the idea would be to remove the rounding and adjust the style so that it works with half-stars in both places where they appear in devhub. For that you're free to either find a way to change the existing CSS (the background image sprite already kinda supports half stars, but it probably needs to be tweaked to make that easier to use) or backport the SVG technique used by addons-frontend instead. Whatever is easiest for you.

bhushan-borole commented 3 years ago

@diox

If I remove the round(), then it can be also 3.7 for an example. So how to handle that?

diox commented 3 years ago

You could round it to the closest integer or half-value. So if it's 3.7 you'd round to 3.5, if it's 3.8 you'd round to 4. This is essentially what we do in addons-frontend.

bhushan-borole commented 3 years ago

Okay will do that. And how do I test this? @diox

bhushan-borole commented 3 years ago

Since the CSS class names cannot contain ., should I just remove the . or replace it with -. Please let me know how should I name the class @diox

diox commented 3 years ago

Replacing with - sounds good.

To test this, first you can write a test in src/olympia/ratings/tests/test_helpers.py that ensures you're generating the right class names.

Then you can test the CSS part visually by using a local addons-server install to upload an add-on, making it public and going to its product page to post a few ratings, then going back to devhub to see the rating. A couple notes for this:

Contact me on Matrix if you require more help.

bhushan-borole commented 3 years ago

That sounds good.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.