Skip to content

Add an explicit target_versions attribute on ContentAction classes#24789

Draft
diox wants to merge 1 commit intomozilla:masterfrom
diox:abuse-actions-target-versions-rebased
Draft

Add an explicit target_versions attribute on ContentAction classes#24789
diox wants to merge 1 commit intomozilla:masterfrom
diox:abuse-actions-target-versions-rebased

Conversation

@diox
Copy link
Copy Markdown
Member

@diox diox commented Apr 21, 2026

Having an actual target_versions attribute rather than a property depending on the decision will allow us to have logic that depends on the target versions inside the action class but still allow the target versions considered for that logic to be set by the caller before a decision has been made.

A set_target_versions() method is provided to keep the decision target versions in sync with the ContentAction.

Split from #24785
Helps mozilla/addons#16123

Having an actual target_versions attribute rather than a property
depending on the decision will allow us to have logic that depends
on the target versions inside the action class but still allow the
target versions considered for that logic to be set by the caller
before a decision has been made.

A set_target_versions() method is provided to keep the decision
target versions in sync with the ContentAction.
@diox
Copy link
Copy Markdown
Member Author

diox commented Apr 21, 2026

The end goal I'm after here is being able to add a should_be_skipped_by_automation() method on ContentAction. It would look like this in the base class (only caring about Addon entities for this demonstration):

def should_be_skipped_by_automation(self):
    return ContentDecision.objects.filter(self.addon, action=self.action).exists()

But then ContentActionRejectVersion would have this:

def should_be_skipped_by_automation(self):
    return ContentDecision.objects.filter(
        addon=self.target, versions__in=self.target_versions, action=self.action
    ).exists()

The problem is, I need to perform this check before the scanner decision has actually taken place. So I'm instantiating the relevant class without a saved ContentDecision, so I don't have access to decision.target_versions:

ContentActionClass = CONTENT_ACTION_FROM_DECISION_ACTION[action]
action_helper = ContentActionClass(ContentDecision(addon=addon))
action_helper.target_versions = Version.unfiltered.filter(pk=version.pk)
if not action_helper.should_be_skipped_by_automation():
    # Proceed ...

This is done in a loop for one or more actions, the action_helper and its decision are thrown away after this - I just needed some place to stick the should_be_skipped_by_automation() logic, and since it is action-dependent, the ContentAction classes was the logical place for it, but since I don't have an actual decision at that point, I need the ability to override target_versions being considered.

Alternatively, I could give up on using target_versions for this, and just pass the version to my should_be_skipped_by_automation() method, and have it ignore self.target_versions entirely - automation is always processing versions one by one anyway. Is that better ? Let me know what you think.

@diox diox marked this pull request as draft April 21, 2026 20:04
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.

1 participant