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

delegate review actions to CinderActions #1982

Closed
eviljeff opened this issue Jan 26, 2024 · 4 comments · Fixed by mozilla/addons-server#23004
Closed

delegate review actions to CinderActions #1982

eviljeff opened this issue Jan 26, 2024 · 4 comments · Fixed by mozilla/addons-server#23004
Assignees
Milestone

Comments

@eviljeff
Copy link
Member

eviljeff commented Jan 26, 2024

Currently we carry out the review actions of, for example, disabling an add-on by:

  • in the reviewer tools call force_disable
  • and then call a task which notifies the cinder API of the decision, while sending out notification emails

This means we have duplicated places where we carry out the "disable addon" action, when for consistency it should be a single place.

┆Issue is synchronized with this Jira Task

@KevinMind
Copy link
Contributor

@ioanarusiczki
Copy link

ioanarusiczki commented Jan 28, 2025

I looked over the main scenarios and some additional (with abuse reports attached) but I'm going to complete it later (noticed something and atm not sure how to interpret that so I'll re-test)

  • emails sent to developer at disable are ok ✅
  • activity logs are registered but the reason, decision/policy would only be present in the reviewer pages ❓ - not sure if ok reading instructions
  • 2nd level approvals have a "Held" activity log ✅
  • not sure for 2nd level approvals with "approve content instead" if it's alright that it's not being registered anywhere (activity log or rev tools) ❓

activity log at disable:

Image

More detailed testing:

normal add-on disabled, email is sent, activity log registered ✅
https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/635672

recommended add-on disabled, "proceed with action" from 2nd level approval queue disabled the add-on and sends an email ✅
https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/635673

line add-on disabled, "proceed with action" from 2nd level approval queue disabled the add-on and sends an email ✅
https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/635674

spotlight add-on disabled ✅

notable add-on disabled, approve content instead -> does not send an email , content stays enabled ✅
https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/635676

@diox
Copy link
Member

diox commented Jan 28, 2025

not sure for 2nd level approvals with "approve content instead" if it's alright that it's not being registered anywhere (activity log or rev tools) ❓

Approving instead leads to a silent approval. To me this was intended, but I'm not sure whether the expected behavior was described anywhere, maybe it should be more explicit in the logs. Can you file a follow-up for this and we'll triage it ? Thanks.

@ioanarusiczki
Copy link

About 2nd level approvals with abuse reports:

  • there's an abuse report that flags the version in rev tools
  • it's rejected/disabled from the manual review queue the version is sent to 2nd level approval queue
  • from 2nd level approval queue a decision is taken: proceed or approve

Only after decision is taken in 2nd level approval queue the NHR and due date are cleared from the Manual Review Queue. -> It sounds like it should be the expected behavior but not 100% sure ❓

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

Successfully merging a pull request may close this issue.

4 participants