diff --git a/src/olympia/abuse/actions.py b/src/olympia/abuse/actions.py index 6a430346e85a..398f55eb4283 100644 --- a/src/olympia/abuse/actions.py +++ b/src/olympia/abuse/actions.py @@ -882,7 +882,7 @@ def process_action(self, release_hold=False): and target.status == amo.STATUS_REJECTED ): AddonApprovalsCounter.objects.update_or_create( - addon=self.target, + addon=target, defaults={ 'last_content_review': datetime.now(), 'last_content_review_pass': True, @@ -919,10 +919,10 @@ def process_action(self, release_hold=False): amo.LOG.UNDELETE_RATING, self.target.addon, extra_details={ - 'body': str(self.target.body), - 'addon_id': self.target.addon.pk, - 'addon_title': str(self.target.addon.name), - 'is_flagged': self.target.ratingflag_set.exists(), + 'body': str(target.body), + 'addon_id': target.addon.pk, + 'addon_title': str(target.addon.name), + 'is_flagged': target.ratingflag_set.exists(), }, ) @@ -941,11 +941,40 @@ def previous_decisions(self): return self.decision.__class__.objects.filter(pk=self.decision.override_of_id) -class ContentActionApproveNoAction(AnyTargetMixin, NoActionMixin, ContentAction): - description = 'Reported content is within policy, initial decision, so no action' +class ContentActionApproveListingContent( + AnyTargetMixin, AnyOwnerEmailMixin, ContentAction +): + description = 'Reported content is within policy' reporter_template_path = 'abuse/emails/reporter_content_approve.txt' reporter_appeal_template_path = 'abuse/emails/reporter_appeal_approve.txt' + def __init__(self, decision): + super().__init__(decision) + self.status = self.target.status if isinstance(self.target, Addon) else None + + def get_owners(self): + if self.status == amo.STATUS_REJECTED: + # If we're approving listing content that was rejected, we need to + # notify the authors. + return self.target.authors.all() + return () + + def process_action(self, release_hold=False): + if isinstance(self.target, Addon): + AddonApprovalsCounter.objects.update_or_create( + addon=self.target, + defaults={ + 'last_content_review': datetime.now(), + 'last_content_review_pass': True, + }, + ) + if self.status == amo.STATUS_REJECTED: + # Call the function to correct it the status + self.target.update_status() + log_entry = self.log_action(amo.LOG.APPROVE_LISTING_CONTENT) + return log_entry + return None + class ContentActionApproveInitialDecision( AnyTargetMixin, NoActionMixin, AnyOwnerEmailMixin, ContentAction @@ -991,7 +1020,7 @@ class ContentActionNotImplemented(AnyTargetMixin, NoActionMixin, ContentAction): DECISION_ACTIONS.AMO_BLOCK_ADDON: ContentActionBlockAddon, DECISION_ACTIONS.AMO_DELETE_COLLECTION: ContentActionDeleteCollection, DECISION_ACTIONS.AMO_DELETE_RATING: ContentActionDeleteRating, - DECISION_ACTIONS.AMO_APPROVE: ContentActionApproveNoAction, + DECISION_ACTIONS.AMO_APPROVE: ContentActionApproveListingContent, DECISION_ACTIONS.AMO_APPROVE_VERSION: ContentActionApproveInitialDecision, DECISION_ACTIONS.AMO_IGNORE: ContentActionIgnore, DECISION_ACTIONS.AMO_CLOSED_NO_ACTION: ContentActionAlreadyModerated, diff --git a/src/olympia/abuse/templates/abuse/emails/ContentActionApproveListingContent.txt b/src/olympia/abuse/templates/abuse/emails/ContentActionApproveListingContent.txt new file mode 100644 index 000000000000..12b16803b264 --- /dev/null +++ b/src/olympia/abuse/templates/abuse/emails/ContentActionApproveListingContent.txt @@ -0,0 +1,19 @@ +Hello, + +Previously, your {{ type }} was suspended/removed from Mozilla Add-ons, based on a finding that you had violated Mozilla's policies. + +We have now determined that your content is within policy, and based on that determination, we have restored your {{ type }}. It is now available at {{ target_url }}. +{% if manual_reasoning_text %}{{ manual_reasoning_text }}. {% endif %} + +{% if has_attachment %} +An attachment was provided. {% if dev_url %}To respond or view the file, visit {{ dev_url }}.{% endif %} + +{% endif %} +Thank you. + +More information about Mozilla's add-on policies can be found at {{ policy_document_url }}. + +[{{ reference_id }}] +-- +Mozilla Add-ons Team +{{ SITE_URL }} diff --git a/src/olympia/abuse/tests/test_actions.py b/src/olympia/abuse/tests/test_actions.py index 0d54bac75e84..c4fa0e1edf10 100644 --- a/src/olympia/abuse/tests/test_actions.py +++ b/src/olympia/abuse/tests/test_actions.py @@ -40,7 +40,7 @@ from ..actions import ( ContentAction, ContentActionApproveInitialDecision, - ContentActionApproveNoAction, + ContentActionApproveListingContent, ContentActionBanUser, ContentActionBlockAddon, ContentActionDeleteCollection, @@ -279,16 +279,18 @@ def test_approve_override_success(self): self._test_approve_appeal_or_override(ContentActionOverrideApprove) assert 'After reviewing your appeal' not in mail.outbox[0].body - def _test_reporter_no_action_taken( - self, - *, - ActionClass=ContentActionApproveNoAction, - action=DECISION_ACTIONS.AMO_APPROVE, - ): + def _test_reporter_no_action_taken(self, *, ActionClass, action): raise NotImplementedError + def _test_reporter_content_approved_action_taken(self): + # For most ActionClasses, there is no action taken. + return self._test_reporter_no_action_taken( + ActionClass=ContentActionApproveListingContent, + action=DECISION_ACTIONS.AMO_APPROVE, + ) + def test_reporter_content_approve_report(self): - subject = self._test_reporter_no_action_taken() + subject = self._test_reporter_content_approved_action_taken() assert len(mail.outbox) == 2 self._test_reporter_content_approve_email(subject) @@ -310,14 +312,15 @@ def test_reporter_appeal_approve(self): decision=original_job.decision, reporter_report=self.abuse_report_auth ) self.cinder_job.reload() - subject = self._test_reporter_no_action_taken() + subject = self._test_reporter_content_approved_action_taken() assert len(mail.outbox) == 1 # only abuse_report_auth reporter self._test_reporter_appeal_approve_email(subject) def test_owner_content_approve_report_email(self): # This isn't called by cinder actions, but is triggered by reviewer actions subject = self._test_reporter_no_action_taken( - ActionClass=ContentActionApproveInitialDecision + ActionClass=ContentActionApproveInitialDecision, + action=DECISION_ACTIONS.AMO_APPROVE, ) assert len(mail.outbox) == 3 self._test_reporter_content_approve_email(subject) @@ -365,7 +368,7 @@ def test_email_content_not_escaped(self): action.notify_owners() assert unsafe_str in mail.outbox[0].body - action = ContentActionApproveNoAction(self.decision) + action = ContentActionApproveListingContent(self.decision) mail.outbox.clear() action.notify_reporters( reporter_abuse_reports=[self.abuse_report_auth], is_appeal=True @@ -508,12 +511,7 @@ def test_ban_user_after_reporter_appeal(self): assert len(mail.outbox) == 2 self._test_reporter_appeal_takedown_email(subject) - def _test_reporter_no_action_taken( - self, - *, - ActionClass=ContentActionApproveNoAction, - action=DECISION_ACTIONS.AMO_APPROVE, - ): + def _test_reporter_no_action_taken(self, *, ActionClass, action): self.decision.update(action=action) action = ActionClass(self.decision) assert action.process_action() is None @@ -755,12 +753,7 @@ def _test_approve_appeal_or_override(self, ContentActionClass): action.notify_owners() self._test_owner_restore_email(f'Mozilla Add-ons: {self.addon.name}') - def _test_reporter_no_action_taken( - self, - *, - ActionClass=ContentActionApproveNoAction, - action=DECISION_ACTIONS.AMO_APPROVE, - ): + def _test_reporter_no_action_taken(self, *, ActionClass, action): self.decision.update(action=action) action = ActionClass(self.decision) assert action.process_action() is None @@ -772,6 +765,70 @@ def _test_reporter_no_action_taken( action.notify_owners() return f'Mozilla Add-ons: {self.addon.name}' + def _test_reporter_content_approved_action_taken(self): + # override because Addon's get content reviewed if marked as Approve + action = DECISION_ACTIONS.AMO_APPROVE + self.decision.update(action=action) + action = ContentActionApproveListingContent(self.decision) + activity = action.process_action() + assert activity.log == amo.LOG.APPROVE_LISTING_CONTENT + assert activity.arguments == [self.addon, self.decision, self.policy] + assert activity.user == self.task_user + + assert self.addon.reload().status == amo.STATUS_APPROVED + assert ActivityLog.objects.count() == 2 + second_activity = ActivityLog.objects.exclude(pk=activity.pk).get() + assert second_activity.log == amo.LOG.REVIEWER_PRIVATE_COMMENT + assert second_activity.arguments == [self.addon, self.decision] + assert second_activity.user == self.task_user + assert second_activity.details == {'comments': self.decision.private_notes} + + counter = AddonApprovalsCounter.objects.get(addon=self.addon) + assert counter.last_content_review_pass is True + self.assertCloseToNow(counter.last_content_review) + + assert len(mail.outbox) == 0 + self.cinder_job.notify_reporters(action) + action.notify_owners() + return f'Mozilla Add-ons: {self.addon.name}' + + def test_content_approve_rejected_listing_content(self): + AddonApprovalsCounter.objects.create( + addon=self.addon, last_content_review_pass=False + ) + self.addon.update(status=amo.STATUS_REJECTED) + action = DECISION_ACTIONS.AMO_APPROVE + self.decision.update(action=action) + action = ContentActionApproveListingContent(self.decision) + activity = action.process_action() + assert activity.log == amo.LOG.APPROVE_LISTING_CONTENT + assert activity.arguments == [self.addon, self.decision, self.policy] + assert activity.user == self.task_user + + assert self.addon.reload().status == amo.STATUS_APPROVED + assert ActivityLog.objects.count() == 3 + second_activity, third_activity = ActivityLog.objects.exclude( + pk=activity.pk + ).filter() + assert second_activity.log == amo.LOG.REVIEWER_PRIVATE_COMMENT + assert second_activity.arguments == [self.addon, self.decision] + assert second_activity.user == self.task_user + assert second_activity.details == {'comments': self.decision.private_notes} + assert third_activity.log == amo.LOG.CHANGE_STATUS + + counter = AddonApprovalsCounter.objects.get(addon=self.addon) + assert counter.last_content_review_pass is True + self.assertCloseToNow(counter.last_content_review) + + assert len(mail.outbox) == 0 + self.cinder_job.notify_reporters(action) + action.notify_owners() + subject = f'Mozilla Add-ons: {self.addon.name}' + + assert len(mail.outbox) == 3 + self._test_reporter_content_approve_email(subject) + assert 'within policy, and based on that determination' in mail.outbox[-1].body + def test_target_appeal_decline(self): self.addon.update(status=amo.STATUS_DISABLED) ActivityLog.objects.all().delete() @@ -2159,12 +2216,7 @@ def test_delete_collection_after_reporter_appeal(self): assert len(mail.outbox) == 2 self._test_reporter_appeal_takedown_email(subject) - def _test_reporter_no_action_taken( - self, - *, - ActionClass=ContentActionApproveNoAction, - action=DECISION_ACTIONS.AMO_APPROVE, - ): + def _test_reporter_no_action_taken(self, *, ActionClass, action): self.decision.update(action=action) action = ActionClass(self.decision) assert action.process_action() is None @@ -2338,12 +2390,7 @@ def test_delete_rating_after_reporter_appeal(self): assert len(mail.outbox) == 2 self._test_reporter_appeal_takedown_email(subject) - def _test_reporter_no_action_taken( - self, - *, - ActionClass=ContentActionApproveNoAction, - action=DECISION_ACTIONS.AMO_APPROVE, - ): + def _test_reporter_no_action_taken(self, *, ActionClass, action): self.decision.update(action=action) action = ActionClass(self.decision) assert action.process_action() is None diff --git a/src/olympia/abuse/tests/test_views.py b/src/olympia/abuse/tests/test_views.py index 442bf1c4d113..3575e887e6f5 100644 --- a/src/olympia/abuse/tests/test_views.py +++ b/src/olympia/abuse/tests/test_views.py @@ -33,7 +33,7 @@ from olympia.ratings.models import Rating from ..actions import ( - ContentActionApproveNoAction, + ContentActionApproveListingContent, ContentActionDisableAddon, ContentActionTargetAppealApprove, ContentActionTargetAppealRemovalAffirmation, @@ -1508,7 +1508,7 @@ def test_process_decision_triggers_no_target_email_for_reporter_approve(self): ) req = self.get_request(data=data) with mock.patch.object( - ContentActionApproveNoAction, 'process_action' + ContentActionApproveListingContent, 'process_action' ) as process_mock: cinder_webhook(req) process_mock.assert_called()