Skip to content

Enable policy->action selection in reviewer tools, for negative policies#24918

Draft
eviljeff wants to merge 1 commit into
mozilla:masterfrom
eviljeff:16122-reviewer-policy-selection-action
Draft

Enable policy->action selection in reviewer tools, for negative policies#24918
eviljeff wants to merge 1 commit into
mozilla:masterfrom
eviljeff:16122-reviewer-policy-selection-action

Conversation

@eviljeff
Copy link
Copy Markdown
Member

@eviljeff eviljeff commented May 20, 2026

Fixes mozilla/addons#16122

Description

Support policy selections directly in reviewer tools, which create decisions based on the enforcement actions of the policies selected. In this (first) patch, only some reviewer tool actions are replaced (reject latest version, and disable addon)

Screenshot 2026-05-21 140053

Context

Follow-up pieces for this:

  • support non-negative enforcement actions - e.g. Approve, Approve Version, Ignore. Difficulty is we need to stop negative and non-negative policies from being selected at the same time as we can't order them in a coherent way.
  • support version selection. Difficultly is you select policies, with enforcement actions, and then those action determine if version selection is appropriate (disable no, reject yes) - and which versions are available for selection.
  • appeal jobs - I expect it'll be a separate reviewer tools action so we can keep the options different.
  • (overriding decisions. Similar to appeal handling in outcome, but we'll need to present a list of decisions to override, rather than a list of jobs)

Testing

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 16122-reviewer-policy-selection-action branch 2 times, most recently from 429f10f to 27b41f1 Compare May 21, 2026 10:56

@library.global_function
@jinja2.pass_context
def toggle_when_action(context, definition_key, *, default=False):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this because I was bored of constructing extra action_xxx lists each time we needed to toggle on something in a single place. It didn't scale well.

@eviljeff eviljeff force-pushed the 16122-reviewer-policy-selection-action branch from 27b41f1 to d00d898 Compare May 21, 2026 13:01
@eviljeff eviljeff marked this pull request as ready for review May 21, 2026 13:10
@eviljeff eviljeff requested a review from diox May 21, 2026 13:11
@eviljeff
Copy link
Copy Markdown
Member Author

Mmm, this needs more work - I overlooked that the reject_version action, that it replaces, still has some logic to clear needshumanreview we don't do in ContentActionRejectVersion. We either need to add in some special casing in the review_with_policy method, or elsewhere in the ReviewHandler class, or migrate it over to ContentActionRejectVersion.

It's something that's going to trip up the expansion to multiple versions and non-negative enforcment actions too - that even after we implemented reviewer tools using ContentAction there's still some logic that is reviewer tools specific that stayed.

@eviljeff eviljeff marked this pull request as draft May 22, 2026 16:37
@diox
Copy link
Copy Markdown
Member

diox commented May 22, 2026

Mmm, this needs more work - I overlooked that the reject_version action, that it replaces, still has some logic to clear needshumanreview we don't do in ContentActionRejectVersion. We either need to add in some special casing in the review_with_policy method, or elsewhere in the ReviewHandler class, or migrate it over to ContentActionRejectVersion.

I think it would make sense to migrate that logic over to ContentActionRejectVersion as well, if it's a human review... (and I guess there might be some special casing regarding when the NHR is cleared when something is held for 2nd level approval to pay attention to)

@diox
Copy link
Copy Markdown
Member

diox commented May 22, 2026

Note to myself: waffle switch for this PR is called enable-policy-review-selection.

@eviljeff
Copy link
Copy Markdown
Member Author

Note to myself: waffle switch for this PR is called enable-policy-review-selection.

better naming welcome

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

2 participants