Skip to content

Don't check auto-approval restrictions for enterprise versions - #25413

Merged
jasonBirchall merged 2 commits into
masterfrom
enterprise-restriction-exempt
Sep 10, 2026
Merged

Don't check auto-approval restrictions for enterprise versions#25413
jasonBirchall merged 2 commits into
masterfrom
enterprise-restriction-exempt

Conversation

@jasonBirchall

@jasonBirchall jasonBirchall commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Part of mozilla/addons#16408

Context

While reviewing #25344, QA pulled up enterprise addons still passing the transparency check. It turns out this existed before the aforementioned PR.

Description

After this PR, Version.from_upload() now skips the auto-approval restriction check entirely for enterprise-channel versions. Enterprise versions are exempt from the auto-approval-disabled flags anyway (AutoApprovalSummary.check_has_auto_approval_disabled()), so the check was recording a "disabled" that never applied — and, as a side effect, setting the unlisted flag on the add-on, since the flag selection predates the enterprise channel. Submission-type restrictions are unaffected.

Testing

New test in TestExtensionVersionFromUpload: a matching "Add-on Approval" restriction plus an enterprise submission.

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.

Skip the check entirely for enterprise: no flag, no history, no activity
log.
@jasonBirchall
jasonBirchall requested a review from diox September 9, 2026 12:45
Comment thread src/olympia/versions/models.py Outdated
Comment on lines +582 to +584
if channel != amo.CHANNEL_ENTERPRISE:
checker = RestrictionChecker(upload=upload)
if not checker.is_auto_approval_allowed():

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.

instantiating the RestrictionChecker is not costly, so I would replace that with:

        checker = RestrictionChecker(upload=upload)
        if channel != amo.CHANNEL_ENTERPRISE and not checker.is_auto_approval_allowed():

And that way you don't have to introduce a new indentation level just for this...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure thing. Changed here eef06d5

Review feedback: as the RestrictionCheck is not costly, it's okay to
fold it into the existing condition instead of adding an indentation
level.
@jasonBirchall
jasonBirchall merged commit 5e222bd into master Sep 10, 2026
91 of 92 checks passed
@jasonBirchall
jasonBirchall deleted the enterprise-restriction-exempt branch September 10, 2026 09:27
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.

2 participants