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

[Bug]: All amo reviewer actions should be registered in Cinder's Decisions tab #14788

Closed
1 task done
ioanarusiczki opened this issue May 15, 2024 · 9 comments
Closed
1 task done
Assignees
Milestone

Comments

@ioanarusiczki
Copy link

ioanarusiczki commented May 15, 2024

What happened?

Followup for #14778 (comment)

  1. Force-enable an extension/theme or approve a new version. It's usually made to reinstate content when resolving an appeal.

What did you expect to happen?

Should be present in Decisions https://stage.cinder.nonprod.webservices.mozgcp.net/investigate/amo_addon/630846/decisions?filters=%7B%7D

Is there an existing issue for this?

  • I have searched the existing issues

┆Issue is synchronized with this Jira Task

@diox
Copy link
Member

diox commented May 15, 2024

Note: we don't need to cherry-pick this because a) resolving jobs in reviewer tools is not active right now and b) we could add the action in Cinder anyway. @eviljeff will investigate further what to do

@eviljeff eviljeff self-assigned this May 15, 2024
@ioanarusiczki
Copy link
Author

ioanarusiczki commented May 16, 2024

What I did not notice yesterday is that emails are not sent to the author about content being reinstated when force-enabling or approving (if the previous version was rejected on the back of a report).

And the jobs (appeals) have status "Open" in Cinder.

On rev tools side Appeals still show up in the DSA section. Another example , with force-enable resolving the appeal.

@eviljeff
Copy link
Member

yes, the call to the cinder api is failing because it doesn't understand the AMO_APPROVE_VERSION enforcement action.

@eviljeff
Copy link
Member

Fixed by adding the enforcement action to cinder (stage and prod).

If in the future there is a need upstream to not differentiate

  • AMO_APPROVE (report is wrong; content is okay; don't change state of add-on/version; don't notify developer) and
  • AMO_APPROVE_VERSION (report is wrong, if we had one; content is okay; change state of add-on/version to approved; notify the developer)

Then we can adjust then.

@eviljeff eviljeff added this to the 2024.05.30 milestone May 17, 2024
@ioanarusiczki
Copy link
Author

For the scenario with an add-on reported and moderated from rev tools I repeated an approve and force-enable action and now it looks good, actions are registered and emails sent.

I tried with https://stage.cinder.nonprod.webservices.mozgcp.net/investigate/amo_addon/630978/decisions?filters=%7B%7D

But I don't understand what's happening for escalated reports from Cinder to rev tools. For example, A reported extension or theme sent and rejected from rev tools seems to be force-disabled. I've noticed it when I tried uploading a new version and I didn't have "Upload New Version" button available

To give clear examples:
https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/630979
or https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/630982

I won't be able to continue investigating today but I wanted to give an update about it until I'll get back.

@eviljeff
Copy link
Member

@ioanarusiczki I know what's happening... it's technically not a new bug but the fix for #14778 is exposing it.

@eviljeff
Copy link
Member

(with #14778 cinder is now sending the decision back to us; CinderActionRejectVersion.process_action is supposed to be overridden with an empty method, but it's not, so we process the decision and end up calling CinderActionDisableAddon.process_action instead. Also, we don't need to process the decision once the cinder job is being handled in the reviewer tools)

@ioanarusiczki
Copy link
Author

I tested today with Cinder , all decisions made from there, I don't understand exactly how Ignore should work over appeals.

When used for a reporter appeal -> decision is "Deny appeal" but I didn't see an email sent to the reporter.

When used for an author appeal -> decision is "Appeal Adjustment" without an email sent to the author.

I've added this to my tracker #14776 and I'll probably file a followup to clarify it.

@eviljeff
Copy link
Member

I tested today with Cinder , all decisions made from there, I don't understand exactly how Ignore should work over appeals.

Ignore wasn't intended to be used for appeals so the behaviour is undefined. Probably shouldn't just do nothing though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants