diff --git a/src/olympia/abuse/actions.py b/src/olympia/abuse/actions.py index c7faf65c2b75..dd00fbd75d2f 100644 --- a/src/olympia/abuse/actions.py +++ b/src/olympia/abuse/actions.py @@ -314,7 +314,7 @@ def log_action(self, activity_log_action, *extra_args, extra_details=None): activity_log__contentdecision__id=self.decision.id ).first(): attachment.update(activity_log=activity_log) - activity_log.attacmentlog = attachment # update fk + activity_log.attachmentlog = attachment # update fk return activity_log def process_action(self): diff --git a/src/olympia/abuse/models.py b/src/olympia/abuse/models.py index e2d1b79a4daa..39b455e73aab 100644 --- a/src/olympia/abuse/models.py +++ b/src/olympia/abuse/models.py @@ -1335,12 +1335,17 @@ def execute_action(self, *, release_hold=False): return log_entry def send_notifications(self): + from olympia.activity.models import AttachmentLog + if not self.action_date: return action_helper = self.get_action_helper() cinder_job = self.originating_job log_entry = self.activities.last() + has_attachment = AttachmentLog.objects.filter( + activity_log__contentdecisionlog__decision=self + ).exists() if cinder_job: cinder_job.notify_reporters(action_helper) @@ -1366,7 +1371,7 @@ def send_notifications(self): 'is_addon_disabled': details.get('is_addon_being_disabled') or getattr(self.target, 'is_disabled', False), 'version_list': ', '.join(ver_str for ver_str in version_numbers), - 'has_attachment': hasattr(log_entry, 'attachmentlog'), + 'has_attachment': has_attachment, 'dev_url': absolutify(self.target.get_dev_url('versions')) if self.addon_id else None, diff --git a/src/olympia/abuse/tests/test_models.py b/src/olympia/abuse/tests/test_models.py index 8f476bcb5bfd..afd320723241 100644 --- a/src/olympia/abuse/tests/test_models.py +++ b/src/olympia/abuse/tests/test_models.py @@ -2886,23 +2886,6 @@ def test_report_to_cinder_approve_override_previous_decision_has_cinder_id(self) expect_create_override_call=True, ) - def _check_notify_emails(self, decision, log_entry): - assert len(mail.outbox) == 1 - assert mail.outbox[0].to == [decision.addon.authors.first().email] - assert str(log_entry.id) in mail.outbox[0].extra_headers['Message-ID'] - assert 'days' not in mail.outbox[0].body - assert 'some review text' in mail.outbox[0].body - assert 'some policy text' not in mail.outbox[0].body - AttachmentLog.objects.create( - activity_log=log_entry, - file=ContentFile('Pseudo File', name='attachment.txt'), - ) - decision.send_notifications() - assert 'An attachment was provided.' not in mail.outbox[0].body - assert 'To respond or view the file,' not in mail.outbox[0].body - assert 'An attachment was provided.' in mail.outbox[1].body - assert 'To respond or view the file,' in mail.outbox[1].body - def _test_execute_action_ban_user_outcome(self, decision): self.assertCloseToNow(decision.action_date) self.assertCloseToNow(decision.user.reload().banned) @@ -2970,12 +2953,19 @@ def test_execute_action_disable_addon_held(self): alog = ActivityLog.objects.filter( action=amo.LOG.HELD_ACTION_FORCE_DISABLE.id ).get() + # attachment is linked to the original decision activity log + AttachmentLog.objects.create( + activity_log=alog, + file=ContentFile('Pseudo File', name='attachment.txt'), + ) assert alog.contentdecisionlog_set.get().decision == decision decision.send_notifications() assert len(mail.outbox) == 0 decision.execute_action(release_hold=True) self._test_execute_action_disable_addon_outcome(decision) + assert 'An attachment was provided.' in mail.outbox[0].body + assert 'To respond or view the file,' in mail.outbox[0].body def test_execute_action_disable_addon(self): addon = addon_factory(users=[user_factory()]) @@ -2988,6 +2978,8 @@ def test_execute_action_disable_addon(self): decision.execute_action() self._test_execute_action_disable_addon_outcome(decision) assert '14 day(s)' not in mail.outbox[0].body + assert 'An attachment was provided.' not in mail.outbox[0].body + assert 'To respond or view the file,' not in mail.outbox[0].body def _test_execute_action_reject_version_outcome(self, decision): decision.send_notifications() @@ -3024,12 +3016,19 @@ def test_execute_action_reject_version_held(self): action=amo.LOG.HELD_ACTION_REJECT_VERSIONS.id ).get() assert alog.contentdecisionlog_set.get().decision == decision + # attachment is linked to the original decision activity log + AttachmentLog.objects.create( + activity_log=alog, + file=ContentFile('Pseudo File', name='attachment.txt'), + ) decision.send_notifications() assert len(mail.outbox) == 0 assert not version.needshumanreview_set.filter(is_active=True).exists() decision.execute_action(release_hold=True) self._test_execute_action_reject_version_outcome(decision) + assert 'An attachment was provided.' in mail.outbox[0].body + assert 'To respond or view the file,' in mail.outbox[0].body def test_execute_action_reject_version(self): addon = addon_factory(users=[user_factory()]) @@ -3061,6 +3060,8 @@ def test_execute_action_reject_version(self): self._test_execute_action_reject_version_outcome(decision) assert '14 day(s)' not in mail.outbox[0].body assert not version.needshumanreview_set.filter(is_active=True).exists() + assert 'An attachment was provided.' not in mail.outbox[0].body + assert 'To respond or view the file,' not in mail.outbox[0].body def _test_execute_action_reject_version_delayed_outcome(self, decision): decision.send_notifications() @@ -3405,7 +3406,21 @@ def test_execute_action_with_action_date_already(self): process_mock.assert_not_called() hold_mock.assert_not_called() decision.send_notifications() - self._check_notify_emails(decision, log_entry) + assert len(mail.outbox) == 1 + assert mail.outbox[0].to == [decision.addon.authors.first().email] + assert str(log_entry.id) in mail.outbox[0].extra_headers['Message-ID'] + assert 'days' not in mail.outbox[0].body + assert 'some review text' in mail.outbox[0].body + assert 'some policy text' not in mail.outbox[0].body + AttachmentLog.objects.create( + activity_log=log_entry, + file=ContentFile('Pseudo File', name='attachment.txt'), + ) + decision.send_notifications() + assert 'An attachment was provided.' not in mail.outbox[0].body + assert 'To respond or view the file,' not in mail.outbox[0].body + assert 'An attachment was provided.' in mail.outbox[1].body + assert 'To respond or view the file,' in mail.outbox[1].body def test_get_target_review_url(self): addon = addon_factory()