Send out emails to developers on approval (#21840)

* Send out emails to developers on approval

* correct `make format`s reformatting

* Update src/olympia/abuse/templates/abuse/emails/CinderActionApproveInitialDecision.txt

Co-authored-by: Andreas Wagner <mail@andreaswagner.org>

* use hide_developer on the log constant instead

---------

Co-authored-by: Andreas Wagner <mail@andreaswagner.org>

src/olympia/abuse/models.py

@@ -345,8 +345,14 @@ def process_decision(
         ).without_parents_if_their_children_are_present()
         self.policies.add(*policies)
         action_helper = self.get_action_helper(existing_decision, override=override)
-        if action_helper.process_action():
-            action_helper.process_notifications()
+        action_occured = action_helper.process_action()
+        if (
+            existing_decision != decision_action
+            or decision_action == self.DECISION_ACTIONS.AMO_APPROVE
+        ):
+            action_helper.notify_reporters()
+        if action_occured:
+            action_helper.notify_owners()
 
     def appeal(self, *, abuse_report, appeal_text, user, is_reporter):
         appealer_entity = None
@@ -419,9 +425,9 @@ def resolve_job(self, *, decision, log_entry):
         action_helper.affected_versions = [
             version_log.version for version_log in log_entry.versionlog_set.all()
         ]
-        action_helper.process_notifications(
-            policy_text=log_entry.details.get('comments')
-        )
+        action_helper.notify_reporters()
+        if not getattr(log_entry.log, 'hide_developer', False):
+            action_helper.notify_owners(policy_text=log_entry.details.get('comments'))
         if (report := self.initial_abuse_report) and report.is_handled_by_reviewers:
             entity_helper.close_job(job_id=self.job_id)
 

src/olympia/abuse/templates/abuse/emails/CinderActionApproveInitialDecision.txt

@@ -0,0 +1,14 @@
+Hello,
+
+Your {{ type }} has been approved on Mozilla Add-ons and it is now available at {{ target_url }}.
+
+Approved versions: {{ version_list }}
+
+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 }}

src/olympia/abuse/tests/test_models.py

@@ -650,7 +650,7 @@ def test_process_decision(self):
         with mock.patch.object(
             CinderActionBanUser, 'process_action'
         ) as action_mock, mock.patch.object(
-            CinderActionBanUser, 'process_notifications'
+            CinderActionBanUser, 'notify_owners'
         ) as notify_mock:
             action_mock.return_value = True
             cinder_job.process_decision(
@@ -681,7 +681,7 @@ def test_process_decision_with_duplicate_parent(self):
         with mock.patch.object(
             CinderActionBanUser, 'process_action'
         ) as action_mock, mock.patch.object(
-            CinderActionBanUser, 'process_notifications'
+            CinderActionBanUser, 'notify_owners'
         ) as notify_mock:
             action_mock.return_value = True
             cinder_job.process_decision(
@@ -800,7 +800,7 @@ def test_appeal_improperly_configured_author(self):
                 is_reporter=False,
             )
 
-    def test_resolve_job(self):
+    def _test_resolve_job(self, activity_action, *, expect_target_email):
         cinder_job = CinderJob.objects.create(job_id='999')
         addon_developer = user_factory()
         abuse_report = AbuseReport.objects.create(
@@ -828,7 +828,7 @@ def test_resolve_job(self):
         )
 
         log_entry = ActivityLog.objects.create(
-            amo.LOG.REJECT_VERSION,
+            activity_action,
             abuse_report.target,
             abuse_report.target.current_version,
             review_action_reason,
@@ -852,12 +852,23 @@ def test_resolve_job(self):
         )
         self.assertCloseToNow(cinder_job.decision_date)
         assert list(cinder_job.policies.all()) == policies
-        assert len(mail.outbox) == 2
+        assert len(mail.outbox) == (2 if expect_target_email else 1)
         assert mail.outbox[0].to == [abuse_report.reporter.email]
-        assert mail.outbox[1].to == [addon_developer.email]
-        assert str(log_entry.id) in mail.outbox[1].extra_headers['Message-ID']
-        assert 'some review text' in mail.outbox[1].body
-        assert str(abuse_report.target.current_version.version) in mail.outbox[1].body
+        if expect_target_email:
+            assert mail.outbox[1].to == [addon_developer.email]
+            assert str(log_entry.id) in mail.outbox[1].extra_headers['Message-ID']
+            assert 'some review text' in mail.outbox[1].body
+            assert (
+                str(abuse_report.target.current_version.version) in mail.outbox[1].body
+            )
+
+    def test_resolve_job_notify_owner(self):
+        self._test_resolve_job(amo.LOG.REJECT_VERSION, expect_target_email=True)
+
+    def test_resolve_job_no_email_to_owner(self):
+        # note: this is a false scenario - AMO_REJECT_VERSION_ADDON would never happen
+        # with a CONFIRM_AUTO_APPROVED log entry, we're just testing hide_developer
+        self._test_resolve_job(amo.LOG.CONFIRM_AUTO_APPROVED, expect_target_email=False)
 
     def test_resolve_job_duplicate_policy(self):
         cinder_job = CinderJob.objects.create(job_id='999')

src/olympia/abuse/tests/test_utils.py

@@ -197,6 +197,15 @@ def test_reporter_ignore_appeal(self):
         assert len(mail.outbox) == 1  # only abuse_report_auth reporter
         self._test_reporter_ignore_appeal_email(subject)
 
+    def test_owner_ignore_report_email(self):
+        # This isn't called by cinder actions, because
+        # CinderActionApproveInitialDecision.process_action returns None,
+        # but could be triggered by reviewer actions
+        subject = self._test_reporter_ignore_initial_or_appeal(send_owner_email=True)
+        assert len(mail.outbox) == 3
+        self._test_reporter_ignore_email(subject)
+        assert 'has been approved' in mail.outbox[-1].body
+
 
 class TestCinderActionUser(BaseTestCinderAction, TestCase):
     ActionClass = CinderActionBanUser
@@ -219,7 +228,8 @@ def _test_ban_user(self):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         subject = f'Mozilla Add-ons: {self.user.name}'
         self._test_owner_takedown_email(subject, 'has been suspended')
         return subject
@@ -240,16 +250,18 @@ def test_ban_user_after_reporter_appeal(self):
         assert len(mail.outbox) == 2
         self._test_reporter_appeal_takedown_email(subject)
 
-    def _test_reporter_ignore_initial_or_appeal(self):
+    def _test_reporter_ignore_initial_or_appeal(self, *, send_owner_email=None):
         self.cinder_job.update(decision_action=CinderJob.DECISION_ACTIONS.AMO_APPROVE)
         action = CinderActionApproveInitialDecision(self.cinder_job)
-        assert action.process_action()
+        assert action.process_action() is None
 
         self.user.reload()
         assert not self.user.banned
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        if send_owner_email:
+            action.notify_owners()
         return f'Mozilla Add-ons: {self.user.name}'
 
     def _test_approve_appeal_or_override(self, CinderActionClass):
@@ -266,7 +278,8 @@ def _test_approve_appeal_or_override(self, CinderActionClass):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_restore_email(f'Mozilla Add-ons: {self.user.name}')
 
     def test_target_appeal_decline(self):
@@ -279,7 +292,8 @@ def test_target_appeal_decline(self):
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_affirmation_email(f'Mozilla Add-ons: {self.user.name}')
 
 
@@ -307,7 +321,8 @@ def _test_disable_addon(self):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         subject = f'Mozilla Add-ons: {self.addon.name}'
         self._test_owner_takedown_email(subject, 'permanently disabled')
         assert f'Your Extension {self.addon.name}' in mail.outbox[-1].body
@@ -342,18 +357,21 @@ def _test_approve_appeal_or_override(self, CinderActionClass):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_restore_email(f'Mozilla Add-ons: {self.addon.name}')
 
-    def _test_reporter_ignore_initial_or_appeal(self):
+    def _test_reporter_ignore_initial_or_appeal(self, *, send_owner_email=None):
         self.cinder_job.update(decision_action=CinderJob.DECISION_ACTIONS.AMO_APPROVE)
         action = CinderActionApproveInitialDecision(self.cinder_job)
-        assert action.process_action()
+        assert action.process_action() is None
 
         assert self.addon.reload().status == amo.STATUS_APPROVED
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
-        action.process_notifications()
+        action.notify_reporters()
+        if send_owner_email:
+            action.notify_owners()
         return f'Mozilla Add-ons: {self.addon.name}'
 
     def test_escalate_addon(self):
@@ -364,7 +382,7 @@ def test_escalate_addon(self):
         )
         ActivityLog.objects.all().delete()
         action = CinderActionEscalateAddon(self.cinder_job)
-        assert action.process_action()
+        assert action.process_action() is None
 
         assert self.addon.reload().status == amo.STATUS_APPROVED
         assert (
@@ -395,7 +413,7 @@ def test_escalate_addon(self):
         assert not other_version.due_date
         ActivityLog.objects.all().delete()
         self.cinder_job.abusereport_set.update(addon_version=other_version.version)
-        assert action.process_action()
+        assert action.process_action() is None
         assert not listed_version.reload().needshumanreview_set.exists()
         assert not unlisted_version.reload().needshumanreview_set.exists()
         other_version.reload()
@@ -410,7 +428,7 @@ def test_escalate_addon(self):
         )
         assert activity.arguments == [other_version]
         assert activity.user == self.task_user
-        action.process_notifications()
+        action.notify_reporters()
         assert len(mail.outbox) == 0
 
     def test_target_appeal_decline(self):
@@ -424,15 +442,16 @@ def test_target_appeal_decline(self):
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_affirmation_email(f'Mozilla Add-ons: {self.addon.name}')
 
     def test_notify_owners_with_manual_policy_block(self):
         self.cinder_job.update(
             decision_action=CinderJob.DECISION_ACTIONS.AMO_DISABLE_ADDON
         )
         self.ActionClass(self.cinder_job).notify_owners(
-            self.addon.authors.all(), policy_text='some other policy justification'
+            policy_text='some other policy justification'
         )
         mail_item = mail.outbox[0]
         self._check_owner_email(
@@ -455,8 +474,8 @@ def _test_reject_version(self):
         self.cinder_job.update(
             decision_action=CinderJob.DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON
         )
-        cinder_action = CinderActionRejectVersion(self.cinder_job)
-        cinder_action.affected_versions = [
+        action = CinderActionRejectVersion(self.cinder_job)
+        action.affected_versions = [
             version_factory(addon=self.addon, version='2.3'),
             version_factory(addon=self.addon, version='3.45'),
         ]
@@ -466,7 +485,8 @@ def _test_reject_version(self):
         subject = f'Mozilla Add-ons: {self.addon.name}'
 
         assert len(mail.outbox) == 0
-        cinder_action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         mail_item = mail.outbox[-1]
         self._check_owner_email(mail_item, subject, 'have been disabled')
 
@@ -526,7 +546,8 @@ def _test_delete_collection(self):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         subject = f'Mozilla Add-ons: {self.collection.name}'
         self._test_owner_takedown_email(subject, 'permanently removed')
         return subject
@@ -547,17 +568,19 @@ def test_delete_collection_after_reporter_appeal(self):
         assert len(mail.outbox) == 2
         self._test_reporter_appeal_takedown_email(subject)
 
-    def _test_reporter_ignore_initial_or_appeal(self):
+    def _test_reporter_ignore_initial_or_appeal(self, *, send_owner_email=None):
         self.cinder_job.update(decision_action=CinderJob.DECISION_ACTIONS.AMO_APPROVE)
         action = CinderActionApproveInitialDecision(self.cinder_job)
-        assert action.process_action()
+        assert action.process_action() is None
 
         assert self.collection.reload()
         assert not self.collection.deleted
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        if send_owner_email:
+            action.notify_owners()
         return f'Mozilla Add-ons: {self.collection.name}'
 
     def _test_approve_appeal_or_override(self, CinderActionClass):
@@ -573,7 +596,8 @@ def _test_approve_appeal_or_override(self, CinderActionClass):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_restore_email(f'Mozilla Add-ons: {self.collection.name}')
 
     def test_target_appeal_decline(self):
@@ -586,7 +610,8 @@ def test_target_appeal_decline(self):
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_affirmation_email(f'Mozilla Add-ons: {self.collection.name}')
 
 
@@ -616,7 +641,8 @@ def _test_delete_rating(self):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         subject = f'Mozilla Add-ons: "Saying ..." for {self.rating.addon.name}'
         self._test_owner_takedown_email(subject, 'permanently removed')
         return subject
@@ -637,16 +663,18 @@ def test_delete_rating_after_reporter_appeal(self):
         assert len(mail.outbox) == 2
         self._test_reporter_appeal_takedown_email(subject)
 
-    def _test_reporter_ignore_initial_or_appeal(self):
+    def _test_reporter_ignore_initial_or_appeal(self, *, send_owner_email=None):
         self.cinder_job.update(decision_action=CinderJob.DECISION_ACTIONS.AMO_APPROVE)
         action = CinderActionApproveInitialDecision(self.cinder_job)
-        assert action.process_action()
+        assert action.process_action() is None
 
         assert not self.rating.reload().deleted
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        if send_owner_email:
+            action.notify_owners()
         return f'Mozilla Add-ons: "Saying ..." for {self.rating.addon.name}'
 
     def _test_approve_appeal_or_override(self, CinderActionClass):
@@ -662,7 +690,8 @@ def _test_approve_appeal_or_override(self, CinderActionClass):
         assert activity.user == self.task_user
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_restore_email(
             f'Mozilla Add-ons: "Saying ..." for {self.rating.addon.name}'
         )
@@ -678,7 +707,8 @@ def test_target_appeal_decline(self):
         assert ActivityLog.objects.count() == 0
         assert len(mail.outbox) == 0
 
-        action.process_notifications()
+        action.notify_reporters()
+        action.notify_owners()
         self._test_owner_affirmation_email(
             f'Mozilla Add-ons: "Saying ..." for {self.rating.addon.name}'
         )