Skip to content

Commit

Permalink
Make initial auto approval delay for listed configurable by admins (#…
Browse files Browse the repository at this point in the history
…22725)

* Make initial auto approval delay for listed configurable by admins

* Fix dry-run verdict display

* Move the delay in this test to a variable
  • Loading branch information
diox authored and KevinMind committed Oct 2, 2024
1 parent bb922c3 commit f1015a7
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/olympia/constants/reviewers.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@
AUTO_APPROVAL_VERDICT_CHOICES = (
(
WOULD_NOT_HAVE_BEEN_AUTO_APPROVED,
'Would have been auto-approved (dry-run mode was in effect)',
'Would *not* have been auto-approved (dry-run mode was in effect)',
),
(
WOULD_HAVE_BEEN_AUTO_APPROVED,
'Would *not* have been auto-approved (dry-run mode was in effect)',
'Would have been auto-approved (dry-run mode was in effect)',
),
(AUTO_APPROVED, 'Was auto-approved'),
(NOT_AUTO_APPROVED, 'Was *not* auto-approved'),
Expand Down
9 changes: 8 additions & 1 deletion src/olympia/reviewers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from olympia.users.utils import get_task_user
from olympia.versions.models import Version, version_uploaded
from olympia.versions.utils import get_staggered_review_due_date_generator
from olympia.zadmin.models import get_config


user_log = olympia.core.logger.getLogger('z.users')
Expand Down Expand Up @@ -651,11 +652,17 @@ def check_should_be_delayed(cls, version):
content_review = addon.addonapprovalscounter.last_content_review
except AddonApprovalsCounter.DoesNotExist:
content_review = None
INITIAL_AUTO_APPROVAL_DELAY_FOR_LISTED = get_config(
'INITIAL_AUTO_APPROVAL_DELAY_FOR_LISTED',
default=24 * 60 * 60,
int_value=True,
)
return (
not is_langpack
and version.channel == amo.CHANNEL_LISTED
and version.addon.status == amo.STATUS_NOMINATED
and now - version.created < timedelta(hours=24)
and now - version.created
< timedelta(seconds=INITIAL_AUTO_APPROVAL_DELAY_FOR_LISTED)
and content_review is None
)

Expand Down
53 changes: 52 additions & 1 deletion src/olympia/reviewers/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
)
from olympia.users.models import UserProfile
from olympia.versions.models import Version, VersionReviewerFlags, version_uploaded
from olympia.zadmin.models import set_config


class TestReviewerSubscription(TestCase):
Expand Down Expand Up @@ -1229,7 +1230,7 @@ def test_check_should_be_delayed(self):
assert AutoApprovalSummary.check_should_be_delayed(self.version) is True

# Update the creation date so it's old enough to be not delayed.
self.version.update(created=self.days_ago(2))
self.version.update(created=datetime.now() - timedelta(hours=24, seconds=1))
assert AutoApprovalSummary.check_should_be_delayed(self.version) is False

# Unlisted shouldn't be affected.
Expand All @@ -1241,6 +1242,34 @@ def test_check_should_be_delayed(self):
AutoApprovalSummary.check_has_auto_approval_disabled(self.version) is False
)

def test_check_should_be_delayed_dynamic(self):
# The delay defaults to 24 hours (see test above) but can be configured
# by admins.
target_delay = 666
set_config('INITIAL_AUTO_APPROVAL_DELAY_FOR_LISTED', target_delay)
# Delete current_version, making self.version the first listed version
# submitted and add-on creation date recent.
self.addon.current_version.delete()
self.addon.update(created=datetime.now())
self.addon.update_status()
assert AutoApprovalSummary.check_should_be_delayed(self.version) is True

self.version.update(
created=datetime.now() - timedelta(seconds=target_delay - 1)
)
assert AutoApprovalSummary.check_should_be_delayed(self.version) is True
self.version.update(
created=datetime.now() - timedelta(seconds=target_delay + 1)
)
assert AutoApprovalSummary.check_should_be_delayed(self.version) is False

# Goes back to 24 hours if the value is invalid.
set_config('INITIAL_AUTO_APPROVAL_DELAY_FOR_LISTED', 'nonsense')
self.version.update(created=datetime.now() - timedelta(hours=23, seconds=1))
assert AutoApprovalSummary.check_should_be_delayed(self.version) is True
self.version.update(created=datetime.now() - timedelta(hours=24, seconds=1))
assert AutoApprovalSummary.check_should_be_delayed(self.version) is False

def test_check_should_be_delayed_only_until_first_content_review(self):
assert AutoApprovalSummary.check_should_be_delayed(self.version) is False

Expand Down Expand Up @@ -1573,6 +1602,28 @@ def test_verdict_info_prettifier(self):
result = list(AutoApprovalSummary.verdict_info_prettifier({}))
assert result == []

def test_verdict_display(self):
assert (
AutoApprovalSummary(verdict=amo.AUTO_APPROVED).get_verdict_display()
== 'Was auto-approved'
)
assert (
AutoApprovalSummary(verdict=amo.NOT_AUTO_APPROVED).get_verdict_display()
== 'Was *not* auto-approved'
)
assert (
AutoApprovalSummary(
verdict=amo.WOULD_HAVE_BEEN_AUTO_APPROVED
).get_verdict_display()
== 'Would have been auto-approved (dry-run mode was in effect)'
)
assert (
AutoApprovalSummary(
verdict=amo.WOULD_NOT_HAVE_BEEN_AUTO_APPROVED
).get_verdict_display()
== 'Would *not* have been auto-approved (dry-run mode was in effect)'
)


class TestReviewActionReason(TestCase):
def test_basic(self):
Expand Down

0 comments on commit f1015a7

Please sign in to comment.