Implement approvals in ContentActions

src/olympia/reviewers/forms.py

@@ -116,13 +116,15 @@ class VersionsChoiceWidget(forms.SelectMultiple):
     actions_filters = {
         amo.CHANNEL_UNLISTED: {
             amo.STATUS_APPROVED: [
+                'review_with_policy_approve',
                 'review_with_policy',
                 'block_multiple_versions',
                 'confirm_multiple_versions',
                 'reject_multiple_versions',
                 'reply',
             ],
             amo.STATUS_AWAITING_REVIEW: [
+                'review_with_policy_approve',
                 'review_with_policy',
                 'approve_multiple_versions',
                 'reject_multiple_versions',
@@ -141,6 +143,7 @@ class VersionsChoiceWidget(forms.SelectMultiple):
                 'reply',
             ],
             amo.STATUS_AWAITING_REVIEW: [
+                'review_with_policy_approve',
                 'review_with_policy',
                 'approve_multiple_versions',
                 'reject_multiple_versions',
@@ -183,6 +186,10 @@ def create_option(self, *args, **kwargs):
             # allow confirmation of its auto-approval.
             if obj.deleted and obj.was_auto_approved:
                 actions.append('confirm_multiple_versions')
+            # If the version is the current version and was auto-approved, make it
+            # possible to confirm the auto-approval via policy-selection
+            if obj == obj.addon.current_version and obj.was_auto_approved:
+                actions.append('review_with_policy_approve')
 
             option['attrs']['class'] = 'data-toggle'
             option['attrs']['data-value'] = ' '.join(actions)
@@ -304,6 +311,7 @@ def create_option(
             hide_for_these_actions = ['appeal_override', 'resolve_reports_job']
         elif is_developer_appeal:
             hide_for_these_actions = [
+                'review_with_policy_approve',
                 'review_with_policy',
                 'reject',
                 'reject_multiple_versions',

src/olympia/reviewers/management/commands/auto_approve.py

@@ -4,13 +4,17 @@
 from django.conf import settings
 from django.core.management.base import BaseCommand
 from django.db import transaction
+from django.utils.functional import cached_property
 
 import waffle
 from django_statsd.clients import statsd
 
 import olympia.core.logger
 from olympia import amo
+from olympia.abuse.models import CinderPolicy, ContentDecision
+from olympia.abuse.tasks import report_decision_to_cinder_and_notify
 from olympia.amo.decorators import use_primary_db
+from olympia.constants.abuse import DECISION_ACTIONS
 from olympia.constants.reviewers import WAIT_ON_SCANNERS_TIMEOUT
 from olympia.files.utils import lock
 from olympia.lib.crypto.signing import SigningError
@@ -39,6 +43,16 @@ class ApprovalNotAvailableError(Exception):
 class Command(BaseCommand):
     help = 'Auto-approve add-on versions based on predefined criteria'
 
+    # The comment is not translated on purpose, to behave like
+    # regular human approval does.
+    LISTED_COMMENT = (
+        'This version has been screened and approved for the '
+        'public. Keep in mind that other reviewers may look into '
+        'this version in the future and determine that it '
+        'requires changes or should be taken down.'
+    )
+    UNLISTED_COMMENT = 'automatic validation'
+
     def add_arguments(self, parser):
         """Handle command arguments."""
         parser.add_argument(
@@ -189,6 +203,14 @@ def process(self, version_id):
 
     @statsd.timer('reviewers.auto_approve.approve')
     def approve(self, version):
+        """Approve a version, either by calling ReviewHelper or by creating a
+        ContentDecision and calling its execute_action() method."""
+        if waffle.switch_is_active('enable-policy-review-selection'):
+            self.approve_with_action_class(version)
+        else:
+            self.approve_with_reviewer_helper(version)
+
+    def approve_with_reviewer_helper(self, version):
         """Do the approval itself, caling ReviewHelper to change the status,
         sign the files, send the e-mail, etc."""
         helper = ReviewHelper(
@@ -201,20 +223,38 @@ def approve(self, version):
         if not approve_action:
             raise ApprovalNotAvailableError
         if version.channel == amo.CHANNEL_LISTED:
-            helper.handler.data = {
-                # The comment is not translated on purpose, to behave like
-                # regular human approval does.
-                'comments': 'This version has been screened and approved for the '
-                'public. Keep in mind that other reviewers may look into '
-                'this version in the future and determine that it '
-                'requires changes or should be taken down.'
-                '\r\n\r\nThank you!'
-            }
+            helper.handler.data = {'comments': self.LISTED_COMMENT}
         else:
             helper.handler.data = {'comments': 'automatic validation'}
         approve_action['method']()
         statsd.incr('reviewers.auto_approve.approve.success')
 
+    @cached_property
+    def approve_policy(self):
+        return CinderPolicy.objects.filter(
+            enforcement_actions=DECISION_ACTIONS.AMO_APPROVE_VERSION
+        ).first()
+
+    def approve_with_action_class(self, version):
+        """Do the approval itself, caling the report_decision_to_cinder_and_notify,
+        which changes the status, signs the files, sends the e-mail, etc."""
+        decision = ContentDecision.objects.create(
+            addon=version.addon,
+            action=DECISION_ACTIONS.AMO_APPROVE_VERSION,
+            reasoning=(
+                self.LISTED_COMMENT
+                if version.channel == amo.CHANNEL_LISTED
+                else self.UNLISTED_COMMENT
+            ),
+            reviewer_user_id=settings.TASK_USER_ID,
+        )
+        if self.approve_policy:
+            decision.policies.set([self.approve_policy])
+        decision.target_versions.set([version])
+        decision.execute_action()
+        report_decision_to_cinder_and_notify.delay(decision_id=decision.id)
+        statsd.incr('reviewers.auto_approve.approve.success')
+
     def disapprove(self, version):
         """Handle a version we are not auto-approving, setting NeedsHumanReview
         if necessary to "transfer" it to the reviewers queue.

src/olympia/reviewers/tests/test_commands.py

@@ -9,6 +9,7 @@
 from django.test.testcases import TransactionTestCase
 
 import responses
+from waffle.testutils import override_switch
 
 from olympia import amo
 from olympia.abuse.models import AbuseReport, CinderJob, CinderPolicy, ContentDecision
@@ -299,7 +300,7 @@ def test_fetch_candidates(self):
 
     @mock.patch('olympia.reviewers.management.commands.auto_approve.statsd.incr')
     @mock.patch('olympia.reviewers.management.commands.auto_approve.ReviewHelper')
-    def test_approve(self, review_helper_mock, statsd_incr_mock):
+    def test_approve_with_review_helper(self, review_helper_mock, statsd_incr_mock):
         review_helper_mock.return_value.actions = {'public': mock.MagicMock()}
         command = auto_approve.Command()
         command.approve(self.version)
@@ -320,6 +321,37 @@ def test_approve(self, review_helper_mock, statsd_incr_mock):
             {},
         )
 
+    @override_switch('enable-policy-review-selection', active=True)
+    @mock.patch('olympia.reviewers.management.commands.auto_approve.statsd.incr')
+    @mock.patch(
+        'olympia.reviewers.management.commands.auto_approve.report_decision_to_cinder_and_notify.delay'
+    )
+    @mock.patch(
+        'olympia.reviewers.management.commands.auto_approve.ContentDecision.execute_action'
+    )
+    def test_approve_with_action_class(
+        self, execute_action_mock, report_decision_mock, statsd_incr_mock
+    ):
+        report_decision_mock.return_value = mock.MagicMock()
+        command = auto_approve.Command()
+        command.approve(self.version)
+        decision = ContentDecision.objects.get()
+        assert report_decision_mock.call_count == 1
+        assert report_decision_mock.call_args == (
+            (),
+            {'decision_id': decision.id},
+        )
+        assert decision.target == self.addon
+        assert decision.target_versions.get() == self.version
+        assert decision.action == DECISION_ACTIONS.AMO_APPROVE_VERSION
+        assert decision.reasoning == auto_approve.Command.LISTED_COMMENT
+        assert decision.reviewer_user_id == settings.TASK_USER_ID
+        assert statsd_incr_mock.call_count == 1
+        assert statsd_incr_mock.call_args == (
+            ('reviewers.auto_approve.approve.success',),
+            {},
+        )
+
     @mock.patch('olympia.reviewers.utils.sign_file')
     def test_full_run(self, sign_file_mock):
         # Simple integration test with as few mocks as possible.

src/olympia/reviewers/tests/test_forms.py

@@ -604,7 +604,7 @@ def test_versions_required_when_enforcement_is_on_versions(self):
         action = form.cleaned_data['action']
         assert action == 'review_with_policy'
         assert form.cleaned_data['cinder_policies'] == [reject_policy]
-        assert not form.helper.get_actions()[action]['multiple_versions']
+        assert form.helper.get_actions()[action]['multiple_versions']
         assert form.errors == {'versions': ['This field is required.']}
 
     def test_cinder_jobs_filtered_for_resolve_reports_job_and_appeal_deny(self):
@@ -884,13 +884,15 @@ def test_versions_queryset_contains_pending_files_for_listed(
             'reject_multiple_versions',
             'reply',
             'set_needs_human_review_multiple_versions',
+            'review_with_policy_approve',
         ]
         assert option1.attrib.get('value') == str(self.version.pk)
 
         option2 = doc('option[value="%s"]' % pending_version.pk)[0]
         assert option2.attrib.get('class') == 'data-toggle'
         assert option2.attrib.get('data-value').split(' ') == [
             # That version is pending.
+            'review_with_policy_approve',
             'review_with_policy',
             'approve_multiple_versions',
             'reject_multiple_versions',
@@ -1017,6 +1019,7 @@ def test_versions_queryset_contains_pending_files_for_unlisted(
         assert option1.attrib.get('class') == 'data-toggle'
         assert option1.attrib.get('data-value').split(' ') == [
             # That version is approved.
+            'review_with_policy_approve',
             'review_with_policy',
             'block_multiple_versions',
             'confirm_multiple_versions',
@@ -1030,6 +1033,7 @@ def test_versions_queryset_contains_pending_files_for_unlisted(
         assert option2.attrib.get('class') == 'data-toggle'
         assert option2.attrib.get('data-value').split(' ') == [
             # That version is pending.
+            'review_with_policy_approve',
             'review_with_policy',
             'approve_multiple_versions',
             'reject_multiple_versions',
@@ -1521,6 +1525,7 @@ def test_cinder_jobs_to_resolve_choices(self):
         assert label_1.attr['class'] == 'data-toggle-hide'
         assert label_1.attr['data-value'] == ' '.join(
             (
+                'review_with_policy_approve',
                 'review_with_policy',
                 'reject',
                 'reject_multiple_versions',

src/olympia/reviewers/tests/test_utils.py

@@ -1109,7 +1109,12 @@ def test_actions_enforcement_actions(self):
                 addon_status=amo.STATUS_NOMINATED,
                 file_status=amo.STATUS_AWAITING_REVIEW,
             )
+            assert actions['review_with_policy_approve']['enforcement_actions'] == (
+                DECISION_ACTIONS.AMO_APPROVE,
+                DECISION_ACTIONS.AMO_APPROVE_VERSION,
+            )
             assert actions['review_with_policy']['enforcement_actions'] == (
+                DECISION_ACTIONS.AMO_REJECT_LISTING_CONTENT,
                 DECISION_ACTIONS.AMO_DISABLE_ADDON,
                 DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
                 DECISION_ACTIONS.AMO_REJECT_VERSION_WARNING_ADDON,
@@ -1217,8 +1222,8 @@ def test_actions_with_enable_policy_review_selection(self):
         self.grant_permission(self.user, 'Addons:Review')
         self.grant_permission(self.user, 'Reviews:Admin')
         expected = [
+            'review_with_policy_approve',
             'review_with_policy',
-            'public',
             'change_or_clear_pending_rejection_multiple_versions',
             'clear_needs_human_review_multiple_versions',
             'set_needs_human_review_multiple_versions',
@@ -4136,14 +4141,63 @@ def test_review_with_policy_with_disable_addon(self):
         self.check_subject(message)
         assert 'disabled' in message.body
 
+    @override_switch('enable-policy-review-selection', active=True)
+    def test_review_with_policy_with_version_approval(self):
+        self.grant_permission(self.user, 'Addons:Review')
+        self.setup_data(amo.STATUS_NOMINATED, file_status=amo.STATUS_AWAITING_REVIEW)
+        assert not ContentDecision.objects.exists()
+        data = {
+            'action': 'review_with_policy',
+            'cinder_policies': [
+                policy := CinderPolicy.objects.create(
+                    uuid='z',
+                    enforcement_actions=[
+                        DECISION_ACTIONS.AMO_APPROVE_VERSION.api_value
+                    ],
+                ),
+            ],
+            'versions': [self.review_version],
+            'most_important_policy_actions': filter_enforcement_actions(
+                policy.split_enforcement_actions, Addon
+            ),
+        }
+        self.helper.set_data(data)
+        self.helper.handler.review_action = self.helper.actions['review_with_policy']
+        with patch('olympia.abuse.actions.sign_file') as sign_file_mock:
+            self.helper.handler.review_with_policy()
+            sign_file_mock.assert_called_once()
+
+        self.addon.reload()
+        assert self.addon.status == amo.STATUS_APPROVED
+        assert self.review_version.file.reload().status == amo.STATUS_APPROVED
+        assert ActivityLog.objects.count() == 2
+        activity_log = ActivityLog.objects.first()
+        assert activity_log.action == amo.LOG.APPROVE_VERSION.id
+        assert activity_log.arguments == [
+            self.addon,
+            ContentDecision.objects.get(),
+            policy,
+            self.review_version,
+        ]
+        activity_log = ActivityLog.objects.last()
+        assert activity_log.action == amo.LOG.CHANGE_STATUS.id
+        assert activity_log.arguments[0] == self.addon
+
+        assert len(mail.outbox) == 1
+        message = mail.outbox[0]
+        self.check_subject(message)
+        assert 'approved' in message.body
+
     def test_enable_addon(self):
         self.grant_permission(self.user, 'Reviews:Admin')
-        self.setup_data(amo.STATUS_APPROVED, file_status=amo.STATUS_APPROVED)
+        self.setup_data(amo.STATUS_NULL, file_status=amo.STATUS_APPROVED)
         other_version = version_factory(addon=self.addon)
         version_factory(addon=self.addon, file_kw={'status': amo.STATUS_DISABLED})
         Addon.disable_all_files(
             [self.addon], File.STATUS_DISABLED_REASONS.ADDON_DISABLE
         )
+        self.addon.update(status=amo.STATUS_NULL)
+        ActivityLog.objects.all().delete()
 
         self.helper.handler.enable_addon()