diff --git a/mail/celery_tasks.py b/mail/celery_tasks.py index d964acfd..a87794b2 100644 --- a/mail/celery_tasks.py +++ b/mail/celery_tasks.py @@ -120,6 +120,7 @@ def on_failure(self, exc, task_id, args, kwargs, einfo): max_retries=MAX_ATTEMPTS, retry_backoff=RETRY_BACKOFF, base=SendEmailBaseTask, + serializer="pickle", ) def send_email_task(message): """ @@ -146,7 +147,7 @@ def send_email_task(message): logger.exception("Another SMTP connection is active, will be retried after backing off") raise SMTPConnectionBusy() - logger.info("Lock acquired, proceeding to send email") + logger.info("Lock acquired, proceeding to send email from %s to %s", message["From"], message["To"]) try: smtp_send(message) @@ -154,11 +155,10 @@ def send_email_task(message): logger.exception("An unexpected error occurred when sending email -> %s") raise - logger.info("Email sent successfully") + logger.info("Email sent successfully to %s", message["To"]) # Notify Users of Rejected Mail -@shared_task def notify_users_of_rejected_licences(mail_id, mail_response_subject): """If a reply is received with rejected licences this task notifies users of the rejection""" @@ -176,10 +176,7 @@ def notify_users_of_rejected_licences(mail_id, mail_response_subject): ) message = build_licence_rejected_email_message(message_dto) - send_email_task.apply_async( - args=(message,), - serializer="pickle", - ) + send_email_task.apply_async(args=(message,)) logger.info("Successfully notified users of rejected licences found in mail with subject %s", mail_response_subject) @@ -281,7 +278,6 @@ def send_licence_details_to_hmrc(): licence_payload_ids = [str(licence.id) for licence in licences] send_email_task.apply_async( args=(message,), - serializer="pickle", link=finalise_sending_lite_licence_details.si(mail.id, mail_dto, licence_payload_ids), ) diff --git a/mail/libraries/routing_controller.py b/mail/libraries/routing_controller.py index 303fdf20..5d4b1adf 100644 --- a/mail/libraries/routing_controller.py +++ b/mail/libraries/routing_controller.py @@ -7,7 +7,7 @@ from rest_framework.exceptions import ValidationError from mail.auth import BasicAuthentication, ModernAuthentication -from mail.enums import ExtractTypeEnum, MailReadStatuses, ReceptionStatusEnum, SourceEnum +from mail.enums import ExtractTypeEnum, MailReadStatuses, ReceptionStatusEnum, ReplyStatusEnum, SourceEnum from mail.libraries.builders import build_email_message from mail.libraries.data_processors import ( lock_db_for_sending_transaction, @@ -138,6 +138,8 @@ def check_and_route_emails(): ) _collect_and_send(mail) + check_and_notify_rejected_licences(mail) + publish_queue_status() @@ -191,7 +193,6 @@ def _collect_and_send(mail: Mail): # Schedule a task to send email send_email_task.apply_async( args=(message,), - serializer="pickle", link=finalise_sending_spire_licence_details.si(mail.id, message_to_send_dto), ) @@ -216,3 +217,13 @@ def get_email_message_dtos(server: MailServer, number: Optional[int] = 3) -> Lis # emails = read_last_three_emails(pop3_connection) server.quit_pop3_connection() return emails + + +def check_and_notify_rejected_licences(mail): + from mail.celery_tasks import notify_users_of_rejected_licences + + if not settings.SEND_REJECTED_EMAIL: + return + + if mail.response_data and ReplyStatusEnum.REJECTED in mail.response_data: + notify_users_of_rejected_licences(str(mail.id), mail.response_subject) diff --git a/mail/models.py b/mail/models.py index 3c0f081c..f0b0b54e 100644 --- a/mail/models.py +++ b/mail/models.py @@ -15,7 +15,6 @@ LicenceActionEnum, MailReadStatuses, ReceptionStatusEnum, - ReplyStatusEnum, SourceEnum, ) @@ -76,10 +75,6 @@ def save(self, *args, **kwargs): super(Mail, self).save(*args, **kwargs) - if settings.SEND_REJECTED_EMAIL: - if self.response_data and ReplyStatusEnum.REJECTED in self.response_data: - self.notify_users(self.id, self.response_subject) - def set_locking_time(self, offset: int = 0): self.currently_processing_at = timezone.now() + timedelta(seconds=offset) self.save() @@ -92,12 +87,6 @@ def set_response_date_time(self, offset: int = 0): self.response_date = timezone.now() + timedelta(seconds=offset) self.save() - @staticmethod - def notify_users(id, response_subject): - from mail.celery_tasks import notify_users_of_rejected_licences - - notify_users_of_rejected_licences.delay(str(id), response_subject) - class LicenceData(models.Model): licence_ids = models.TextField() diff --git a/mail/tests/factories.py b/mail/tests/factories.py index 22706b14..9adb3667 100644 --- a/mail/tests/factories.py +++ b/mail/tests/factories.py @@ -2,8 +2,8 @@ from uuid import uuid4 -from mail.enums import LicenceActionEnum -from mail.models import LicencePayload, Mail +from mail.enums import LicenceActionEnum, SourceEnum +from mail.models import LicenceData, LicencePayload, Mail class MailFactory(factory.django.DjangoModelFactory): @@ -16,6 +16,15 @@ class Meta: model = Mail +class LicenceDataFactory(factory.django.DjangoModelFactory): + source = SourceEnum.SPIRE + mail = factory.SubFactory(Mail) + licence_ids = "" + + class Meta: + model = LicenceData + + class LicencePayloadFactory(factory.django.DjangoModelFactory): lite_id = uuid4() reference = factory.Faker("word") diff --git a/mail/tests/test_celery_task.py b/mail/tests/test_celery_task.py index 9ba65974..d5590f63 100644 --- a/mail/tests/test_celery_task.py +++ b/mail/tests/test_celery_task.py @@ -3,13 +3,17 @@ from datetime import datetime, timezone from parameterized import parameterized +from email.mime.multipart import MIMEMultipart from unittest import mock from django.test import TestCase, override_settings +from unittest.mock import MagicMock from mail.celery_tasks import manage_inbox, notify_users_of_rejected_licences from mail.enums import ExtractTypeEnum, ReceptionStatusEnum, SourceEnum +from mail.libraries.email_message_dto import EmailMessageDto from mail.libraries.routing_controller import check_and_route_emails from mail.models import LicenceData, Mail +from mail.tests.factories import LicenceDataFactory, MailFactory from mail.tests.libraries.client import LiteHMRCTestClient @@ -28,7 +32,7 @@ def test_send_rejected_notification_email_success(self, lock_acquired, mock_cach """Test sending of licence rejected emails without and with retry scenario""" mock_cache.add.side_effect = lock_acquired - notify_users_of_rejected_licences.delay("123", "CHIEF_SPIRE_licenceReply_202401180900_42557") + notify_users_of_rejected_licences("123", "CHIEF_SPIRE_licenceReply_202401180900_42557") assert mock_cache.add.call_count == len(lock_acquired) mock_smtp_send.assert_called_once() @@ -114,3 +118,112 @@ def test_sending_of_new_message_from_spire_success( assert mock_cache.add.call_count == len(lock_acquired) mock_smtp_send.assert_called_once() + + @parameterized.expand( + [ + # SEND_REJECTED_EMAIL state, mail sender and recipients + ( + True, + [ + { + "sender": "lite-hmrc@gov.uk", + "recipients": "spire@example.com", + "subject": "ILBDOTI_live_CHIEF_licenceReply_78120_202403060300", + }, + { + "sender": "lite-hmrc@gov.uk", + "recipients": "ecju@gov.uk", + "subject": "Licence rejected by HMRC", + }, + ], + ), + ( + False, + [ + { + "sender": "lite-hmrc@gov.uk", + "recipients": "spire@example.com", + "subject": "ILBDOTI_live_CHIEF_licenceReply_78120_202403060300", + } + ], + ), + ] + ) + @override_settings( + EMAIL_USER="lite-hmrc@gov.uk", + NOTIFY_USERS=["ecju@gov.uk"], + SPIRE_ADDRESS="spire@example.com", + ) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") + @mock.patch("mail.celery_tasks.smtp_send") + @mock.patch("mail.celery_tasks.cache") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") + def test_processing_of_licence_reply_with_rejected_licences( + self, + send_rejected_email_flag, + emails_data, + email_dtos, + mock_cache, + mock_smtp_send, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): + """ + Test processing of licence reply from HMRC with rejected licences. + If SEND_REJECTED_EMAIL=True then we send email notifications to users if any licences are rejected. + """ + obj = MagicMock() + mock_get_hmrc_to_dit_mailserver.return_value = obj + mock_get_spire_to_dit_mailserver.return_value = obj + mock_cache.add.return_value = True + + run_number = 78120 + mail = MailFactory( + extract_type=ExtractTypeEnum.LICENCE_DATA, + edi_filename=self.licence_data_file_name, + edi_data=self.licence_data_file_body.decode("utf-8"), + status=ReceptionStatusEnum.REPLY_PENDING, + ) + LicenceDataFactory(mail=mail, source_run_number=run_number, hmrc_run_number=run_number) + + licence_reply_filename = f"ILBDOTI_live_CHIEF_licenceReply_{run_number}_202403060300" + file_lines = "\n".join( + [ + f"1\\fileHeader\\CHIEF\SPIRE\\licenceReply\\202403061600\\{run_number}", + "2\\accepted\\340631", + "3\\rejected\\340632", + "4\\end\\rejected\\3", + "5\\fileTrailer\\1\\1\\0", + ] + ) + + email_message_dto = EmailMessageDto( + run_number=f"{run_number}", + sender="test@example.com", + receiver="receiver@example.com", + date="Mon, 17 May 2021 14:20:18 +0100", + body="licence rejected", + subject=licence_reply_filename, + attachment=[licence_reply_filename, file_lines.encode("utf-8")], + raw_data="qwerty", + ) + email_dtos.return_value = [ + (email_message_dto, lambda x: x), + ] + + with override_settings(SEND_REJECTED_EMAIL=send_rejected_email_flag): + check_and_route_emails() + + mail.refresh_from_db() + self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_SENT) + + assert mock_cache.add.call_count == len(emails_data) + mock_smtp_send.call_count == len(emails_data) + + for index, item in enumerate(mock_smtp_send.call_args_list): + message = item.args[0] + self.assertTrue(isinstance(message, MIMEMultipart)) + self.assertEqual(message["From"], emails_data[index]["sender"]) + self.assertEqual(message["To"], emails_data[index]["recipients"]) + self.assertEqual(message["Subject"], emails_data[index]["subject"]) diff --git a/mail/tests/test_models.py b/mail/tests/test_models.py index b5325866..9abc05dc 100644 --- a/mail/tests/test_models.py +++ b/mail/tests/test_models.py @@ -1,54 +1,6 @@ import uuid -from unittest.mock import create_autospec -import pytest -from django.test import override_settings -from django.utils import timezone - -from mail.enums import ExtractTypeEnum, ReceptionStatusEnum, ReplyStatusEnum -from mail.models import LicencePayload, Mail - - -class TestSendNotifyEmail: - @pytest.fixture(autouse=True) - def _setup(self, db, monkeypatch): - self.mock_notify_users = create_autospec(spec=Mail.notify_users) - - # # Create a mail object that is waiting for a licence reply from HMRC - self.mail = Mail.objects.create( - status=ReceptionStatusEnum.REPLY_PENDING, - extract_type=ExtractTypeEnum.LICENCE_DATA, - edi_filename="the_licence_data_file", - edi_data="lovely data", - sent_filename="the_licence_data_file", - sent_data="lovely data", - ) - - monkeypatch.setattr(self.mail, "notify_users", self.mock_notify_users) - - @override_settings(SEND_REJECTED_EMAIL=True) - def test_notify_users_called_when_setting_enabled(self): - self.mail.response_data = f"test {ReplyStatusEnum.REJECTED}" - self.mail.response_date = timezone.now() - self.mail.save() - - self.mock_notify_users.assert_called_with(self.mail.id, self.mail.response_filename) - - @override_settings(SEND_REJECTED_EMAIL=True) - def test_notify_users_not_called_when_setting_enabled_but_response_accepted(self): - self.mail.response_data = f"test {ReplyStatusEnum.ACCEPTED}" - self.mail.response_date = timezone.now() - self.mail.save() - - self.mock_notify_users.assert_not_called() - - @override_settings(SEND_REJECTED_EMAIL=False) - def test_notify_users_not_called_when_setting_disabled(self): - self.mail.response_data = f"test {ReplyStatusEnum.REJECTED}" - self.mail.response_date = timezone.now() - self.mail.save() - - self.mock_notify_users.assert_not_called() +from mail.models import LicencePayload def test_licence_payload_model__str__():