diff --git a/atst/domain/task_orders.py b/atst/domain/task_orders.py index 3f997d4e..499bccb0 100644 --- a/atst/domain/task_orders.py +++ b/atst/domain/task_orders.py @@ -91,9 +91,3 @@ class TaskOrders(BaseDomainClass): ) .all() ) - - @classmethod - def update_pdf_last_sent_at(cls, task_order): - task_order.pdf_last_sent_at = datetime.now() - db.session.add(task_order) - db.session.commit() diff --git a/atst/jobs.py b/atst/jobs.py index 69bc84be..1ffe675f 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -1,5 +1,7 @@ import pendulum from flask import current_app as app +from smtplib import SMTPException +from azure.core.exceptions import AzureError from atst.database import db from atst.domain.application_roles import ApplicationRoles @@ -45,8 +47,8 @@ class RecordFailure(celery.Task): @celery.task(ignore_result=True) -def send_mail(recipients, subject, body): - app.mailer.send(recipients, subject, body) +def send_mail(recipients, subject, body, attachments=[]): + app.mailer.send(recipients, subject, body, attachments) @celery.task(ignore_result=True) @@ -146,14 +148,6 @@ def do_provision_portfolio(csp: CloudProviderInterface, portfolio_id=None): fsm.trigger_next_transition() -def do_send_with_attachment( - csp: CloudProviderInterface, recipients, subject, body, attachments -): - app.mailer.send( - recipients=recipients, subject=subject, body=body, attachments=attachments - ) - - @celery.task(bind=True, base=RecordFailure) def provision_portfolio(self, portfolio_id=None): do_work(do_provision_portfolio, self, app.csp.cloud, portfolio_id=portfolio_id) @@ -176,32 +170,6 @@ def create_environment(self, environment_id=None): do_work(do_create_environment, self, app.csp.cloud, environment_id=environment_id) -@celery.task(bind=True) -def send_with_attachment(self, recipients, subject, body, attachments): - do_work( - do_send_with_attachment, - self, - app.csp.cloud, - recipients=recipients, - subject=subject, - body=body, - attachments=attachments, - ) - - -@celery.task(bind=True) -def send_with_attachment(self, recipients, subject, body, attachments): - do_work( - do_send_with_attachment, - self, - app.csp.cloud, - recipients=recipients, - subject=subject, - body=body, - attachments=attachments, - ) - - @celery.task(bind=True) def dispatch_provision_portfolio(self): """ @@ -250,11 +218,17 @@ def dispatch_send_task_order_files(self): ) body = translate("email.task_order_sent.body", {"to_number": task_order.number}) - file = app.csp.files.download_task_order(task_order.pdf.object_name) - file["maintype"] = "application" - file["subtype"] = "pdf" + try: + file = app.csp.files.download_task_order(task_order.pdf.object_name) + file["maintype"] = "application" + file["subtype"] = "pdf" + send_mail( + recipients=recipients, subject=subject, body=body, attachments=[file] + ) + except (AzureError, SMTPException): + continue - send_with_attachment.delay( - recipients=recipients, subject=subject, body=body, attachments=[file] - ) - TaskOrders.update_pdf_last_sent_at(task_order) + task_order.pdf_last_sent_at = pendulum.now() + db.session.add(task_order) + + db.session.commit() diff --git a/atst/utils/mailer.py b/atst/utils/mailer.py index a5dbfc0b..761e95da 100644 --- a/atst/utils/mailer.py +++ b/atst/utils/mailer.py @@ -72,7 +72,7 @@ class Mailer(object): msg = EmailMessage() msg.set_content(body) msg["From"] = self.sender - msg["To"] = ", ".join(recipients) + msg["To"] = ", ".join(recipients) if type(recipients) == list else recipients msg["Subject"] = subject return msg diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 1e878ca5..a0a92982 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -2,6 +2,8 @@ import pendulum import pytest from uuid import uuid4 from unittest.mock import Mock +from smtplib import SMTPException +from azure.core.exceptions import AzureError from atst.domain.csp.cloud import MockCloudProvider from atst.domain.portfolios import Portfolios @@ -292,11 +294,9 @@ def test_provision_portfolio_create_tenant( # mock.delay.assert_called_once_with(portfolio_id=portfolio.id) -def test_dispatch_send_task_order_files( - csp, session, celery_app, celery_worker, monkeypatch, app -): +def test_dispatch_send_task_order_files(monkeypatch, app): mock = Mock() - monkeypatch.setattr("atst.jobs.send_with_attachment", mock) + monkeypatch.setattr("atst.jobs.send_mail", mock) def _download_task_order(MockFileService, object_name): return {"name": object_name} @@ -313,7 +313,7 @@ def test_dispatch_send_task_order_files( dispatch_send_task_order_files.run() # Check that send_with_attachment was called once for each task order - assert mock.delay.call_count == 3 + assert mock.call_count == 3 mock.reset_mock() # Create new TO @@ -323,7 +323,7 @@ def test_dispatch_send_task_order_files( dispatch_send_task_order_files.run() # Check that send_with_attachment was called with correct kwargs - mock.delay.assert_called_once_with( + mock.assert_called_once_with( recipients=app.config.get("MICROSOFT_TASK_ORDER_EMAIL_ADDRESS"), subject=translate( "email.task_order_sent.subject", {"to_number": task_order.number} @@ -339,3 +339,35 @@ def test_dispatch_send_task_order_files( ) assert task_order.pdf_last_sent_at + + +def test_dispatch_send_task_order_files_send_failure(monkeypatch): + def _raise_smtp_exception(**kwargs): + raise SMTPException + + monkeypatch.setattr("atst.jobs.send_mail", _raise_smtp_exception) + + task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) + dispatch_send_task_order_files.run() + + # Check that pdf_last_sent_at has not been updated + assert not task_order.pdf_last_sent_at + + +def test_dispatch_send_task_order_files_download_failure(monkeypatch): + mock = Mock() + monkeypatch.setattr("atst.jobs.send_mail", mock) + + def _download_task_order(MockFileService, object_name): + raise AzureError("something went wrong") + + monkeypatch.setattr( + "atst.domain.csp.files.MockFileService.download_task_order", + _download_task_order, + ) + + task_order = TaskOrderFactory.create(create_clins=[{"number": "0002"}]) + dispatch_send_task_order_files.run() + + # Check that pdf_last_sent_at has not been updated + assert not task_order.pdf_last_sent_at