Skip to content

Implement approvals in ContentActions#25054

Open
eviljeff wants to merge 4 commits into
mozilla:masterfrom
eviljeff:16239-reviewer-policy-selection-approve
Open

Implement approvals in ContentActions#25054
eviljeff wants to merge 4 commits into
mozilla:masterfrom
eviljeff:16239-reviewer-policy-selection-approve

Conversation

@eviljeff

@eviljeff eviljeff commented Jun 19, 2026

Copy link
Copy Markdown
Member

Fixes mozilla/addons#16239

Description

Adds support for approving versions, confirming approval of versions, and approving the addon (i.e. listing content approval) via policies in the reviewer tools.

image

Context

The split into two actions between negative and positive actions is only to have a cleaner list of policies for reviewers, and to be able to filter the versions list appropriately - you can see the underlying function call is the same. If it was desirable we could have a single list of policies (like Cinder), and add some (more) javascript to filter versions based on policies selected. But I think it's fine this way, if not actually preferable.

I've made a specific Add-on version approval policy in Cinder staging for this to work - otherwise there's no way to really differentiate whether we need amo-approve or amo-approve-version. Another plus to this is more clarity, that amo-approve is addon level action, and amo-addon-version is version level action.

Part of the size of this patch is I had to transplant a lot of code from reviewers/utils to abuse/actions for this - a consequence of not quite finishing the reviewer tools -> cinder action work in 2025 (2024?). But because there's a lot of commonality between approval for listed and unlisted, and confirming and approving, the end result will be - I really hope - more DRY than before.

the other culprit for the size is test_actions.py underwent yet-another-refactoring (sorry) as the previous pattern of having everything in a single base class and disabling certain tests with pass became too difficult to keep track of. The refactor to have the negative tests for negative ActionClasses in one mixin, and positive in the other, makes that pattern much less necessary.

Testing

Sync your Cinder policies + turn on waffle switch

Try:

  • approving a version
  • confirming an already approved version
  • approving a listing
  • (other scenarios such as: auto-approval; unlisted; promoted groups)

Everything should work externally as a now, e.g. emails for version approval; no emails for confirmation of approval.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@eviljeff eviljeff force-pushed the 16239-reviewer-policy-selection-approve branch from 6bb9e6f to 71bbeea Compare June 19, 2026 15:31
@eviljeff eviljeff marked this pull request as ready for review June 19, 2026 15:52
@eviljeff eviljeff requested a review from diox June 19, 2026 16:03
@eviljeff eviljeff changed the title Implement approvals in ContentActionApproveInitialDecision Implement approvals in ContentActions Jun 19, 2026
@diox

diox commented Jun 22, 2026

Copy link
Copy Markdown
Member

confirming an already approved version

That didn't work for me.

I selected Review -> Approve add-on version, selected the version, submitted, I can see Auto-Approval confirmed activity, with Approve Add-on version: Approve, or confirm approval of, add-on versions and the NHR is gone, but the auto-approval has not been confirmed for this version.

This was on the listed channel, where before/with the waffle switch off we' don't let you select which version to approve.

@diox

diox commented Jun 23, 2026

Copy link
Copy Markdown
Member

auto_approve currently depends on public action being available - it should be changed to use review_with_policy_approve when the waffle switch is on, now that you've disabled that action if the waffle switch is on. (+test)

Found out when I ran out of unconfirmed auto-approved add-ons and tried to auto-approve new ones, got a bunch of Version 1.0 was skipped because approval action was not available in the logs...

@diox diox left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment about auto_approve breakage

Comment thread src/olympia/reviewers/utils.py Outdated
Comment thread src/olympia/abuse/actions.py Outdated
@eviljeff

Copy link
Copy Markdown
Member Author

auto_approve currently depends on public action being available - it should be changed to use review_with_policy_approve when the waffle switch is on, now that you've disabled that action if the waffle switch is on. (+test)

Now that all the functionality has been moved into ContentActionApproveVersion I changed auto_approve.py to directly create the decision rather than take a round trip via ReviewHelper.

@diox diox self-requested a review June 23, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: Have reviewers select a policy rather than an action, for non-negative actions

2 participants