From 411f0477e9f8339d1a2e83fb6ce56a97558d6b8f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 10:35:43 -0500 Subject: [PATCH 1/3] Schedule celery task for dispatch_send_task_order_files --- atst/queue.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atst/queue.py b/atst/queue.py index 10bcb350..392fc8e2 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -27,6 +27,10 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_create_environment_role", "schedule": 60, }, + "beat-dispatch_send_task_order_files": { + "task": "atst.jobs.dispatch_send_task_order_files", + "schedule": 60, + }, } class ContextTask(celery.Task): From b8bf106a96fa000223407d9ee659883d04bc6b12 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 10:56:35 -0500 Subject: [PATCH 2/3] Move dispatch_send_task_order_files tests into a test class --- tests/test_jobs.py | 127 ++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 66 deletions(-) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 25a0b4c8..dd285ca4 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -415,82 +415,77 @@ def test_create_environment_role(): assert env_role.cloud_id == "a-cloud-id" -# TODO: Refactor the tests related to dispatch_send_task_order_files() into a class -# and separate the success test into two tests -def test_dispatch_send_task_order_files(monkeypatch, app): - mock = Mock() - monkeypatch.setattr("atst.jobs.send_mail", mock) +class TestDispatchSendTaskOrderFiles: + @pytest.fixture(scope="function") + def send_mail(self, monkeypatch): + mock = Mock() + monkeypatch.setattr("atst.jobs.send_mail", mock) + return mock - def _download_task_order(MockFileService, object_name): - return {"name": object_name} + @pytest.fixture(scope="function") + def download_task_order(self, monkeypatch): + def _download_task_order(MockFileService, object_name): + return {"name": object_name} - monkeypatch.setattr( - "atst.domain.csp.files.MockFileService.download_task_order", - _download_task_order, - ) + monkeypatch.setattr( + "atst.domain.csp.files.MockFileService.download_task_order", + _download_task_order, + ) - # Create 3 new Task Orders - for i in range(3): - TaskOrderFactory.create(create_clins=[{"number": "0001"}]) + def test_sends_multiple_emails(self, send_mail, download_task_order): + # Create 3 Task Orders + for i in range(3): + TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + dispatch_send_task_order_files.run() - # Check that send_with_attachment was called once for each task order - assert mock.call_count == 3 - mock.reset_mock() + # Check that send_with_attachment was called once for each task order + assert send_mail.call_count == 3 - # Create new TO - task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - assert not task_order.pdf_last_sent_at + def test_kwargs(self, send_mail, download_task_order, app): + task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) + dispatch_send_task_order_files.run() - dispatch_send_task_order_files.run() + # Check that send_with_attachment was called with correct kwargs + send_mail.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} + ), + body=translate( + "email.task_order_sent.body", {"to_number": task_order.number} + ), + attachments=[ + { + "name": task_order.pdf.object_name, + "maintype": "application", + "subtype": "pdf", + } + ], + ) + assert task_order.pdf_last_sent_at - # Check that send_with_attachment was called with correct kwargs - 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} - ), - body=translate("email.task_order_sent.body", {"to_number": task_order.number}), - attachments=[ - { - "name": task_order.pdf.object_name, - "maintype": "application", - "subtype": "pdf", - } - ], - ) + def test_send_failure(self, monkeypatch): + def _raise_smtp_exception(**kwargs): + raise SMTPException - assert task_order.pdf_last_sent_at + 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_send_failure(monkeypatch): - def _raise_smtp_exception(**kwargs): - raise SMTPException + def test_download_failure(self, send_mail, monkeypatch): + def _download_task_order(MockFileService, object_name): + raise AzureError("something went wrong") - monkeypatch.setattr("atst.jobs.send_mail", _raise_smtp_exception) + 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() - 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 + # Check that pdf_last_sent_at has not been updated + assert not task_order.pdf_last_sent_at From 1d8bb2811d7c0c5050bd41f5efcaa87f939f6073 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 18 Feb 2020 11:55:22 -0500 Subject: [PATCH 3/3] Update name of function --- atst/jobs.py | 4 ++-- atst/queue.py | 4 ++-- tests/test_jobs.py | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/atst/jobs.py b/atst/jobs.py index 55ff6027..c125bcb5 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -292,7 +292,7 @@ def dispatch_create_atat_admin_user(self): @celery.task(bind=True) -def dispatch_send_task_order_files(self): +def send_task_order_files(self): task_orders = TaskOrders.get_for_send_task_order_files() recipients = [app.config.get("MICROSOFT_TASK_ORDER_EMAIL_ADDRESS")] @@ -313,7 +313,7 @@ def dispatch_send_task_order_files(self): app.logger.exception(err) continue - task_order.pdf_last_sent_at = pendulum.now() + task_order.pdf_last_sent_at = pendulum.now(tz="UTC") db.session.add(task_order) db.session.commit() diff --git a/atst/queue.py b/atst/queue.py index 392fc8e2..5ed2f136 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -27,8 +27,8 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_create_environment_role", "schedule": 60, }, - "beat-dispatch_send_task_order_files": { - "task": "atst.jobs.dispatch_send_task_order_files", + "beat-send_task_order_files": { + "task": "atst.jobs.send_task_order_files", "schedule": 60, }, } diff --git a/tests/test_jobs.py b/tests/test_jobs.py index dd285ca4..ff681c7e 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -16,7 +16,6 @@ from atst.jobs import ( dispatch_create_user, dispatch_create_environment_role, dispatch_provision_portfolio, - dispatch_send_task_order_files, create_environment, do_create_user, do_provision_portfolio, @@ -24,6 +23,7 @@ from atst.jobs import ( do_create_environment_role, do_create_application, send_PPOC_email, + send_task_order_files, ) from tests.factories import ( ApplicationFactory, @@ -415,7 +415,7 @@ def test_create_environment_role(): assert env_role.cloud_id == "a-cloud-id" -class TestDispatchSendTaskOrderFiles: +class TestSendTaskOrderFiles: @pytest.fixture(scope="function") def send_mail(self, monkeypatch): mock = Mock() @@ -437,14 +437,14 @@ class TestDispatchSendTaskOrderFiles: for i in range(3): TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that send_with_attachment was called once for each task order assert send_mail.call_count == 3 def test_kwargs(self, send_mail, download_task_order, app): task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that send_with_attachment was called with correct kwargs send_mail.assert_called_once_with( @@ -471,7 +471,7 @@ class TestDispatchSendTaskOrderFiles: monkeypatch.setattr("atst.jobs.send_mail", _raise_smtp_exception) task_order = TaskOrderFactory.create(create_clins=[{"number": "0001"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that pdf_last_sent_at has not been updated assert not task_order.pdf_last_sent_at @@ -485,7 +485,7 @@ class TestDispatchSendTaskOrderFiles: _download_task_order, ) task_order = TaskOrderFactory.create(create_clins=[{"number": "0002"}]) - dispatch_send_task_order_files.run() + send_task_order_files.run() # Check that pdf_last_sent_at has not been updated assert not task_order.pdf_last_sent_at