From f1015a715643b0061807f77a9cb60bf12f6fc8ea Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Wed, 2 Oct 2024 15:07:42 +0200 Subject: [PATCH] Make initial auto approval delay for listed configurable by admins (#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 --- src/olympia/constants/reviewers.py | 4 +- src/olympia/reviewers/models.py | 9 +++- src/olympia/reviewers/tests/test_models.py | 53 +++++++++++++++++++++- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/olympia/constants/reviewers.py b/src/olympia/constants/reviewers.py index b553f26cb8ed..6dc01b6c8da6 100644 --- a/src/olympia/constants/reviewers.py +++ b/src/olympia/constants/reviewers.py @@ -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'), diff --git a/src/olympia/reviewers/models.py b/src/olympia/reviewers/models.py index 6ba121f5dfc3..ad3e3c2d11b1 100644 --- a/src/olympia/reviewers/models.py +++ b/src/olympia/reviewers/models.py @@ -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') @@ -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 ) diff --git a/src/olympia/reviewers/tests/test_models.py b/src/olympia/reviewers/tests/test_models.py index c480daf48421..802563abb492 100644 --- a/src/olympia/reviewers/tests/test_models.py +++ b/src/olympia/reviewers/tests/test_models.py @@ -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): @@ -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. @@ -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 @@ -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):