From e736c90d002a90a25fb1534ac87458dd6df467d7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Feb 2018 18:38:32 +0000 Subject: [PATCH] Switch to using the pdf letter flow. When sending letters always use the pdf letter flow regardless of service permissions. --- app/celery/tasks.py | 43 +++----- app/config.py | 15 --- app/v2/notifications/post_notifications.py | 27 +++-- tests/app/celery/test_tasks.py | 100 ++---------------- .../test_post_letter_notifications.py | 77 +++----------- .../notifications/test_post_notifications.py | 24 ----- 6 files changed, 53 insertions(+), 233 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ff819e30e..697a160ed 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -119,21 +119,11 @@ def process_job(job_id): ).enumerated_recipients_and_personalisation: process_row(row_number, recipient, personalisation, template, job, service) - job_complete(job, service, template.template_type, start=start) + job_complete(job, start=start) -def job_complete(job, service, template_type, resumed=False, start=None): - if ( - template_type == LETTER_TYPE and - not service.has_permission('letters_as_pdf') - ): - if service.research_mode: - update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE) - else: - build_dvla_file.apply_async([str(job.id)], queue=QueueNames.JOBS) - current_app.logger.debug("send job {} to build-dvla-file in the {} queue".format(job.id, QueueNames.JOBS)) - else: - job.job_status = JOB_STATUS_FINISHED +def job_complete(job, resumed=False, start=None): + job.job_status = JOB_STATUS_FINISHED finished = datetime.utcnow() job.processing_finished = finished @@ -326,19 +316,18 @@ def save_letter( status=status ) - if service.has_permission('letters_as_pdf'): - if not service.research_mode: - letters_pdf_tasks.create_letters_pdf.apply_async( - [str(saved_notification.id)], - queue=QueueNames.CREATE_LETTERS_PDF - ) - elif current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: - research_mode_tasks.create_fake_letter_response_file.apply_async( - (saved_notification.reference,), - queue=QueueNames.RESEARCH_MODE - ) - else: - update_notification_status_by_reference(saved_notification.reference, 'delivered') + if not service.research_mode: + letters_pdf_tasks.create_letters_pdf.apply_async( + [str(saved_notification.id)], + queue=QueueNames.CREATE_LETTERS_PDF + ) + elif current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: + research_mode_tasks.create_fake_letter_response_file.apply_async( + (saved_notification.reference,), + queue=QueueNames.RESEARCH_MODE + ) + else: + update_notification_status_by_reference(saved_notification.reference, 'delivered') current_app.logger.debug("Letter {} created at {}".format(saved_notification.id, saved_notification.created_at)) except SQLAlchemyError as e: @@ -608,4 +597,4 @@ def process_incomplete_job(job_id): if row_number > resume_from_row: process_row(row_number, recipient, personalisation, template, job, job.service) - job_complete(job, job.service, template.template_type, resumed=True) + job_complete(job, resumed=True) diff --git a/app/config.py b/app/config.py index a29e654b9..3cbe9130c 100644 --- a/app/config.py +++ b/app/config.py @@ -170,11 +170,6 @@ class Config(object): 'schedule': crontab(minute=1), 'options': {'queue': QueueNames.PERIODIC} }, - # 'send-scheduled-notifications': { - # 'task': 'send-scheduled-notifications', - # 'schedule': crontab(minute='*/15'), - # 'options': {'queue': 'periodic'} - # }, 'delete-verify-codes': { 'task': 'delete-verify-codes', 'schedule': timedelta(minutes=63), @@ -252,11 +247,6 @@ class Config(object): 'schedule': crontab(hour=16, minute=30), 'options': {'queue': QueueNames.PERIODIC} }, - 'run-letter-jobs': { - 'task': 'run-letter-jobs', - 'schedule': crontab(hour=17, minute=30), - 'options': {'queue': QueueNames.PERIODIC} - }, 'trigger-letter-pdfs-for-day': { 'task': 'trigger-letter-pdfs-for-day', 'schedule': crontab(hour=17, minute=50), @@ -267,11 +257,6 @@ class Config(object): 'schedule': crontab(hour=23, minute=00), 'options': {'queue': QueueNames.PERIODIC} }, - 'run-letter-api-notifications': { - 'task': 'run-letter-api-notifications', - 'schedule': crontab(hour=17, minute=40), - 'options': {'queue': QueueNames.PERIODIC} - }, 'check-job-status': { 'task': 'check-job-status', 'schedule': crontab(), diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index a619c0dc3..1e802697d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -183,20 +183,19 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text status=status, reply_to_text=reply_to_text) - if api_key.service.has_permission('letters_as_pdf'): - if should_send: - create_letters_pdf.apply_async( - [str(notification.id)], - queue=QueueNames.CREATE_LETTERS_PDF - ) - elif (api_key.service.research_mode and - current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']): - create_fake_letter_response_file.apply_async( - (notification.reference,), - queue=QueueNames.RESEARCH_MODE - ) - else: - update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED) + if should_send: + create_letters_pdf.apply_async( + [str(notification.id)], + queue=QueueNames.CREATE_LETTERS_PDF + ) + elif (api_key.service.research_mode and + current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']): + create_fake_letter_response_file.apply_async( + (notification.reference,), + queue=QueueNames.RESEARCH_MODE + ) + else: + update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED) return notification diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index f3967cff9..1f0f3407e 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -20,7 +20,6 @@ from app.celery.scheduled_tasks import check_job_status from app.celery.tasks import ( build_dvla_file, create_dvla_file_contents_for_job, - job_complete, process_job, process_row, save_sms, @@ -33,7 +32,7 @@ from app.celery.tasks import ( send_inbound_sms_to_service, ) from app.config import QueueNames -from app.dao import jobs_dao, services_dao, service_permissions_dao +from app.dao import jobs_dao, services_dao from app.models import ( Job, Notification, @@ -110,9 +109,7 @@ def test_should_process_sms_job(sample_job, mocker): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) mocker.patch('app.celery.tasks.save_sms.apply_async') mocker.patch('app.encryption.encrypt', return_value="something_encrypted") - mocker.patch('app.celery.tasks.build_dvla_file') mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") - mocker.patch('app.celery.tasks.build_dvla_file') process_job(sample_job.id) s3.get_job_from_s3.assert_called_once_with( @@ -132,7 +129,6 @@ def test_should_process_sms_job(sample_job, mocker): ) job = jobs_dao.dao_get_job_by_id(sample_job.id) assert job.job_status == 'finished' - tasks.build_dvla_file.assert_not_called() @freeze_time("2016-01-01 11:09:00.061258") @@ -144,7 +140,6 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits(notify_db, mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms')) mocker.patch('app.celery.tasks.process_row') - mocker.patch('app.celery.tasks.build_dvla_file') process_job(job.id) @@ -152,7 +147,6 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits(notify_db, assert job.job_status == 'sending limits exceeded' assert s3.get_job_from_s3.called is False assert tasks.process_row.called is False - tasks.build_dvla_file.assert_not_called() def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify_db, @@ -165,7 +159,6 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms')) mocker.patch('app.celery.tasks.process_row') - mocker.patch('app.celery.tasks.build_dvla_file') process_job(job.id) @@ -173,7 +166,6 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify assert job.job_status == 'sending limits exceeded' assert s3.get_job_from_s3.called is False assert tasks.process_row.called is False - tasks.build_dvla_file.assert_not_called() def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(notify_db, notify_db_session, mocker): @@ -185,7 +177,6 @@ def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(noti mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.process_row') - mocker.patch('app.celery.tasks.build_dvla_file') process_job(job.id) @@ -193,7 +184,6 @@ def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(noti assert job.job_status == 'sending limits exceeded' assert s3.get_job_from_s3.called is False assert tasks.process_row.called is False - tasks.build_dvla_file.assert_not_called() @freeze_time("2016-01-01 11:09:00.061258") @@ -204,7 +194,6 @@ def test_should_not_process_email_job_if_would_exceed_send_limits(notify_db, not mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.process_row') - mocker.patch('app.celery.tasks.build_dvla_file') process_job(job.id) @@ -212,7 +201,6 @@ def test_should_not_process_email_job_if_would_exceed_send_limits(notify_db, not assert job.job_status == 'sending limits exceeded' assert s3.get_job_from_s3.called is False assert tasks.process_row.called is False - tasks.build_dvla_file.assert_not_called() def test_should_not_process_job_if_already_pending(notify_db, notify_db_session, mocker): @@ -220,13 +208,11 @@ def test_should_not_process_job_if_already_pending(notify_db, notify_db_session, mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.process_row') - mocker.patch('app.celery.tasks.build_dvla_file') process_job(job.id) assert s3.get_job_from_s3.called is False assert tasks.process_row.called is False - tasks.build_dvla_file.assert_not_called() def test_should_process_email_job_if_exactly_on_send_limits(notify_db, @@ -313,7 +299,6 @@ def test_should_process_letter_job(sample_letter_job, mocker): s3_mock = mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) process_row_mock = mocker.patch('app.celery.tasks.process_row') mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") - mocker.patch('app.celery.tasks.build_dvla_file') process_job(sample_letter_job.id) @@ -338,8 +323,7 @@ def test_should_process_letter_job(sample_letter_job, mocker): assert process_row_mock.call_count == 1 - assert sample_letter_job.job_status == 'in progress' - tasks.build_dvla_file.apply_async.assert_called_once_with([str(sample_letter_job.id)], queue="job-tasks") + assert sample_letter_job.job_status == 'finished' def test_should_process_all_sms_job(sample_job_with_placeholdered_template, @@ -650,60 +634,6 @@ def test_should_put_save_email_task_in_research_mode_queue_if_research_mode_serv ) -def test_should_not_build_dvla_file_in_research_mode_for_letter_job( - mocker, sample_letter_job, fake_uuid -): - test_encrypted_data = 'some encrypted data' - sample_letter_job.service.research_mode = True - - csv = """address_line_1,address_line_2,address_line_3,address_line_4,postcode,name - A1,A2,A3,A4,A_POST,Alice - """ - mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) - mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') - mocker.patch('app.celery.tasks.save_letter.apply_async') - mocker.patch('app.celery.tasks.create_uuid', return_value=fake_uuid) - mocker.patch('app.celery.tasks.encryption.encrypt', return_value=test_encrypted_data) - mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') - - process_job(sample_letter_job.id) - - assert not mock_dvla_file_task.called - - -def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( - mocker, sample_letter_job, fake_uuid -): - test_encrypted_data = 'some encrypted data' - sample_letter_job.service.research_mode = True - - csv = """address_line_1,address_line_2,address_line_3,address_line_4,postcode,name - A1,A2,A3,A4,A_POST,Alice - """ - mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) - mock_update_job_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') - mocker.patch('app.celery.tasks.save_letter.apply_async') - mocker.patch('app.celery.tasks.create_uuid', return_value=fake_uuid) - mocker.patch('app.celery.tasks.encryption.encrypt', return_value=test_encrypted_data) - mocker.patch('app.celery.tasks.build_dvla_file.apply_async') - - process_job(sample_letter_job.id) - - job = jobs_dao.dao_get_job_by_id(sample_letter_job.id) - - save_letter.apply_async.assert_called_once_with( - ( - str(sample_letter_job.service_id), - fake_uuid, - test_encrypted_data, - ), - queue=QueueNames.RESEARCH_MODE - ) - - mock_update_job_task.assert_called_once_with( - [str(job.id)], queue=QueueNames.RESEARCH_MODE) - - def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, sample_api_key, mocker): notification = _notification_json( sample_job.template, @@ -999,6 +929,7 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session): job = create_job(template=template) mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life") + mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') personalisation = { 'addressline1': 'Foo', @@ -1048,6 +979,7 @@ def test_save_letter_saves_letter_to_database_right_reply_to(mocker, notify_db_s job = create_job(template=template) mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life") + mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') personalisation = { 'addressline1': 'Foo', @@ -1107,6 +1039,7 @@ def test_save_letter_uses_template_reply_to_text(mocker, notify_db_session): job = create_job(template=template) mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life") + mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') personalisation = { 'addressline1': 'Foo', @@ -1155,7 +1088,6 @@ def test_save_letter_sets_delivered_letters_as_pdf_permission_in_research_mode_i notify_api, mocker, notify_db_session, sample_letter_job, env): sample_letter_job.service.research_mode = True sample_reference = "this-is-random-in-real-life" - service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') mock_create_fake_letter_response_file = mocker.patch( 'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async') mocker.patch('app.celery.tasks.create_random_identifier', return_value=sample_reference) @@ -1189,11 +1121,10 @@ def test_save_letter_sets_delivered_letters_as_pdf_permission_in_research_mode_i @pytest.mark.parametrize('env', ['development', 'preview']) -def test_save_letter_calls_create_fake_response_for_letters_as_pdf_permission_in_research_mode_on_development_preview( +def test_save_letter_calls_create_fake_response_for_letters_in_research_mode_on_development_preview( notify_api, mocker, notify_db_session, sample_letter_job, env): sample_letter_job.service.research_mode = True sample_reference = "this-is-random-in-real-life" - service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') mock_create_fake_letter_response_file = mocker.patch( 'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async') mocker.patch('app.celery.tasks.create_random_identifier', return_value=sample_reference) @@ -1227,9 +1158,8 @@ def test_save_letter_calls_create_fake_response_for_letters_as_pdf_permission_in ) -def test_save_letter_calls_create_letters_pdf_task_with_letters_as_pdf_permission_and_not_in_research( +def test_save_letter_calls_create_letters_pdf_task_not_in_research( mocker, notify_db_session, sample_letter_job): - service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') mock_create_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') personalisation = { @@ -1259,18 +1189,6 @@ def test_save_letter_calls_create_letters_pdf_task_with_letters_as_pdf_permissio ) -def test_job_complete_does_not_call_build_dvla_file_with_letters_as_pdf_permission( - mocker, notify_db_session, sample_letter_job): - service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') - mock_build_dvla_files = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') - - job_complete(sample_letter_job, sample_letter_job.service, sample_letter_job.template.template_type) - - assert not sample_letter_job.service.research_mode - assert not mock_build_dvla_files.called - assert sample_letter_job.job_status == JOB_STATUS_FINISHED - - def test_should_cancel_job_if_service_is_inactive(sample_service, sample_job, mocker): @@ -1278,7 +1196,6 @@ def test_should_cancel_job_if_service_is_inactive(sample_service, mocker.patch('app.celery.tasks.s3.get_job_from_s3') mocker.patch('app.celery.tasks.process_row') - mock_dvla_file_task = mocker.patch('app.celery.tasks.build_dvla_file') process_job(sample_job.id) @@ -1286,7 +1203,6 @@ def test_should_cancel_job_if_service_is_inactive(sample_service, assert job.job_status == 'cancelled' s3.get_job_from_s3.assert_not_called() tasks.process_row.assert_not_called() - mock_dvla_file_task.assert_not_called() @pytest.mark.parametrize('template_type, expected_class', [ @@ -1669,7 +1585,6 @@ def test_process_incomplete_job_email(mocker, sample_email_template): def test_process_incomplete_job_letter(mocker, sample_letter_template): mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_letter')) mock_letter_saver = mocker.patch('app.celery.tasks.save_letter.apply_async') - mock_build_dvla = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') job = create_job(template=sample_letter_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1684,5 +1599,4 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template): process_incomplete_job(str(job.id)) - assert mock_build_dvla.called assert mock_letter_saver.call_count == 8 diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 31830da45..f426e2efa 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -5,7 +5,6 @@ from flask import url_for import pytest from app.config import QueueNames -from app.dao import service_permissions_dao from app.models import EMAIL_TYPE from app.models import Job from app.models import KEY_TYPE_NORMAL @@ -46,6 +45,7 @@ def letter_request(client, data, service_id, key_type=KEY_TYPE_NORMAL, _expected @pytest.mark.parametrize('reference', [None, 'reference_from_client']) def test_post_letter_notification_returns_201(client, sample_letter_template, mocker, reference): + mock = mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') data = { 'template_id': str(sample_letter_template.id), 'personalisation': { @@ -81,38 +81,15 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo ) assert not resp_json['scheduled_for'] assert not notification.reply_to_text - - -def test_post_letter_notification_for_letters_as_pdf_calls_celery_task(client, sample_letter_template, mocker): - service_permissions_dao.dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') - - data = { - 'template_id': str(sample_letter_template.id), - 'personalisation': { - 'address_line_1': 'Her Royal Highness Queen Elizabeth II', - 'address_line_2': 'Buckingham Palace', - 'address_line_3': 'London', - 'postcode': 'SW1 1AA', - 'name': 'Lizzie' - }, - 'reference': 'foo' - } - fake_task = mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') - - letter_request(client, data, service_id=sample_letter_template.service_id) - - notification = Notification.query.one() - - fake_task.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF) + mock.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF) @pytest.mark.parametrize('env', [ 'development', 'preview', ]) -def test_post_letter_notification_for_letters_as_pdf_calls_create_fake_response_in_research_and_test_key_correct_env( +def test_post_letter_notification_calls_create_fake_response_in_research_and_test_key_correct_env( notify_api, client, sample_letter_template, mocker, env): - service_permissions_dao.dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') sample_letter_template.service.research_mode = True data = { @@ -147,9 +124,8 @@ def test_post_letter_notification_for_letters_as_pdf_calls_create_fake_response_ 'staging', 'live', ]) -def test_post_letter_noti_for_letters_as_pdf_sets_status_delivered_in_research_and_test_key_incorrect_env( +def test_post_letter_notification_sets_status_delivered_in_research_and_test_key_incorrect_env( notify_api, client, sample_letter_template, mocker, env): - service_permissions_dao.dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') sample_letter_template.service.research_mode = True data = { @@ -186,9 +162,8 @@ def test_post_letter_noti_for_letters_as_pdf_sets_status_delivered_in_research_a 'staging', 'live', ]) -def test_post_letter_noti_for_letters_as_pdf_sets_status_to_delivered_using_test_key_and_not_research_all_env( +def test_post_letter_notification_sets_status_to_delivered_using_test_key_and_not_research_all_env( notify_api, client, sample_letter_template, mocker, env): - service_permissions_dao.dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') sample_letter_template.service.research_mode = False data = { @@ -350,32 +325,8 @@ def test_post_letter_notification_returns_403_if_not_allowed_to_send_notificatio ] -@pytest.mark.parametrize('research_mode, key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_post_letter_notification_updates_noti_sending( - client, - notify_db_session, - mocker, - research_mode, - key_type -): - service = create_service(research_mode=research_mode, service_permissions=[LETTER_TYPE]) - template = create_template(service, template_type=LETTER_TYPE) - - data = { - 'template_id': str(template.id), - 'personalisation': {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'} - } - - letter_request(client, data, service_id=service.id, key_type=key_type) - - notification = Notification.query.one() - assert notification.status == NOTIFICATION_SENDING - - -def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_template): +def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_template, mocker): + mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') data = { 'template_id': str(sample_letter_template.id), 'personalisation': {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'} @@ -393,7 +344,8 @@ def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_t assert error_json['errors'] == [{'error': 'BadRequestError', 'message': 'Cannot send letters with a team api key'}] -def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_letter_template): +def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_letter_template, mocker): + mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') data = { 'template_id': str(sample_trial_letter_template.id), 'personalisation': {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'} @@ -411,11 +363,13 @@ def test_post_letter_notification_doesnt_send_in_trial(client, sample_trial_lett {'error': 'BadRequestError', 'message': 'Cannot send letters when service is in trial mode'}] -def test_post_letter_notification_fakes_dvla_when_service_is_in_trial_mode_but_using_test_key( +def test_post_letter_notification_is_delivered_if_in_trial_mode_and_using_test_key( client, sample_trial_letter_template, mocker ): + fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') + data = { "template_id": sample_trial_letter_template.id, "personalisation": {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'} @@ -424,12 +378,15 @@ def test_post_letter_notification_fakes_dvla_when_service_is_in_trial_mode_but_u letter_request(client, data=data, service_id=sample_trial_letter_template.service_id, key_type=KEY_TYPE_TEST) notification = Notification.query.one() - assert notification.status == NOTIFICATION_SENDING + assert notification.status == NOTIFICATION_DELIVERED + assert not fake_create_letter_task.called def test_post_letter_notification_persists_notification_reply_to_text( - client, notify_db_session + client, notify_db_session, mocker ): + mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') + service = create_service(service_permissions=[LETTER_TYPE]) service_address = "12 Main Street, London" letter_contact = create_letter_contact(service=service, contact_block=service_address, is_default=True) diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index df669d258..7ce825ac7 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -28,7 +28,6 @@ from tests.app.db import ( create_service, create_template, create_reply_to_email, - create_letter_contact, create_service_sms_sender, create_service_with_inbound_number ) @@ -695,26 +694,3 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa assert 'email_reply_to_id {} does not exist in database for service id {}'. \ format(fake_uuid, sample_email_template.service_id) in resp_json['errors'][0]['message'] assert 'BadRequestError' in resp_json['errors'][0]['error'] - - -def test_letter_notification_should_use_template_reply_to(client, sample_letter_template): - letter_contact = create_letter_contact(sample_letter_template.service, "Edinburgh, ED1 1AA", is_default=False) - sample_letter_template.reply_to = str(letter_contact.id) - - data = { - 'template_id': str(sample_letter_template.id), - 'personalisation': { - 'address_line_1': '123', - 'address_line_2': '234', - 'postcode': 'W1A1AA', - } - } - response = client.post("v2/notifications/letter", - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), - create_authorization_header(service_id=sample_letter_template.service.id)] - ) - assert response.status_code == 201, response.get_data(as_text=True) - notifications = Notification.query.all() - assert len(notifications) == 1 - assert notifications[0].reply_to_text == "Edinburgh, ED1 1AA"