Addons-server: Update link to ratings in reviewer tools

Created on 29 Nov 2018  路  18Comments  路  Source: mozilla/addons-server

Describe the problem and steps to reproduce it:

The current link for ratings in the reviewer tools points to the public ratings page, which is only available if the add-on itself is public. If it's disabled, it will respond with 404.

We also have an admin ratings page (/admin/models/ratings/rating) that will show ratings even for disabled add-ons, but only for admins.

What did you expect to happen?

What we should probably do is only linkify the text X reviews if the user has the Ratings:Moderate ~and Admin:Tools~ permission, pointing to /admin/models/ratings/rating/?addon=<id>. For users that don't have that permission (ie regular reviewers, they would still see that text, but without a link.

If that is too much effort, we can linkify the text always, but then should add a suffix like (admin only) so non-admins will be aware that the link is not supposed to work for them.

reviewer tools mentor assigned welcome p3 verified fixed triaged

All 18 comments

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.

I would like to work on this, from what I have understood is that

Screenshot 2020-02-25 at 9 17 40 PM

goes to error 404 page if the addon is not public.

and that hyperlink should be hidden if the user is a regular user & plugin is not public

else if the user is Ratings:moderator or Admin:tools and the plugin is not public then the link should point to /admin/models/ratings/rating

Is that correct?

Mentor: @eviljeff

So, this isn't as straightforward as it would seem at first glance. The link is within a jinja helper that's used by developer hub as well as reviewer tools. I don't think using the helper is gaining us much so my recommendation would be to include the template fragment directly in the reviewer tools template.

I.e. replace {{ reviews_link(addon) }} in https://github.com/mozilla/addons-server/blob/master/src/olympia/reviewers/templates/reviewers/addon_details_box.html#L88 with the contents of https://github.com/mozilla/addons-server/blob/master/src/olympia/ratings/templates/ratings/reviews_link.html and change the link to point to the admin tools page @wagnerand mentioned.

The link will need wrapping in a {% if ... to selectively not link the Reviews text if the reviewer doesn't have those permissions.

This issue requires a fair bit of knowledge of jinja2 template syntax so I'm replacing the contrib:goodfirstbug label with contrib:welcome. If you're still interested let me know if you have more questions @varunbankar

Hi @eviljeff Can I work on this. I have set up the server locally

@VishalCR7 okay

The Admin:Tools permission has been removed in the meantime, so we no longer need to check for that. I updated the initial comment.

@wagnerand I'm not sure what pages should be verified because I have the following results with disabled addons on AMO stage:

  • as an admin I can access the admin tools ratings, the models/ratings/rating/?addon=1004978 page from the addon review page.
  • a user only with Ratings:Moderate cannot view the review pages, so I added this permission to Reviewers: Add-ons group. This type of user can access the review page but when "x reviews" is clicked I get a 403 Forbidden.
  • users with Ratings:Moderate permission can only access Ratings Awaiting Moderation queue and Moderated Review log.

I have a question about scenario 2:

a user only with Ratings:Moderate cannot view the review pages, so I added this permission to Reviewers: Add-ons group. This type of user can access the review page but when "x reviews" is clicked I get a 403 Forbidden.

Did you use an account mozilla.com with a mozilla.com email? I am not entirely sure, but that might be necessary, since the link leads to the admin.

Did you use an account mozilla.com with a mozilla.com email? I am not entirely sure, but that might be necessary, since the link leads to the admin.

are there any users who have Ratings:Moderate but _don't_ have a @mozilla.com email address?

Did you use an account mozilla.com with a mozilla.com email? I am not entirely sure, but that might be necessary, since the link leads to the admin.

are there any users who have Ratings:Moderate but _don't_ have a @mozilla.com email address?

Yes, it's also used for the rating moderation in the reviwer tools. In hindsight, maybe using a new permission would be better. What do you think?

@wagnerand

  • the second scenario with a mozilla.com account is redirecting to admin reviews page. Removing the Ratings:Moderate - the x rating will no longer redirect, it's read only.

no URL

  • the third scenario (user with only Ratings:Moderate permission) and a mozilla.com e-mail address has the same results: no access to the review page.

are there any users who have Ratings:Moderate but _don't_ have a @mozilla.com email address?

Yes, it's also used for the rating moderation in the reviwer tools. In hindsight, maybe using a new permission would be better. What do you think?

we could do, though whatever the new permission is, it would still need a @mozilla.com email address to access the admin so showing the link if they can't access the admin seems incorrect to me.

are there any users who have Ratings:Moderate but _don't_ have a @mozilla.com email address?

Yes, it's also used for the rating moderation in the reviwer tools. In hindsight, maybe using a new permission would be better. What do you think?

we could do, though whatever the new permission is, it would still need a @mozilla.com email address to access the admin so showing the link if they can't access the admin seems incorrect to me.

Yes. So, does this need an additional check for is_staff?

are there any users who have Ratings:Moderate but _don't_ have a @mozilla.com email address?

Yes, it's also used for the rating moderation in the reviwer tools. In hindsight, maybe using a new permission would be better. What do you think?

we could do, though whatever the new permission is, it would still need a @mozilla.com email address to access the admin so showing the link if they can't access the admin seems incorrect to me.

Yes. So, does this need an additional check for is_staff?

that's what I was thinking in the PR

maybe @VishalCR7 can follow-up with a fix in another PR?

@wagnerand So, if I understand well, a scenario 2 user with an e-mail address other than mozilla.com should see the "x ratings" in the review page in read only, I could file an issue as a followup.
Let me know.

@wagnerand So, if I understand well, a scenario 2 user with an e-mail address other than mozilla.com should see the "x ratings" in the review page in read only, I could file an issue as a followup.
Let me know.

Correct. I had hoped we could avoid that to have more flexibility, but it seems we can't have both.

Was this page helpful?
0 / 5 - 0 ratings