{{ _("We have already reviewed a similar appeal from another reporter, and have reversed our prior decision. We have taken action against the content and/or account holder in accordance with our policies.") }}
{{ _("Because the decision you are appealing has already been overturned, your appeal will not be processed.") }}
{% endif %} + {% elif appealed_decision_overridden %} +{{ _("Thank you for your report.") }}
+{{ _("The decision you are appealing has already been overridden by a new decision, so this decision can't be appealed.") }}
{% else %}{{ _("This decision can't be appealed.") }}
{% endif %} diff --git a/src/olympia/abuse/tests/test_utils.py b/src/olympia/abuse/tests/test_actions.py similarity index 95% rename from src/olympia/abuse/tests/test_utils.py rename to src/olympia/abuse/tests/test_actions.py index 6b6e6e4f51d4..5813204258d3 100644 --- a/src/olympia/abuse/tests/test_utils.py +++ b/src/olympia/abuse/tests/test_actions.py @@ -1,9 +1,12 @@ +import json +import uuid from datetime import datetime from django.conf import settings from django.core import mail from django.urls import reverse +import responses from waffle.testutils import override_switch from olympia import amo @@ -22,6 +25,7 @@ ContentActionDeleteCollection, ContentActionDeleteRating, ContentActionDisableAddon, + ContentActionForwardToLegal, ContentActionIgnore, ContentActionOverrideApprove, ContentActionRejectVersion, @@ -830,6 +834,53 @@ def test_hold_action(self): 'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON, } + def test_forward_to_reviewers_no_job(self): + self.decision.update(action=DECISION_ACTIONS.AMO_LEGAL_FORWARD) + self.decision.cinder_job.update(decision=None) + action = ContentActionForwardToLegal(self.decision) + responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}create_report', + json={'job_id': '1234-xyz'}, + status=201, + ) + + action.process_action() + + assert CinderJob.objects.get(job_id='1234-xyz') + request_body = json.loads(responses.calls[0].request.body) + assert request_body['reasoning'] == self.decision.notes + assert request_body['queue_slug'] == 'legal-escalations' + + def test_forward_to_reviewers_with_job(self): + self.decision.update(action=DECISION_ACTIONS.AMO_LEGAL_FORWARD) + action = ContentActionForwardToLegal(self.decision) + responses.add_callback( + responses.POST, + f'{settings.CINDER_SERVER_URL}jobs/{self.cinder_job.job_id}/decision', + callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})), + ) + responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}create_report', + json={'job_id': '1234-xyz'}, + status=201, + ) + + action.process_action() + + new_cinder_job = CinderJob.objects.get(job_id='1234-xyz') + assert new_cinder_job != self.cinder_job + assert new_cinder_job.job_id == '1234-xyz' + # The old cinder_job should have a reference to the new job + assert self.cinder_job.reload().forwarded_to_job == new_cinder_job + # And the reports should now be part of the new job instead + assert self.abuse_report_auth.reload().cinder_job == new_cinder_job + assert self.abuse_report_no_auth.reload().cinder_job == new_cinder_job + request_body = json.loads(responses.calls[0].request.body) + assert request_body['reasoning'] == self.decision.notes + assert request_body['queue_slug'] == 'legal-escalations' + class TestContentActionCollection(BaseTestContentAction, TestCase): ActionClass = ContentActionDeleteCollection diff --git a/src/olympia/abuse/tests/test_cinder.py b/src/olympia/abuse/tests/test_cinder.py index c832345c8fb0..244e4dfa8fe1 100644 --- a/src/olympia/abuse/tests/test_cinder.py +++ b/src/olympia/abuse/tests/test_cinder.py @@ -36,6 +36,7 @@ from ..cinder import ( CinderAddon, + CinderAddonHandledByLegal, CinderAddonHandledByReviewers, CinderCollection, CinderRating, @@ -1403,7 +1404,7 @@ def test_close_job(self): cinder_instance = self.CinderClass(target) assert cinder_instance.close_job(job_id=job_id) == job_id - def _setup_workflow_move_test(self): + def _setup_post_queue_move_test(self): addon = self._create_dummy_target() listed_version = addon.current_version unlisted_version = version_factory(addon=addon, channel=amo.CHANNEL_UNLISTED) @@ -1424,7 +1425,7 @@ def _setup_workflow_move_test(self): ) return cinder_instance, cinder_job, listed_version, unlisted_version - def _check_workflow_move_test(self, listed_version, unlisted_version): + def _check_post_queue_move_test(self, listed_version, unlisted_version): assert listed_version.addon.reload().status == amo.STATUS_APPROVED assert ( listed_version.reload().needshumanreview_set.get().reason @@ -1440,19 +1441,19 @@ def _check_workflow_move_test(self, listed_version, unlisted_version): assert activity.arguments == [listed_version] assert activity.user == self.task_user - def test_workflow_move(self): + def test_post_queue_move(self): cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) - cinder_instance.workflow_move(job=cinder_job) + cinder_instance.post_queue_move(job=cinder_job) - self._check_workflow_move_test(listed_version, unlisted_version) + self._check_post_queue_move_test(listed_version, unlisted_version) - def test_workflow_move_specific_version(self): + def test_post_queue_move_specific_version(self): # but if we have a version specified, we flag that version cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) other_version = version_factory( addon=listed_version.addon, file_kw={'status': amo.STATUS_DISABLED} @@ -1461,7 +1462,7 @@ def test_workflow_move_specific_version(self): cinder_job.abusereport_set.update(addon_version=other_version.version) ActivityLog.objects.all().delete() - cinder_instance.workflow_move(job=cinder_job) + cinder_instance.post_queue_move(job=cinder_job) assert not listed_version.reload().needshumanreview_set.exists() assert not unlisted_version.reload().needshumanreview_set.exists() @@ -1477,7 +1478,7 @@ def test_workflow_move_specific_version(self): def test_workflow_recreate(self): cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) responses.add( responses.POST, @@ -1486,13 +1487,14 @@ def test_workflow_recreate(self): status=201, ) - assert cinder_instance.workflow_recreate(job=cinder_job) == '2' + assert cinder_instance.workflow_recreate(notes='foo', job=cinder_job) == '2' + assert json.loads(responses.calls[0].request.body)['reasoning'] == 'foo' - self._check_workflow_move_test(listed_version, unlisted_version) + self._check_post_queue_move_test(listed_version, unlisted_version) - def test_workflow_move_no_versions_to_flag(self): + def test_post_queue_move_no_versions_to_flag(self): cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) NeedsHumanReview.objects.create( reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, version=listed_version @@ -1503,13 +1505,13 @@ def test_workflow_move_no_versions_to_flag(self): assert NeedsHumanReview.objects.count() == 2 ActivityLog.objects.all().delete() - cinder_instance.workflow_move(job=cinder_job) + cinder_instance.post_queue_move(job=cinder_job) assert NeedsHumanReview.objects.count() == 2 assert ActivityLog.objects.count() == 0 def test_workflow_recreate_no_versions_to_flag(self): cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) NeedsHumanReview.objects.create( reason=NeedsHumanReview.REASONS.CINDER_ESCALATION, version=listed_version @@ -1525,19 +1527,19 @@ def test_workflow_recreate_no_versions_to_flag(self): json={'job_id': '2'}, status=201, ) - assert cinder_instance.workflow_recreate(job=cinder_job) == '2' + assert cinder_instance.workflow_recreate(notes=None, job=cinder_job) == '2' assert NeedsHumanReview.objects.count() == 2 assert ActivityLog.objects.count() == 0 @override_switch('dsa-cinder-forwarded-review', active=False) - def test_workflow_move_waffle_switch_off(self): + def test_post_queue_move_waffle_switch_off(self): # Escalation when the waffle switch is off is essentially a no-op on # AMO side. cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) - cinder_instance.workflow_move(job=cinder_job) + cinder_instance.post_queue_move(job=cinder_job) assert listed_version.addon.reload().status == amo.STATUS_APPROVED assert not listed_version.reload().needshumanreview_set.exists() @@ -1553,7 +1555,7 @@ def test_workflow_move_waffle_switch_off(self): assert not other_version.due_date ActivityLog.objects.all().delete() cinder_job.abusereport_set.update(addon_version=other_version.version) - cinder_instance.workflow_move(job=cinder_job) + cinder_instance.post_queue_move(job=cinder_job) assert not listed_version.reload().needshumanreview_set.exists() assert not unlisted_version.reload().needshumanreview_set.exists() other_version.reload() @@ -1564,7 +1566,7 @@ def test_workflow_move_waffle_switch_off(self): @override_switch('dsa-cinder-forwarded-review', active=False) def test_workflow_recreate_waffle_switch_off(self): cinder_instance, cinder_job, listed_version, unlisted_version = ( - self._setup_workflow_move_test() + self._setup_post_queue_move_test() ) responses.add( responses.POST, @@ -1572,7 +1574,7 @@ def test_workflow_recreate_waffle_switch_off(self): json={'job_id': '2'}, status=201, ) - assert cinder_instance.workflow_recreate(job=cinder_job) == '2' + assert cinder_instance.workflow_recreate(notes='', job=cinder_job) == '2' assert listed_version.addon.reload().status == amo.STATUS_APPROVED assert not listed_version.reload().needshumanreview_set.exists() @@ -1582,6 +1584,63 @@ def test_workflow_recreate_waffle_switch_off(self): assert ActivityLog.objects.count() == 0 +class TestCinderAddonHandledByLegal(TestCinderAddon): + CinderClass = CinderAddonHandledByLegal + # For rendering the payload to Cinder like CinderAddon: + # - 1 Fetch Addon authors + # - 2 Fetch Promoted Addon + expected_queries_for_report = 2 + expected_queue_suffix = None + + def test_queue(self): + extension = self._create_dummy_target() + assert self.CinderClass(extension).queue == 'legal-escalations' + + def test_queue_appeal(self): + extension = self._create_dummy_target() + assert self.CinderClass(extension).queue_appeal == 'legal-escalations' + + def test_queue_with_theme(self): + # Contrary to reports handled by Cinder moderators, for reports handled + # by legal the queue should remain the same regardless of the + # addon-type. + target = self._create_dummy_target(type=amo.ADDON_STATICTHEME) + assert self.CinderClass(target).queue_appeal == 'legal-escalations' + + def test_workflow_recreate(self): + """Test that a job is created in the legal queue.""" + # Specifically create signed files because there are some circumstances where we + # filter out unsigned files from NeedsHumanReview and we don't want a false + # positive. + addon = self._create_dummy_target(file_kw={'is_signed': True}) + listed_version = addon.current_version + unlisted_version = version_factory( + addon=addon, channel=amo.CHANNEL_UNLISTED, file_kw={'is_signed': True} + ) + cinder_instance = self.CinderClass(addon) + cinder_job = CinderJob.objects.create(target_addon=addon, job_id='1') + responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}create_report', + json={'job_id': '2'}, + status=201, + ) + + assert cinder_instance.workflow_recreate(notes='foo', job=cinder_job) == '2' + + # Check that we've not inadvertently changed the status + assert listed_version.addon.reload().status == amo.STATUS_APPROVED + # And check there have been no needshumanreview instances created or activity + # - only reviewer tools handled jobs should generated needshumanreviews + assert not listed_version.reload().needshumanreview_set.exists() + assert not unlisted_version.reload().needshumanreview_set.exists() + assert ( + ActivityLog.objects.filter(action=amo.LOG.NEEDS_HUMAN_REVIEW.id).count() + == 0 + ) + assert json.loads(responses.calls[0].request.body)['reasoning'] == 'foo' + + class TestCinderUser(BaseTestCinderCase, TestCase): CinderClass = CinderUser # 2 queries expected: diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 9d5ca2416c20..bd90abf9dd34 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -39,18 +39,10 @@ from olympia.versions.models import Version, VersionReviewerFlags from ..actions import ( - ContentActionAlreadyRemoved, - ContentActionApproveInitialDecision, - ContentActionApproveNoAction, ContentActionBanUser, ContentActionDeleteCollection, ContentActionDeleteRating, - ContentActionDisableAddon, - ContentActionEscalateAddon, - ContentActionIgnore, ContentActionOverrideApprove, - ContentActionRejectVersion, - ContentActionRejectVersionDelayed, ContentActionTargetAppealApprove, ContentActionTargetAppealRemovalAffirmation, ) @@ -1652,7 +1644,7 @@ def test_resolve_job_forwarded(self): ).exists() assert NeedsHumanReview.objects.filter(is_active=True).count() == 2 - def test_abuse_reports(self): + def test_all_abuse_reports(self): job = CinderJob.objects.create(job_id='fake_job_id') assert list(job.all_abuse_reports) == [] @@ -1701,6 +1693,24 @@ def test_abuse_reports(self): assert list(appeal_job.all_abuse_reports) == [report, report2, report3] assert list(job.all_abuse_reports) == [report, report2] + # Now test the scenario where the original decision was an override instead of + # the first decision. The reports should still be found by all_abuse_reports. + job.decision.update(appeal_job=None) + ContentDecision.objects.create( + action=DECISION_ACTIONS.AMO_DISABLE_ADDON, + addon=addon, + appeal_job=appeal_job, + override_of=job.decision, + ) + assert list(appeal_appeal_job.all_abuse_reports) == [ + report, + report2, + report3, + report4, + ] + assert list(appeal_job.all_abuse_reports) == [report, report2, report3] + assert list(job.all_abuse_reports) == [report, report2] + def test_is_appeal(self): job = CinderJob.objects.create(job_id='fake_job_id') assert not job.is_appeal @@ -1824,6 +1834,22 @@ def nhr_exists(reason): assert nhr_exists(NeedsHumanReview.REASONS.CINDER_ESCALATION) assert not nhr_exists(NeedsHumanReview.REASONS.ADDON_REVIEW_APPEAL) + def test_final_decision(self): + addon = addon_factory() + job = CinderJob.objects.create(job_id='1') + assert job.final_decision is None + + decision = ContentDecision.objects.create( + addon=addon, action=DECISION_ACTIONS.AMO_DISABLE_ADDON + ) + job.update(decision=decision) + assert job.final_decision == decision + + override = ContentDecision.objects.create( + addon=addon, action=DECISION_ACTIONS.AMO_DISABLE_ADDON, override_of=decision + ) + assert job.final_decision == override + class TestContentDecisionCanBeAppealed(TestCase): def setUp(self): @@ -1919,6 +1945,28 @@ def test_reporter_cant_appeal_approve_decision_already_appealed(self): is_reporter=True, abuse_report=initial_report ) + def test_reporter_cant_appeal_approve_decision_overridden(self): + initial_report = AbuseReport.objects.create( + guid=self.addon.guid, + cinder_job=CinderJob.objects.create(decision=self.decision), + reporter=self.reporter, + reason=AbuseReport.REASONS.ILLEGAL, + ) + assert self.decision.can_be_appealed( + is_reporter=True, abuse_report=initial_report + ) + override = ContentDecision.objects.create( + addon=self.addon, + action=self.decision.action, + override_of=self.decision, + action_date=datetime.now(), + ) + assert not self.decision.can_be_appealed( + is_reporter=True, abuse_report=initial_report + ) + # but can appeal the override + assert override.can_be_appealed(is_reporter=True, abuse_report=initial_report) + def test_reporter_can_appeal_approve_decision_already_appealed_someone_else(self): initial_report = AbuseReport.objects.create( guid=self.addon.guid, @@ -2068,6 +2116,19 @@ def test_author_cant_appeal_disable_decision_already_appealed(self): self.decision.update(appeal_job=appeal_job) assert not self.decision.can_be_appealed(is_reporter=False) + def test_author_cant_appeal_disable_decision_overridden(self): + self.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + assert self.decision.can_be_appealed(is_reporter=False) + override = ContentDecision.objects.create( + addon=self.addon, + action=self.decision.action, + override_of=self.decision, + action_date=datetime.now(), + ) + assert not self.decision.can_be_appealed(is_reporter=False) + # but can appeal the override + assert override.can_be_appealed(is_reporter=False) + def test_author_can_appeal_appealed_decision(self): appeal_job = CinderJob.objects.create( job_id='fake_appeal_job_id', @@ -2209,6 +2270,23 @@ def setUp(self): self.task_user = user_factory(pk=settings.TASK_USER_ID) set_user(self.task_user) + def test_originating_job(self): + decision = ContentDecision() + assert decision.originating_job is None + + job = CinderJob(job_id='123') + decision.cinder_job = job + assert decision.originating_job == job + + new_decision = ContentDecision() + assert new_decision.originating_job is None + + new_decision.override_of = decision + assert new_decision.originating_job == job + + decision.cinder_job = None + assert new_decision.originating_job is None + def test_get_reference_id(self): decision = ContentDecision() assert decision.get_reference_id() == 'NoClass#None' @@ -2291,21 +2369,10 @@ def test_get_action_helper(self): ) targets = { ContentActionBanUser: {'user': user_factory()}, - ContentActionDisableAddon: {'addon': addon}, - ContentActionRejectVersion: {'addon': addon}, - ContentActionRejectVersionDelayed: {'addon': addon}, - ContentActionEscalateAddon: {'addon': addon}, ContentActionDeleteCollection: {'collection': collection_factory()}, ContentActionDeleteRating: { 'rating': Rating.objects.create(addon=addon, user=user_factory()) }, - ContentActionApproveInitialDecision: {'addon': addon}, - ContentActionApproveNoAction: {'addon': addon}, - ContentActionOverrideApprove: {'addon': addon}, - ContentActionTargetAppealApprove: {'addon': addon}, - ContentActionTargetAppealRemovalAffirmation: {'addon': addon}, - ContentActionIgnore: {'addon': addon}, - ContentActionAlreadyRemoved: {'addon': addon}, } action_to_class = [ (decision_action, ContentDecision.get_action_helper_class(decision_action)) @@ -2349,7 +2416,7 @@ def test_get_action_helper(self): 'rating': None, 'collection': None, 'user': None, - **targets[ActionClass], + **targets.get(ActionClass, {'addon': addon}), } ) helper = decision.get_action_helper( @@ -2378,7 +2445,7 @@ def test_get_action_helper(self): 'rating': None, 'collection': None, 'user': None, - **targets[ActionClass], + **targets.get(ActionClass, {'addon': addon}), } ) helper = decision.get_action_helper( @@ -2795,6 +2862,7 @@ def _test_notify_reviewer_decision( expect_create_decision_call, expect_create_job_decision_call, extra_log_details=None, + expected_decision_object_count=1, ): create_decision_response = responses.add( responses.POST, @@ -2818,6 +2886,7 @@ def _test_notify_reviewer_decision( decision.addon, resolved_in_reviewer_tools=True ) addon_version = decision.addon.versions.all()[0] + cinder_action = cinder_action or getattr(activity_action, 'cinder_action', None) log_entry = ActivityLog.objects.create( activity_action, decision.addon, @@ -2851,7 +2920,6 @@ def _test_notify_reviewer_decision( ] self.assertCloseToNow(decision.action_date) assert list(decision.policies.all()) == policies - assert ContentDecision.objects.count() == 1 assert decision.id elif expect_create_job_decision_call: assert create_decision_response.call_count == 0 @@ -2866,13 +2934,14 @@ def _test_notify_reviewer_decision( ] self.assertCloseToNow(decision.action_date) assert list(decision.policies.all()) == policies - assert ContentDecision.objects.count() == 1 assert decision.id else: assert create_decision_response.call_count == 0 assert create_job_decision_response.call_count == 0 assert CinderPolicy.contentdecision_set.through.objects.count() == 0 assert not decision.id + assert ContentDecision.objects.count() == expected_decision_object_count + if expect_email: assert len(mail.outbox) == 1 assert mail.outbox[0].to == [decision.addon.authors.first().email] @@ -2896,7 +2965,7 @@ def _test_notify_reviewer_decision( else: assert len(mail.outbox) == 0 - def test_notify_reviewer_decision_new_decision(self): + def test_notify_reviewer_decision_first_decision(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) decision = ContentDecision(addon=addon) @@ -2910,18 +2979,22 @@ def test_notify_reviewer_decision_new_decision(self): assert parse.quote(f'/firefox/addon/{addon.slug}/') in mail.outbox[0].body assert '/developers/' not in mail.outbox[0].body - def test_notify_reviewer_decision_updated_decision(self): + def test_notify_reviewer_decision_override_decision(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) - decision = ContentDecision.objects.create( - addon=addon, action=DECISION_ACTIONS.AMO_REJECT_VERSION_WARNING_ADDON + previous_decision = ContentDecision.objects.create( + addon=addon, + action=DECISION_ACTIONS.AMO_REJECT_VERSION_WARNING_ADDON, + action_date=datetime.now(), ) + decision = ContentDecision(addon=addon, override_of=previous_decision) self._test_notify_reviewer_decision( decision, amo.LOG.REJECT_VERSION, DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON, expect_create_decision_call=True, expect_create_job_decision_call=False, + expected_decision_object_count=2, ) assert parse.quote(f'/firefox/addon/{addon.slug}/') in mail.outbox[0].body assert '/developers/' not in mail.outbox[0].body @@ -2945,7 +3018,7 @@ def test_notify_reviewer_decision_unlisted_version(self): in mail.outbox[0].body ) - def test_notify_reviewer_decision_new_decision_no_email_to_owner(self): + def test_notify_reviewer_decision_first_decision_no_email_to_owner(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) decision = ContentDecision(addon=addon) @@ -2959,13 +3032,18 @@ def test_notify_reviewer_decision_new_decision_no_email_to_owner(self): expect_create_job_decision_call=True, ) - def test_notify_reviewer_decision_updated_decision_no_email_to_owner(self): + def test_notify_reviewer_decision_override_decision_no_email_to_owner(self): addon_developer = user_factory() addon = addon_factory(users=[addon_developer]) - decision = ContentDecision.objects.create( - addon=addon, action=DECISION_ACTIONS.AMO_REJECT_VERSION_WARNING_ADDON + previous_decision = ContentDecision.objects.create( + addon=addon, + action=DECISION_ACTIONS.AMO_REJECT_VERSION_WARNING_ADDON, + action_date=datetime.now(), ) - decision.cinder_job = CinderJob.objects.create(job_id='1234') + previous_decision.cinder_job = CinderJob.objects.create( + job_id='1234', decision=previous_decision + ) + decision = ContentDecision(addon=addon, override_of=previous_decision) self._test_notify_reviewer_decision( decision, amo.LOG.CONFIRM_AUTO_APPROVED, @@ -2973,6 +3051,7 @@ def test_notify_reviewer_decision_updated_decision_no_email_to_owner(self): expect_email=False, expect_create_decision_call=True, expect_create_job_decision_call=False, + expected_decision_object_count=2, ) def test_no_create_decision_for_approve_without_a_job(self): @@ -2987,6 +3066,7 @@ def test_no_create_decision_for_approve_without_a_job(self): expect_create_decision_call=False, expect_create_job_decision_call=False, expect_email=True, + expected_decision_object_count=0, ) def test_notify_reviewer_decision_auto_approve_email_for_non_human_review(self): @@ -3000,6 +3080,7 @@ def test_notify_reviewer_decision_auto_approve_email_for_non_human_review(self): expect_email=True, expect_create_decision_call=False, expect_create_job_decision_call=False, + expected_decision_object_count=0, extra_log_details={'human_review': False}, ) assert 'automatically screened and tentatively approved' in mail.outbox[0].body @@ -3015,6 +3096,7 @@ def test_notify_reviewer_decision_auto_approve_email_for_human_review(self): expect_email=True, expect_create_decision_call=False, expect_create_job_decision_call=False, + expected_decision_object_count=0, extra_log_details={'human_review': True}, ) assert 'has been approved' in mail.outbox[0].body @@ -3129,6 +3211,35 @@ def test_notify_reviewer_decision_rejection_addon_already_disabled(self): not in mail.outbox[0].body ) + def test_notify_reviewer_decision_legal_forward(self): + """Test a reviewer "decision" to forward to legal. Because there is no job there + is no decision though, so we don't expect any decision to be notified to Cinder. + """ + addon_developer = user_factory() + # Set to disabled because we already don't create decisions for approvals. + addon = addon_factory(users=[addon_developer], status=amo.STATUS_DISABLED) + decision = ContentDecision(addon=addon) + # Check there isn't a job already so our .get later isn't a false positive. + assert not CinderJob.objects.exists() + responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}create_report', + json={'job_id': '123456'}, + status=201, + ) + self._test_notify_reviewer_decision( + decision, + amo.LOG.REQUEST_LEGAL, + None, + # as above, we arne't making a decision on a job, so no call is expected + expect_create_decision_call=False, + expect_create_job_decision_call=False, + expected_decision_object_count=0, + # and certainly no email to the developer + expect_email=False, + ) + assert CinderJob.objects.get().job_id == '123456' + def _test_process_action_ban_user_outcome(self, decision): self.assertCloseToNow(decision.action_date) self.assertCloseToNow(decision.user.reload().banned) diff --git a/src/olympia/abuse/tests/test_tasks.py b/src/olympia/abuse/tests/test_tasks.py index b62a00005bab..ac0edb59ae82 100644 --- a/src/olympia/abuse/tests/test_tasks.py +++ b/src/olympia/abuse/tests/test_tasks.py @@ -1038,3 +1038,4 @@ def test_handle_escalate_action(): assert new_job.resolvable_in_reviewer_tools assert new_job.target_addon == addon assert report.reload().cinder_job == new_job + assert json.loads(responses.calls[0].request.body)['reasoning'] == 'blah' diff --git a/src/olympia/abuse/tests/test_views.py b/src/olympia/abuse/tests/test_views.py index bde111ca14e9..c9af425db4d1 100644 --- a/src/olympia/abuse/tests/test_views.py +++ b/src/olympia/abuse/tests/test_views.py @@ -3046,6 +3046,67 @@ def test_reporter_cant_appeal_appealed_decision_already_made_for_other_turned(se 'and have reversed our prior decision' ) in doc.text() + def test_reporter_cant_appeal_overridden_decision(self): + user = user_factory() + self.abuse_report.update(reporter=user) + + self.client.force_login(user) + response = self.client.get(self.reporter_appeal_url) + assert response.status_code == 200 + doc = pq(response.content) + assert doc('#id_reason') + assert not doc('#appeal-thank-you') + assert doc('#appeal-submit') + + ContentDecision.objects.create( + cinder_id='appeal decision id', + action=DECISION_ACTIONS.AMO_APPROVE, + addon=self.addon, + override_of=self.cinder_job.decision, + ) + + response = self.client.get(self.reporter_appeal_url) + assert response.status_code == 200 + doc = pq(response.content) + assert not doc('#id_reason') + assert not doc('#appeal-thank-you') + assert not doc('#appeal-submit') + assert ( + 'The decision you are appealing has already been overridden by a new ' + 'decision' in doc.text() + ) + + def test_author_cant_appeal_overridden_decision(self): + self.cinder_job.decision.update(action=DECISION_ACTIONS.AMO_DISABLE_ADDON) + user = user_factory() + self.addon.authors.add(user) + self.client.force_login(user) + response = self.client.get(self.author_appeal_url) + + assert response.status_code == 200 + doc = pq(response.content) + assert doc('#id_reason') + assert not doc('#appeal-thank-you') + assert doc('#appeal-submit') + + ContentDecision.objects.create( + cinder_id='appeal decision id', + action=DECISION_ACTIONS.AMO_APPROVE, + addon=self.addon, + override_of=self.cinder_job.decision, + ) + + response = self.client.get(self.author_appeal_url) + assert response.status_code == 200 + doc = pq(response.content) + assert not doc('#id_reason') + assert not doc('#appeal-thank-you') + assert not doc('#appeal-submit') + assert ( + 'The decision you are appealing has already been overridden by a new ' + 'decision' in doc.text() + ) + def test_throttling_initial_email_form(self): expected_error_message = ( 'You have submitted this form too many times recently. ' diff --git a/src/olympia/abuse/views.py b/src/olympia/abuse/views.py index 98341f86e7bb..cabe0cb3dcec 100644 --- a/src/olympia/abuse/views.py +++ b/src/olympia/abuse/views.py @@ -202,7 +202,7 @@ def process_webhook_payload_decision(payload): log.debug('ContentDecision instance not found for id %s', decision_id) raise ValidationError('No matching decision id found') from exc - cinder_job = getattr(decision, 'cinder_job', None) + cinder_job = decision.originating_job if not cinder_job: log.debug('No job for ContentDecision with id %s', decision_id) raise ValidationError('No matching job found for decision id') @@ -316,7 +316,7 @@ def appeal(request, *, abuse_report_id, decision_cinder_id, **kwargs): abuse_report = get_object_or_404( AbuseReport.objects.filter(cinder_job__isnull=False), id=abuse_report_id, - cinder_job=getattr(cinder_decision, 'cinder_job', None), + cinder_job=cinder_decision.originating_job, ) else: abuse_report = None @@ -416,7 +416,12 @@ def appeal(request, *, abuse_report_id, decision_cinder_id, **kwargs): # this point should only contain the hidden email input) if the # report can't be appealed. No form should be left on the page. context_data.pop('appeal_email_form', None) - if ( + if hasattr(cinder_decision, 'overridden_by'): + # The reason we can't appeal this is that the decision has already been + # overriden by a new decision. We want a specific error message in this + # case. + context_data['appealed_decision_overridden'] = True + elif ( is_reporter and not hasattr(abuse_report, 'cinderappeal') and cinder_decision.appealed_decision_already_made() @@ -426,7 +431,8 @@ def appeal(request, *, abuse_report_id, decision_cinder_id, **kwargs): # error message in this case. context_data['appealed_decision_already_made'] = True context_data['appealed_decision_affirmed'] = ( - cinder_decision.appeal_job.decision.action == cinder_decision.action + cinder_decision.appeal_job.final_decision.action + == cinder_decision.action ) return TemplateResponse(request, 'abuse/appeal.html', context=context_data) diff --git a/src/olympia/constants/abuse.py b/src/olympia/constants/abuse.py index 8c7510de5e62..37f7b9d57edd 100644 --- a/src/olympia/constants/abuse.py +++ b/src/olympia/constants/abuse.py @@ -22,6 +22,7 @@ ('AMO_APPROVE_VERSION', 10, 'Approved (new version approval)'), ('AMO_IGNORE', 11, 'Invalid report, so ignored'), ('AMO_CLOSED_NO_ACTION', 12, 'Content already removed (no action)'), + ('AMO_LEGAL_FORWARD', 13, 'Forward add-on to legal'), ) DECISION_ACTIONS.add_subset( 'APPEALABLE_BY_AUTHOR', @@ -47,6 +48,9 @@ ), ) DECISION_ACTIONS.add_subset('APPROVING', ('AMO_APPROVE', 'AMO_APPROVE_VERSION')) +DECISION_ACTIONS.add_subset( + 'SKIP_DECISION', ('AMO_APPROVE', 'AMO_APPROVE_VERSION', 'AMO_LEGAL_FORWARD') +) # Illegal categories, only used when the reason is `illegal`. The constants # are derived from the "spec" but without the `STATEMENT_CATEGORY_` prefix. diff --git a/src/olympia/constants/activity.py b/src/olympia/constants/activity.py index e8c2894fc5e1..5abab0347aed 100644 --- a/src/olympia/constants/activity.py +++ b/src/olympia/constants/activity.py @@ -1110,6 +1110,15 @@ class BLOCKLIST_VERSION_SOFT_BLOCKED(_LOG): short = 'Version Soft Blocked' +class REQUEST_LEGAL(_LOG): + id = 198 + reviewer_review_action = True + format = _('{addon} forwarded for legal review') + short = 'Forwarded to Legal' + hide_developer = True + cinder_action = DECISION_ACTIONS.AMO_LEGAL_FORWARD + + LOGS = [x for x in vars().values() if isclass(x) and issubclass(x, _LOG) and x != _LOG] # Make sure there's no duplicate IDs. assert len(LOGS) == len({log.id for log in LOGS}) diff --git a/src/olympia/reviewers/management/commands/send_pending_rejection_last_warning_notifications.py b/src/olympia/reviewers/management/commands/send_pending_rejection_last_warning_notifications.py index 5c57e6c2dbd4..33455b264e32 100644 --- a/src/olympia/reviewers/management/commands/send_pending_rejection_last_warning_notifications.py +++ b/src/olympia/reviewers/management/commands/send_pending_rejection_last_warning_notifications.py @@ -92,7 +92,7 @@ def notify_developers(self, *, addon, versions): ) # We just need the decision to send an accurate email decision = ( - cinder_job.decision + cinder_job.final_decision if cinder_job # Fake a decision if there isn't a job else ContentDecision( diff --git a/src/olympia/reviewers/tests/test_forms.py b/src/olympia/reviewers/tests/test_forms.py index 67a6a121f5f0..39942d93e802 100644 --- a/src/olympia/reviewers/tests/test_forms.py +++ b/src/olympia/reviewers/tests/test_forms.py @@ -84,6 +84,7 @@ def test_actions_addon_status_null(self): assert list(actions.keys()) == [ 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -99,6 +100,7 @@ def test_actions_addon_status_null(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] @@ -118,6 +120,7 @@ def test_actions_addon_status_null_unlisted(self): 'confirm_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -136,6 +139,7 @@ def test_actions_addon_status_null_unlisted(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] @@ -149,6 +153,7 @@ def test_actions_addon_status_deleted(self): assert list(actions.keys()) == [ 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -162,6 +167,7 @@ def test_actions_addon_status_deleted(self): 'clear_needs_human_review_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -176,6 +182,7 @@ def test_actions_no_pending_files(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -191,6 +198,7 @@ def test_actions_no_pending_files(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] @@ -204,6 +212,7 @@ def test_actions_no_pending_files(self): 'clear_needs_human_review_multiple_versions', 'reply', 'enable_addon', + 'request_legal_review', 'comment', ] diff --git a/src/olympia/reviewers/tests/test_utils.py b/src/olympia/reviewers/tests/test_utils.py index 14dc3a831a89..0483076f6e17 100644 --- a/src/olympia/reviewers/tests/test_utils.py +++ b/src/olympia/reviewers/tests/test_utils.py @@ -244,6 +244,7 @@ def test_actions_full_nominated(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -264,6 +265,7 @@ def test_actions_full_update(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -282,6 +284,7 @@ def test_actions_full_nonpending(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] f_statuses = [amo.STATUS_APPROVED, amo.STATUS_DISABLED] @@ -301,6 +304,7 @@ def test_actions_public_post_review(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -321,6 +325,7 @@ def test_actions_public_post_review(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -354,6 +359,7 @@ def test_actions_content_review(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -376,6 +382,7 @@ def test_actions_content_review_non_approved_addon(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -416,6 +423,7 @@ def test_actions_public_static_theme(self): 'set_needs_human_review_multiple_versions', 'reply', 'request_admin_review', + 'request_legal_review', 'comment', ] assert ( @@ -457,7 +465,6 @@ def test_actions_recommended(self): == expected ) - expected = ['reply', 'comment'] assert ( list( self.get_review_actions( @@ -476,6 +483,7 @@ def test_actions_recommended(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -514,6 +522,7 @@ def test_actions_recommended_content_review(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -562,6 +571,7 @@ def test_actions_promoted_admin_review_needs_admin_permission(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -601,6 +611,7 @@ def test_actions_promoted_admin_review_needs_admin_permission(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] assert ( @@ -638,6 +649,7 @@ def test_actions_unlisted(self): 'confirm_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -675,6 +687,7 @@ def test_actions_unlisted(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] assert ( @@ -695,6 +708,7 @@ def test_actions_version_blocked(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -716,6 +730,7 @@ def test_actions_version_blocked(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -738,6 +753,7 @@ def test_actions_version_blocked(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert ( @@ -777,6 +793,7 @@ def test_actions_pending_rejection(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] self.review_version = version_factory(addon=self.addon) @@ -811,6 +828,7 @@ def test_actions_pending_rejection_admin(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] assert ( @@ -837,6 +855,7 @@ def test_actions_pending_rejection_admin(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] self.review_version = version_factory(addon=self.addon) @@ -853,7 +872,7 @@ def test_actions_pending_rejection_admin(self): def test_actions_disabled_addon(self): self.grant_permission(self.user, 'Addons:Review') - expected = ['reply', 'comment'] + expected = ['reply', 'request_legal_review', 'comment'] actions = list( self.get_review_actions( addon_status=amo.STATUS_DISABLED, @@ -872,6 +891,7 @@ def test_actions_disabled_addon(self): 'clear_needs_human_review_multiple_versions', 'reply', 'enable_addon', + 'request_legal_review', 'comment', ] actions = list( @@ -883,7 +903,12 @@ def test_actions_disabled_addon(self): def test_actions_rejected_version(self): self.grant_permission(self.user, 'Addons:Review') - expected = ['set_needs_human_review_multiple_versions', 'reply', 'comment'] + expected = [ + 'set_needs_human_review_multiple_versions', + 'reply', + 'request_legal_review', + 'comment', + ] self.file.update(status=amo.STATUS_DISABLED) self.file.version.update(human_review_date=datetime.now()) @@ -899,6 +924,7 @@ def test_actions_rejected_version(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] actions = list(self.get_helper().actions.keys()) @@ -922,7 +948,12 @@ def test_actions_non_human_reviewer(self): def test_actions_deleted_addon(self): self.grant_permission(self.user, 'Addons:Review') - expected = ['set_needs_human_review_multiple_versions', 'reply', 'comment'] + expected = [ + 'set_needs_human_review_multiple_versions', + 'reply', + 'request_legal_review', + 'comment', + ] actions = list( self.get_review_actions( addon_status=amo.STATUS_DELETED, @@ -934,7 +965,12 @@ def test_actions_deleted_addon(self): def test_actions_versions_needing_human_review(self): NeedsHumanReview.objects.create(version=self.review_version) self.grant_permission(self.user, 'Addons:Review') - expected = ['set_needs_human_review_multiple_versions', 'reply', 'comment'] + expected = [ + 'set_needs_human_review_multiple_versions', + 'reply', + 'request_legal_review', + 'comment', + ] actions = list( self.get_review_actions( addon_status=amo.STATUS_DELETED, @@ -949,6 +985,7 @@ def test_actions_versions_needing_human_review(self): 'clear_needs_human_review_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] actions = list( @@ -969,6 +1006,7 @@ def test_actions_cinder_jobs_to_resolve(self): 'set_needs_human_review_multiple_versions', 'reply', 'resolve_reports_job', + 'request_legal_review', 'comment', ] actions = list( @@ -987,6 +1025,7 @@ def test_actions_cinder_jobs_to_resolve(self): 'set_needs_human_review_multiple_versions', 'reply', 'resolve_appeal_job', + 'request_legal_review', 'comment', ] actions = list( @@ -3528,6 +3567,10 @@ def test_notify_decision_called_everywhere_checkbox_shown_listed(self): 'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON, }, 'resolve_reports_job': {'should_email': False, 'cinder_action': None}, + 'request_legal_review': { + 'should_email': False, + 'cinder_action': DECISION_ACTIONS.AMO_LEGAL_FORWARD, + }, } ) self.setup_data(amo.STATUS_APPROVED, file_status=amo.STATUS_DISABLED) @@ -3547,6 +3590,10 @@ def test_notify_decision_called_everywhere_checkbox_shown_listed(self): 'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON, }, 'resolve_reports_job': {'should_email': False, 'cinder_action': None}, + 'request_legal_review': { + 'should_email': False, + 'cinder_action': DECISION_ACTIONS.AMO_LEGAL_FORWARD, + }, } ) self.setup_data(amo.STATUS_DISABLED, file_status=amo.STATUS_DISABLED) @@ -3561,6 +3608,10 @@ def test_notify_decision_called_everywhere_checkbox_shown_listed(self): 'cinder_action': DECISION_ACTIONS.AMO_APPROVE_VERSION, }, 'resolve_reports_job': {'should_email': False, 'cinder_action': None}, + 'request_legal_review': { + 'should_email': False, + 'cinder_action': DECISION_ACTIONS.AMO_LEGAL_FORWARD, + }, } ) @@ -3598,6 +3649,10 @@ def test_notify_decision_called_everywhere_checkbox_shown_unlisted(self): 'cinder_action': DECISION_ACTIONS.AMO_DISABLE_ADDON, }, 'resolve_reports_job': {'should_email': False, 'cinder_action': None}, + 'request_legal_review': { + 'should_email': False, + 'cinder_action': DECISION_ACTIONS.AMO_LEGAL_FORWARD, + }, } ) self.setup_data(amo.STATUS_DISABLED, file_status=amo.STATUS_DISABLED) @@ -3620,6 +3675,10 @@ def test_notify_decision_called_everywhere_checkbox_shown_unlisted(self): 'cinder_action': DECISION_ACTIONS.AMO_APPROVE_VERSION, }, 'resolve_reports_job': {'should_email': False, 'cinder_action': None}, + 'request_legal_review': { + 'should_email': False, + 'cinder_action': DECISION_ACTIONS.AMO_LEGAL_FORWARD, + }, } ) @@ -3721,6 +3780,53 @@ def test_reject_multiple_versions_resets_original_status_too(self): File.STATUS_DISABLED_REASONS.NONE ) + def _test_request_legal_review(self, *, data=None): + self.setup_data( + amo.STATUS_APPROVED, + file_status=amo.STATUS_APPROVED, + ) + if data: + data = {**self.get_data(), **data} + self.helper.set_data(data) + report_request = responses.add( + responses.POST, + f'{settings.CINDER_SERVER_URL}create_report', + json={'job_id': '5678'}, + status=201, + ) + + self.helper.handler.request_legal_review() + + assert len(mail.outbox) == 0 + assert report_request.call_count == 1 + assert CinderJob.objects.get(job_id='5678') + request_body = json.loads(responses.calls[0].request.body) + assert ( + request_body['reasoning'] == data['comments'] + if data + else self.get_data()['comments'] + ) + + def test_request_legal_review_no_job(self): + self._test_request_legal_review() + + def test_request_legal_review_resolve_job(self): + # Set up a typical job that would be handled in the reviewer tools + job = CinderJob.objects.create( + target_addon=self.addon, resolvable_in_reviewer_tools=True, job_id='1234' + ) + AbuseReport.objects.create(guid=self.addon.guid, cinder_job=job) + responses.add_callback( + responses.POST, + f'{settings.CINDER_SERVER_URL}jobs/1234/decision', + callback=lambda r: (201, {}, json.dumps({'uuid': uuid.uuid4().hex})), + ) + self._test_request_legal_review(data={'cinder_jobs_to_resolve': [job]}) + + # And check that the job was resolved in the way we expected + assert job.reload().decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD + assert job.forwarded_to_job == CinderJob.objects.get(job_id='5678') + @override_settings(ENABLE_ADDON_SIGNING=True) class TestReviewHelperSigning(TestReviewHelperBase): diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index ce6f07a3b8a7..9c91a32975df 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -2422,6 +2422,7 @@ def test_need_correct_reviewer_for_promoted_addon(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert choices == expected_choices @@ -2460,6 +2461,7 @@ def test_need_correct_reviewer_for_promoted_addon(self): 'set_needs_human_review_multiple_versions', 'reply', 'disable_addon', + 'request_legal_review', 'comment', ] assert choices == expected_choices @@ -2636,6 +2638,38 @@ def test_resolve_appeal_job(self, resolve_mock): assert log2.details['cinder_action'] == 'AMO_APPROVE' assert resolve_mock.call_count == 2 + @mock.patch('olympia.reviewers.utils.resolve_job_in_cinder.delay') + def test_request_legal_review(self, resolve_mock): + appeal_job = CinderJob.objects.create( + job_id='1', resolvable_in_reviewer_tools=True, target_addon=self.addon + ) + ContentDecision.objects.create( + appeal_job=appeal_job, + action=DECISION_ACTIONS.AMO_DISABLE_ADDON, + addon=self.addon, + ) + ContentDecision.objects.create( + appeal_job=appeal_job, + action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON, + addon=self.addon, + ) + + response = self.client.post( + self.url, + { + 'action': 'request_legal_review', + 'comments': 'Nope', + 'cinder_jobs_to_resolve': [appeal_job.id], + }, + ) + assert response.status_code == 302 + + activity_log_qs = ActivityLog.objects.filter(action=amo.LOG.REQUEST_LEGAL.id) + assert activity_log_qs.count() == 1 + log = activity_log_qs.get() + assert log.details['cinder_action'] == 'AMO_LEGAL_FORWARD' + assert resolve_mock.call_count == 1 + def test_reviewer_reply(self): reason = ReviewActionReason.objects.create( name='reason 1', is_active=True, canned_response='reason' @@ -5116,6 +5150,7 @@ def test_data_value_attributes(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert [ @@ -5141,6 +5176,7 @@ def test_data_value_attributes(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -5174,6 +5210,7 @@ def test_data_value_attributes_unlisted(self): 'confirm_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert [ @@ -5198,6 +5235,7 @@ def test_data_value_attributes_unlisted(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] @@ -5249,6 +5287,7 @@ def test_data_value_attributes_unreviewed(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert [ @@ -5265,6 +5304,7 @@ def test_data_value_attributes_unreviewed(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert doc('.data-toggle.review-files')[0].attrib['data-value'].split(' ') == [ @@ -5294,6 +5334,7 @@ def test_data_value_attributes_static_theme(self): 'set_needs_human_review_multiple_versions', 'reply', 'request_admin_review', + 'request_legal_review', 'comment', ] assert [ @@ -5311,6 +5352,7 @@ def test_data_value_attributes_static_theme(self): 'set_needs_human_review_multiple_versions', 'reply', 'request_admin_review', + 'request_legal_review', 'comment', ] # we don't show files, reasons, and tested with for any static theme actions @@ -5335,6 +5377,7 @@ def test_post_review_ignore_disabled(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert [action[0] for action in response.context['actions']] == expected_actions @@ -5355,6 +5398,7 @@ def test_content_review_ignore_disabled(self): 'reject_multiple_versions', 'set_needs_human_review_multiple_versions', 'reply', + 'request_legal_review', 'comment', ] assert [action[0] for action in response.context['actions']] == expected_actions diff --git a/src/olympia/reviewers/utils.py b/src/olympia/reviewers/utils.py index a50d2a42df80..137033cbb177 100644 --- a/src/olympia/reviewers/utils.py +++ b/src/olympia/reviewers/utils.py @@ -493,6 +493,7 @@ def get_actions(self): .prefetch_related( 'abusereport_set', 'appealed_decisions__cinder_job', + 'appealed_decisions__override_of__cinder_job', 'appealed_decisions__appeals', ) ) @@ -834,7 +835,19 @@ def get_actions(self): ), 'minimal': True, 'available': is_reviewer and has_unresolved_appeal_jobs, - 'comments': True, + 'resolves_cinder_jobs': True, + } + actions['request_legal_review'] = { + 'method': self.handler.request_legal_review, + 'label': 'Request review from Mozilla Legal', + 'details': ( + 'If you have concerns about the legality of this add-on that requires ' + "Mozilla's legal to investigate, enter your comments in the area " + 'below. They will not be sent to the developer.' + 'If it relates to an open abuse report job or appeal resolve then job.' + ), + 'minimal': True, + 'available': is_appropriate_reviewer, 'resolves_cinder_jobs': True, } actions['comment'] = { @@ -1513,6 +1526,12 @@ def disable_addon(self): log.info('Sending email for %s' % (self.addon)) self.notify_decision() + def request_legal_review(self): + """Forward add-on and/or job to legal via Cinder.""" + self.log_action(amo.LOG.REQUEST_LEGAL) + log.info('Forwarding %s for legal review' % (self.addon)) + self.notify_decision() + class ReviewAddon(ReviewBase): set_addon_status = True