diff --git a/app/aws/s3.py b/app/aws/s3.py index 48a54e709..927eb0ad1 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -72,7 +72,7 @@ def remove_s3_object(bucket_name, object_key): def remove_transformed_dvla_file(job_id): - bucket_name = current_app.config['DVLA_UPLOAD_BUCKET_NAME'] + bucket_name = current_app.config['DVLA_BUCKETS']['job'] file_location = '{}-dvla-job.text'.format(job_id) obj = get_s3_object(bucket_name, file_location) return obj.delete() diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index e18cbe8e6..f014c65ed 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -5,6 +5,7 @@ from datetime import ( from flask import current_app from sqlalchemy.exc import SQLAlchemyError +from notifications_utils.s3 import s3upload from app.aws import s3 from app import notify_celery @@ -27,7 +28,9 @@ from app.dao.notifications_dao import ( is_delivery_slow_for_provider, delete_notifications_created_more_than_a_week_ago_by_type, dao_get_scheduled_notifications, - set_scheduled_notification_to_processed) + set_scheduled_notification_to_processed, + dao_set_created_live_letter_api_notifications_to_pending, +) from app.dao.statistics_dao import dao_timeout_job_statistics from app.dao.provider_details_dao import ( get_current_provider, @@ -37,7 +40,7 @@ from app.dao.users_dao import delete_codes_older_created_more_than_a_day_ago from app.models import LETTER_TYPE, JOB_STATUS_READY_TO_SEND from app.notifications.process_notifications import send_notification_to_queue from app.statsd_decorators import statsd -from app.celery.tasks import process_job +from app.celery.tasks import process_job, create_dvla_file_contents_for_notifications from app.config import QueueNames, TaskNames from app.utils import convert_utc_to_bst @@ -313,9 +316,41 @@ def populate_monthly_billing(): @statsd(namespace="tasks") def run_letter_jobs(): job_ids = dao_get_letter_job_ids_by_status(JOB_STATUS_READY_TO_SEND) - notify_celery.send_task( - name=TaskNames.DVLA_JOBS, - args=(job_ids,), - queue=QueueNames.PROCESS_FTP - ) - current_app.logger.info("Queued {} ready letter job ids onto {}".format(len(job_ids), QueueNames.PROCESS_FTP)) + if job_ids: + notify_celery.send_task( + name=TaskNames.DVLA_JOBS, + args=(job_ids,), + queue=QueueNames.PROCESS_FTP + ) + current_app.logger.info("Queued {} ready letter job ids onto {}".format(len(job_ids), QueueNames.PROCESS_FTP)) + + +@notify_celery.task(name="run-letter-api-notifications") +@statsd(namespace="tasks") +def run_letter_api_notifications(): + current_time = datetime.utcnow().isoformat() + + notifications = dao_set_created_live_letter_api_notifications_to_pending() + + if notifications: + file_contents = create_dvla_file_contents_for_notifications(notifications) + + filename = '{}-dvla-notifications.txt'.format(current_time) + s3upload( + filedata=file_contents + '\n', + region=current_app.config['AWS_REGION'], + bucket_name=current_app.config['DVLA_BUCKETS']['notification'], + file_location=filename + ) + + notify_celery.send_task( + name=TaskNames.DVLA_NOTIFICATIONS, + kwargs={'filename': filename}, + queue=QueueNames.PROCESS_FTP + ) + current_app.logger.info( + "Queued {} ready letter api notifications onto {}".format( + len(notifications), + QueueNames.PROCESS_FTP + ) + ) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index e621350ce..a34cae506 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -27,7 +27,11 @@ from app.dao.jobs_dao import ( all_notifications_are_created_for_job, dao_get_all_notifications_for_job, dao_update_job_status) -from app.dao.notifications_dao import get_notification_by_id, dao_update_notifications_sent_to_dvla +from app.dao.notifications_dao import ( + get_notification_by_id, + dao_update_notifications_for_job_to_sent_to_dvla, + dao_update_notifications_by_reference +) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count @@ -42,7 +46,10 @@ from app.models import ( JOB_STATUS_IN_PROGRESS, JOB_STATUS_FINISHED, JOB_STATUS_READY_TO_SEND, - JOB_STATUS_SENT_TO_DVLA, JOB_STATUS_ERROR) + JOB_STATUS_SENT_TO_DVLA, JOB_STATUS_ERROR, + NOTIFICATION_SENDING, + NOTIFICATION_TECHNICAL_FAILURE +) from app.notifications.process_notifications import persist_notification from app.service.utils import service_allowed_to_send_to from app.statsd_decorators import statsd @@ -279,11 +286,11 @@ def persist_letter( def build_dvla_file(self, job_id): try: if all_notifications_are_created_for_job(job_id): - file_contents = create_dvla_file_contents(job_id) + file_contents = create_dvla_file_contents_for_job(job_id) s3upload( filedata=file_contents + '\n', region=current_app.config['AWS_REGION'], - bucket_name=current_app.config['DVLA_UPLOAD_BUCKET_NAME'], + bucket_name=current_app.config['DVLA_BUCKETS']['job'], file_location="{}-dvla-job.text".format(job_id) ) dao_update_job_status(job_id, JOB_STATUS_READY_TO_SEND) @@ -302,7 +309,7 @@ def update_job_to_sent_to_dvla(self, job_id): # and update all notifications for this job to sending, provider = DVLA provider = get_current_provider(LETTER_TYPE) - updated_count = dao_update_notifications_sent_to_dvla(job_id, provider.identifier) + updated_count = dao_update_notifications_for_job_to_sent_to_dvla(job_id, provider.identifier) dao_update_job_status(job_id, JOB_STATUS_SENT_TO_DVLA) current_app.logger.info("Updated {} letter notifications to sending. " @@ -316,7 +323,48 @@ def update_dvla_job_to_error(self, job_id): current_app.logger.info("Updated {} job to {}".format(job_id, JOB_STATUS_ERROR)) -def create_dvla_file_contents(job_id): +@notify_celery.task(bind=True, name='update-letter-notifications-to-sent') +@statsd(namespace="tasks") +def update_letter_notifications_to_sent_to_dvla(self, notification_references): + # This task will be called by the FTP app to update notifications as sent to DVLA + provider = get_current_provider(LETTER_TYPE) + + updated_count = dao_update_notifications_by_reference( + notification_references, + { + 'status': NOTIFICATION_SENDING, + 'sent_by': provider.identifier, + 'sent_at': datetime.utcnow(), + 'updated_at': datetime.utcnow() + } + ) + + current_app.logger.info("Updated {} letter notifications to sending".format(updated_count)) + + +@notify_celery.task(bind=True, name='update-letter-notifications-to-error') +@statsd(namespace="tasks") +def update_letter_notifications_to_error(self, notification_references): + # This task will be called by the FTP app to update notifications as sent to DVLA + + updated_count = dao_update_notifications_by_reference( + notification_references, + { + 'status': NOTIFICATION_TECHNICAL_FAILURE, + 'updated_at': datetime.utcnow() + } + ) + + current_app.logger.info("Updated {} letter notifications to technical-failure".format(updated_count)) + + +def create_dvla_file_contents_for_job(job_id): + notifications = dao_get_all_notifications_for_job(job_id) + + return create_dvla_file_contents_for_notifications(notifications) + + +def create_dvla_file_contents_for_notifications(notifications): file_contents = '\n'.join( str(LetterDVLATemplate( notification.template.__dict__, @@ -325,7 +373,7 @@ def create_dvla_file_contents(job_id): contact_block=notification.service.letter_contact_block, org_id=notification.service.dvla_organisation.id, )) - for notification in dao_get_all_notifications_for_job(job_id) + for notification in notifications ) return file_contents diff --git a/app/config.py b/app/config.py index 6f19d68be..7730c4254 100644 --- a/app/config.py +++ b/app/config.py @@ -49,7 +49,7 @@ class QueueNames(object): class TaskNames(object): DVLA_JOBS = 'send-jobs-to-dvla' - DVLA_NOTIFICATIONS = 'send-notifications-to-dvla' + DVLA_NOTIFICATIONS = 'send-api-notifications-to-dvla' class Config(object): @@ -137,7 +137,7 @@ class Config(object): 'visibility_timeout': 310, 'queue_name_prefix': NOTIFICATION_QUEUE_PREFIX } - CELERY_ENABLE_UTC = True, + CELERY_ENABLE_UTC = True CELERY_TIMEZONE = 'Europe/London' CELERY_ACCEPT_CONTENT = ['json'] CELERY_TASK_SERIALIZER = 'json' @@ -165,27 +165,27 @@ class Config(object): }, 'delete-sms-notifications': { 'task': 'delete-sms-notifications', - 'schedule': crontab(minute=0, hour=0), + 'schedule': crontab(hour=0, minute=0), 'options': {'queue': QueueNames.PERIODIC} }, 'delete-email-notifications': { 'task': 'delete-email-notifications', - 'schedule': crontab(minute=20, hour=0), + 'schedule': crontab(hour=0, minute=20), 'options': {'queue': QueueNames.PERIODIC} }, 'delete-letter-notifications': { 'task': 'delete-letter-notifications', - 'schedule': crontab(minute=40, hour=0), + 'schedule': crontab(hour=0, minute=40), 'options': {'queue': QueueNames.PERIODIC} }, 'delete-inbound-sms': { 'task': 'delete-inbound-sms', - 'schedule': crontab(minute=0, hour=1), + 'schedule': crontab(hour=1, minute=0), 'options': {'queue': QueueNames.PERIODIC} }, 'send-daily-performance-platform-stats': { 'task': 'send-daily-performance-platform-stats', - 'schedule': crontab(minute=0, hour=2), + 'schedule': crontab(hour=2, minute=0), 'options': {'queue': QueueNames.PERIODIC} }, 'switch-current-sms-provider-on-slow-delivery': { @@ -195,39 +195,44 @@ class Config(object): }, 'timeout-sending-notifications': { 'task': 'timeout-sending-notifications', - 'schedule': crontab(minute=0, hour=3), + 'schedule': crontab(hour=3, minute=0), 'options': {'queue': QueueNames.PERIODIC} }, 'remove_sms_email_jobs': { 'task': 'remove_csv_files', - 'schedule': crontab(minute=0, hour=4), + 'schedule': crontab(hour=4, minute=0), 'options': {'queue': QueueNames.PERIODIC}, 'kwargs': {'job_types': [EMAIL_TYPE, SMS_TYPE]} }, 'remove_letter_jobs': { 'task': 'remove_csv_files', - 'schedule': crontab(minute=20, hour=4), + 'schedule': crontab(hour=4, minute=20), 'options': {'queue': QueueNames.PERIODIC}, 'kwargs': {'job_types': [LETTER_TYPE]} }, 'remove_transformed_dvla_files': { 'task': 'remove_transformed_dvla_files', - 'schedule': crontab(minute=40, hour=4), + 'schedule': crontab(hour=4, minute=40), 'options': {'queue': QueueNames.PERIODIC} }, 'timeout-job-statistics': { 'task': 'timeout-job-statistics', - 'schedule': crontab(minute=0, hour=5), + 'schedule': crontab(hour=5, minute=0), 'options': {'queue': QueueNames.PERIODIC} }, 'populate_monthly_billing': { 'task': 'populate_monthly_billing', - 'schedule': crontab(minute=10, hour=5), + 'schedule': crontab(hour=5, minute=10), 'options': {'queue': QueueNames.PERIODIC} }, 'run-letter-jobs': { 'task': 'run-letter-jobs', - 'schedule': crontab(hour=17, minute=30), + 'schedule': crontab(hour=5, minute=30), + 'options': {'queue': QueueNames.PERIODIC} + }, + 'run-letter-api-notifications': { + 'task': 'run-letter-api-notifications', + 'schedule': crontab(hour=5, minute=40), 'options': {'queue': QueueNames.PERIODIC} } } @@ -253,7 +258,10 @@ class Config(object): FUNCTIONAL_TEST_PROVIDER_SERVICE_ID = None FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID = None - DVLA_UPLOAD_BUCKET_NAME = "{}-dvla-file-per-job".format(os.getenv('NOTIFY_ENVIRONMENT')) + DVLA_BUCKETS = { + 'job': '{}-dvla-file-per-job'.format(os.getenv('NOTIFY_ENVIRONMENT')), + 'notification': '{}-dvla-letter-api-files'.format(os.getenv('NOTIFY_ENVIRONMENT')) + } API_KEY_LIMITS = { KEY_TYPE_TEAM: { diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 17e388422..fef7b9567 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -460,7 +460,7 @@ def is_delivery_slow_for_provider( @statsd(namespace="dao") @transactional -def dao_update_notifications_sent_to_dvla(job_id, provider): +def dao_update_notifications_for_job_to_sent_to_dvla(job_id, provider): now = datetime.utcnow() updated_count = db.session.query( Notification).filter(Notification.job_id == job_id).update( @@ -473,6 +473,27 @@ def dao_update_notifications_sent_to_dvla(job_id, provider): return updated_count +@statsd(namespace="dao") +@transactional +def dao_update_notifications_by_reference(references, update_dict): + now = datetime.utcnow() + updated_count = Notification.query.filter( + Notification.reference.in_(references) + ).update( + update_dict, + synchronize_session=False + ) + + NotificationHistory.query.filter( + NotificationHistory.reference.in_(references) + ).update( + update_dict, + synchronize_session=False + ) + + return updated_count + + @statsd(namespace="dao") def dao_get_notifications_by_to_field(service_id, search_term, statuses=None): try: @@ -555,3 +576,30 @@ def dao_get_total_notifications_sent_per_day_for_performance_platform(start_date NotificationHistory.key_type != KEY_TYPE_TEST, NotificationHistory.notification_type != LETTER_TYPE ).one() + + +def dao_set_created_live_letter_api_notifications_to_pending(): + """ + Sets all past scheduled jobs to pending, and then returns them for further processing. + + this is used in the run_scheduled_jobs task, so we put a FOR UPDATE lock on the job table for the duration of + the transaction so that if the task is run more than once concurrently, one task will block the other select + from completing until it commits. + """ + notifications = db.session.query( + Notification + ).filter( + Notification.notification_type == LETTER_TYPE, + Notification.status == NOTIFICATION_CREATED, + Notification.key_type == KEY_TYPE_NORMAL, + Notification.api_key != None # noqa + ).with_for_update( + ).all() + + for notification in notifications: + notification.status = NOTIFICATION_PENDING + + db.session.add_all(notifications) + db.session.commit() + + return notifications diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 332f6ed32..32b151f86 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -1,45 +1,23 @@ from app import create_random_identifier -from app.models import LETTER_TYPE, JOB_STATUS_READY_TO_SEND, Job -from app.dao.jobs_dao import dao_create_job +from app.models import LETTER_TYPE from app.notifications.process_notifications import persist_notification -from app.v2.errors import InvalidRequest -from app.variables import LETTER_API_FILENAME -def create_letter_api_job(template): - service = template.service - if not service.active: - raise InvalidRequest('Service {} is inactive'.format(service.id), 403) - if template.archived: - raise InvalidRequest('Template {} is deleted'.format(template.id), 400) - - job = Job( - original_file_name=LETTER_API_FILENAME, - service=service, - template=template, - template_version=template.version, - notification_count=1, - job_status=JOB_STATUS_READY_TO_SEND, - created_by=None - ) - dao_create_job(job) - return job - - -def create_letter_notification(letter_data, job, api_key): +def create_letter_notification(letter_data, template, api_key, status): notification = persist_notification( - template_id=job.template.id, - template_version=job.template.version, + template_id=template.id, + template_version=template.version, # we only accept addresses_with_underscores from the API (from CSV we also accept dashes, spaces etc) recipient=letter_data['personalisation']['address_line_1'], - service=job.service, + service=template.service, personalisation=letter_data['personalisation'], notification_type=LETTER_TYPE, api_key_id=api_key.id, key_type=api_key.key_type, - job_id=job.id, - job_row_number=0, + job_id=None, + job_row_number=None, reference=create_random_identifier(), - client_reference=letter_data.get('reference') + client_reference=letter_data.get('reference'), + status=status ) return notification diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index e7e8779f2..8a75473b4 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -3,6 +3,7 @@ from datetime import datetime from flask import current_app +from notifications_utils.clients import redis from notifications_utils.recipients import ( get_international_phone_info, validate_and_format_phone_number, @@ -11,10 +12,8 @@ from notifications_utils.recipients import ( from app import redis_store from app.celery import provider_tasks -from notifications_utils.clients import redis - from app.config import QueueNames -from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, ScheduledNotification +from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, NOTIFICATION_CREATED, ScheduledNotification from app.dao.notifications_dao import (dao_create_notification, dao_delete_notifications_and_history_by_id, dao_created_scheduled_notification) @@ -52,7 +51,8 @@ def persist_notification( client_reference=None, notification_id=None, simulated=False, - created_by_id=None + created_by_id=None, + status=NOTIFICATION_CREATED ): notification_created_at = created_at or datetime.utcnow() if not notification_id: @@ -73,7 +73,8 @@ def persist_notification( job_row_number=job_row_number, client_reference=client_reference, reference=reference, - created_by_id=created_by_id + created_by_id=created_by_id, + status=status ) if notification_type == SMS_TYPE: diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 47a47a711..b3cf1c0f8 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -4,16 +4,23 @@ from flask import request, jsonify, current_app, abort from app import api_user, authenticated_service from app.config import QueueNames -from app.dao.jobs_dao import dao_update_job -from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, PRIORITY, KEY_TYPE_TEST, KEY_TYPE_TEAM -from app.celery.tasks import build_dvla_file, update_job_to_sent_to_dvla +from app.models import ( + SMS_TYPE, + EMAIL_TYPE, + LETTER_TYPE, + PRIORITY, + KEY_TYPE_TEST, + KEY_TYPE_TEAM, + NOTIFICATION_CREATED, + NOTIFICATION_SENDING +) +from app.celery.tasks import update_letter_notifications_to_sent_to_dvla from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, simulated_recipient, persist_scheduled_notification) from app.notifications.process_letter_notifications import ( - create_letter_api_job, create_letter_notification ) from app.notifications.validators import ( @@ -36,7 +43,6 @@ from app.v2.notifications.create_response import ( create_post_email_response_from_notification, create_post_letter_response_from_notification ) -from app.variables import LETTER_TEST_API_FILENAME @v2_notification_blueprint.route('/', methods=['POST']) @@ -153,19 +159,17 @@ def process_letter_notification(*, letter_data, api_key, template): if api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST: raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) - job = create_letter_api_job(template) - notification = create_letter_notification(letter_data, job, api_key) + should_send = not (api_key.service.research_mode or api_key.key_type == KEY_TYPE_TEST) - if api_key.service.research_mode or api_key.key_type == KEY_TYPE_TEST: - # distinguish real API jobs from test jobs by giving the test jobs a different filename - job.original_file_name = LETTER_TEST_API_FILENAME - dao_update_job(job) - 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) + # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up + status = NOTIFICATION_CREATED if should_send else NOTIFICATION_SENDING + + notification = create_letter_notification(letter_data, template, api_key, status=status) + + if not should_send: + update_letter_notifications_to_sent_to_dvla.apply_async( + kwargs={'notification_references': [notification.reference]}, + queue=QueueNames.RESEARCH_MODE + ) - current_app.logger.info("send job {} for api notification {} to build-dvla-file in the process-job queue".format( - job.id, - notification.id - )) return notification diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index b44d66fc8..d6e4c1b75 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -39,7 +39,7 @@ def test_remove_transformed_dvla_file_makes_correct_call(notify_api, mocker): remove_transformed_dvla_file(fake_uuid) s3_mock.assert_has_calls([ - call(current_app.config['DVLA_UPLOAD_BUCKET_NAME'], '{}-dvla-job.text'.format(fake_uuid)), + call(current_app.config['DVLA_BUCKETS']['job'], '{}-dvla-job.text'.format(fake_uuid)), call().delete() ]) diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py new file mode 100644 index 000000000..05680d32b --- /dev/null +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -0,0 +1,127 @@ +from datetime import datetime + +import pytest +from freezegun import freeze_time +from flask import current_app + +from app.models import ( + Job, + Notification, + NOTIFICATION_SENDING, + NOTIFICATION_CREATED, + NOTIFICATION_TECHNICAL_FAILURE +) +from app.celery.tasks import ( + update_job_to_sent_to_dvla, + update_dvla_job_to_error, + update_letter_notifications_statuses, + update_letter_notifications_to_sent_to_dvla, + update_letter_notifications_to_error, + process_updates_from_file +) + +from tests.app.db import create_notification +from tests.conftest import set_config + + +def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job): + create_notification(template=sample_letter_template, job=sample_letter_job) + create_notification(template=sample_letter_template, job=sample_letter_job) + update_job_to_sent_to_dvla(job_id=sample_letter_job.id) + + updated_notifications = Notification.query.all() + assert [(n.status == 'sending', n.sent_by == 'dvla') for n in updated_notifications] + + assert Job.query.filter_by(id=sample_letter_job.id).one().job_status == 'sent to dvla' + + +def test_update_dvla_job_to_error(sample_letter_template, sample_letter_job): + create_notification(template=sample_letter_template, job=sample_letter_job) + create_notification(template=sample_letter_template, job=sample_letter_job) + update_dvla_job_to_error(job_id=sample_letter_job.id) + + updated_notifications = Notification.query.all() + for n in updated_notifications: + assert n.status == 'created' + assert not n.sent_by + + assert Job.query.filter_by(id=sample_letter_job.id).one().job_status == 'error' + + +def test_update_letter_notifications_statuses_raises_for_invalid_format(notify_api, mocker): + invalid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2' + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=invalid_file) + + with pytest.raises(TypeError): + update_letter_notifications_statuses(filename='foo.txt') + + +def test_update_letter_notifications_statuses_calls_with_correct_bucket_location(notify_api, mocker): + s3_mock = mocker.patch('app.celery.tasks.s3.get_s3_object') + + with set_config(notify_api, 'NOTIFY_EMAIL_DOMAIN', 'foo.bar'): + update_letter_notifications_statuses(filename='foo.txt') + s3_mock.assert_called_with('{}-ftp'.format(current_app.config['NOTIFY_EMAIL_DOMAIN']), 'foo.txt') + + +def test_update_letter_notifications_statuses_builds_updates_from_content(notify_api, mocker): + valid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted' + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + update_mock = mocker.patch('app.celery.tasks.process_updates_from_file') + + update_letter_notifications_statuses(filename='foo.txt') + + update_mock.assert_called_with('ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted') + + +def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mocker): + valid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted' + updates = process_updates_from_file(valid_file) + + assert len(updates) == 2 + + assert updates[0].reference == 'ref-foo' + assert updates[0].status == 'Sent' + assert updates[0].page_count == '1' + assert updates[0].cost_threshold == 'Unsorted' + + assert updates[1].reference == 'ref-bar' + assert updates[1].status == 'Sent' + assert updates[1].page_count == '2' + assert updates[1].cost_threshold == 'Sorted' + + +def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references( + client, + sample_letter_template +): + first = create_notification(sample_letter_template) + second = create_notification(sample_letter_template) + + dt = datetime.utcnow() + with freeze_time(dt): + update_letter_notifications_to_sent_to_dvla([first.reference]) + + assert first.status == NOTIFICATION_SENDING + assert first.sent_by == 'dvla' + assert first.sent_at == dt + assert first.updated_at == dt + assert second.status == NOTIFICATION_CREATED + + +def test_update_letter_notifications_to_error_updates_based_on_notification_references( + client, + sample_letter_template +): + first = create_notification(sample_letter_template) + second = create_notification(sample_letter_template) + + dt = datetime.utcnow() + with freeze_time(dt): + update_letter_notifications_to_error([first.reference]) + + assert first.status == NOTIFICATION_TECHNICAL_FAILURE + assert first.sent_by is None + assert first.sent_at is None + assert first.updated_at == dt + assert second.status == NOTIFICATION_CREATED diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index fca018597..9d770888b 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -21,6 +21,7 @@ from app.celery.scheduled_tasks import ( remove_transformed_dvla_files, run_scheduled_jobs, run_letter_jobs, + run_letter_api_notifications, s3, send_daily_performance_platform_stats, send_scheduled_notifications, @@ -41,6 +42,11 @@ from app.models import ( Service, Template, SMS_TYPE, LETTER_TYPE, JOB_STATUS_READY_TO_SEND, + JOB_STATUS_IN_PROGRESS, + JOB_STATUS_SENT_TO_DVLA, + NOTIFICATION_PENDING, + NOTIFICATION_CREATED, + KEY_TYPE_TEST, MonthlyBilling) from app.utils import get_london_midnight_in_utc from tests.app.db import create_notification, create_service, create_template, create_job, create_rate @@ -693,3 +699,74 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): mock_celery.assert_called_once_with(name=TaskNames.DVLA_JOBS, args=(job_ids,), queue=QueueNames.PROCESS_FTP) + + +def test_run_letter_jobs_does_nothing_if_no_ready_jobs(client, mocker, sample_letter_template): + job_ids = [ + str(create_job(sample_letter_template, job_status=JOB_STATUS_IN_PROGRESS).id), + str(create_job(sample_letter_template, job_status=JOB_STATUS_SENT_TO_DVLA).id) + ] + + mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") + + run_letter_jobs() + + assert not mock_celery.called + + +def test_run_letter_api_notifications_triggers_ftp_task(client, mocker, sample_letter_notification): + file_contents_mock = mocker.patch( + 'app.celery.scheduled_tasks.create_dvla_file_contents_for_notifications', + return_value='foo\nbar' + ) + s3upload = mocker.patch('app.celery.scheduled_tasks.s3upload') + mock_celery = mocker.patch('app.celery.tasks.notify_celery.send_task') + filename = '2017-01-01T12:00:00-dvla-notifications.txt' + + with freeze_time('2017-01-01 12:00:00'): + run_letter_api_notifications() + + assert sample_letter_notification.status == NOTIFICATION_PENDING + file_contents_mock.assert_called_once_with([sample_letter_notification]) + s3upload.assert_called_once_with( + # with trailing new line added + filedata='foo\nbar\n', + region='eu-west-1', + bucket_name='test-dvla-letter-api-files', + file_location=filename + ) + mock_celery.assert_called_once_with( + name=TaskNames.DVLA_NOTIFICATIONS, + kwargs={'filename': filename}, + queue=QueueNames.PROCESS_FTP + ) + + +def test_run_letter_api_notifications_does_nothing_if_no_created_notifications( + client, + mocker, + sample_letter_template, + sample_letter_job, + sample_api_key +): + letter_job_notification = create_notification( + sample_letter_template, + job=sample_letter_job + ) + pending_letter_notification = create_notification( + sample_letter_template, + status=NOTIFICATION_PENDING, + api_key=sample_api_key + ) + test_api_key_notification = create_notification( + sample_letter_template, + key_type=KEY_TYPE_TEST + ) + + mock_celery = mocker.patch('app.celery.tasks.notify_celery.send_task') + + run_letter_api_notifications() + + assert not mock_celery.called + assert letter_job_notification.status == NOTIFICATION_CREATED + assert test_api_key_notification.status == NOTIFICATION_CREATED diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index d82249953..e16f819ea 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -17,7 +17,7 @@ from app.celery import tasks from app.celery.tasks import ( s3, build_dvla_file, - create_dvla_file_contents, + create_dvla_file_contents_for_job, update_dvla_job_to_error, process_job, process_row, @@ -25,9 +25,6 @@ from app.celery.tasks import ( send_email, persist_letter, get_template_class, - update_job_to_sent_to_dvla, - update_letter_notifications_statuses, - process_updates_from_file, send_inbound_sms_to_service) from app.config import QueueNames from app.dao import jobs_dao, services_dao @@ -39,11 +36,9 @@ from app.models import ( SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, - Job, - JOB_STATUS_ERROR) + Job) from tests.app import load_example_csv -from tests.conftest import set_config from tests.app.conftest import ( sample_service as create_sample_service, sample_template as create_sample_template, @@ -642,7 +637,7 @@ def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( 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') + mock_update_job_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') mocker.patch('app.celery.tasks.persist_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) @@ -662,7 +657,7 @@ def test_should_update_job_to_sent_to_dvla_in_research_mode_for_letter_job( queue=QueueNames.RESEARCH_MODE ) - update_job_to_sent_to_dvla.apply_async.assert_called_once_with( + mock_update_job_task.assert_called_once_with( [str(job.id)], queue=QueueNames.RESEARCH_MODE) @@ -1058,7 +1053,7 @@ def test_build_dvla_file(sample_letter_template, mocker): mocked_upload.assert_called_once_with( filedata="dvla|string\ndvla|string\n", region=current_app.config['AWS_REGION'], - bucket_name=current_app.config['DVLA_UPLOAD_BUCKET_NAME'], + bucket_name=current_app.config['DVLA_BUCKETS']['job'], file_location="{}-dvla-job.text".format(job.id) ) assert Job.query.get(job.id).job_status == 'ready to send' @@ -1081,7 +1076,7 @@ def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_let mocked_send_task.assert_not_called() -def test_create_dvla_file_contents(sample_letter_template, mocker): +def test_create_dvla_file_contents_for_job(sample_letter_template, mocker): job = create_job(template=sample_letter_template, notification_count=2) create_notification(template=job.template, job=job, reference=1) create_notification(template=job.template, job=job, reference=2) @@ -1089,7 +1084,7 @@ def test_create_dvla_file_contents(sample_letter_template, mocker): mocked_letter_template_instance = mocked_letter_template.return_value mocked_letter_template_instance.__str__.return_value = "dvla|string" - create_dvla_file_contents(job.id) + create_dvla_file_contents_for_job(job.id) calls = mocked_letter_template.call_args_list # Template assert calls[0][0][0]['subject'] == 'Template subject' @@ -1114,73 +1109,6 @@ def test_dvla_letter_template(sample_letter_notification): assert str(letter) == "140|500|001||random-string|||||||||||||A1||A2|A3|A4|A5|A6|A_POST|||||||||23 March 2017

Template subjectDear Sir/Madam, Hello. Yours Truly, The Government." # noqa -def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job): - create_notification(template=sample_letter_template, job=sample_letter_job) - create_notification(template=sample_letter_template, job=sample_letter_job) - update_job_to_sent_to_dvla(job_id=sample_letter_job.id) - - updated_notifications = Notification.query.all() - assert [(n.status == 'sending', n.sent_by == 'dvla') for n in updated_notifications] - - assert 'sent to dvla' == Job.query.filter_by(id=sample_letter_job.id).one().job_status - - -def test_update_dvla_job_to_error(sample_letter_template, sample_letter_job): - create_notification(template=sample_letter_template, job=sample_letter_job) - create_notification(template=sample_letter_template, job=sample_letter_job) - update_dvla_job_to_error(job_id=sample_letter_job.id) - - updated_notifications = Notification.query.all() - for n in updated_notifications: - assert n.status == 'created' - assert not n.sent_by - - assert 'error' == Job.query.filter_by(id=sample_letter_job.id).one().job_status - - -def test_update_letter_notifications_statuses_raises_for_invalid_format(notify_api, mocker): - invalid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2' - mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=invalid_file) - - with pytest.raises(TypeError): - update_letter_notifications_statuses(filename='foo.txt') - - -def test_update_letter_notifications_statuses_calls_with_correct_bucket_location(notify_api, mocker): - s3_mock = mocker.patch('app.celery.tasks.s3.get_s3_object') - - with set_config(notify_api, 'NOTIFY_EMAIL_DOMAIN', 'foo.bar'): - update_letter_notifications_statuses(filename='foo.txt') - s3_mock.assert_called_with('{}-ftp'.format(current_app.config['NOTIFY_EMAIL_DOMAIN']), 'foo.txt') - - -def test_update_letter_notifications_statuses_builds_updates_from_content(notify_api, mocker): - valid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted' - mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) - update_mock = mocker.patch('app.celery.tasks.process_updates_from_file') - - update_letter_notifications_statuses(filename='foo.txt') - - update_mock.assert_called_with('ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted') - - -def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mocker): - valid_file = 'ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted' - updates = process_updates_from_file(valid_file) - - assert len(updates) == 2 - - assert updates[0].reference == 'ref-foo' - assert updates[0].status == 'Sent' - assert updates[0].page_count == '1' - assert updates[0].cost_threshold == 'Unsorted' - - assert updates[1].reference == 'ref-bar' - assert updates[1].status == 'Sent' - assert updates[1].page_count == '2' - assert updates[1].cost_threshold == 'Sorted' - - def test_send_inbound_sms_to_service_post_https_request_to_service(notify_api, sample_service): inbound_api = create_service_inbound_api(service=sample_service, url="https://some.service.gov.uk/", bearer_token="something_unique") diff --git a/tests/app/dao/notification_dao/test_letter_api_notification_dao.py b/tests/app/dao/notification_dao/test_letter_api_notification_dao.py new file mode 100644 index 000000000..41869b15e --- /dev/null +++ b/tests/app/dao/notification_dao/test_letter_api_notification_dao.py @@ -0,0 +1,51 @@ +from app.models import ( + NOTIFICATION_CREATED, + NOTIFICATION_PENDING, + NOTIFICATION_SENDING, + KEY_TYPE_TEST, + KEY_TYPE_NORMAL +) +from app.dao.notifications_dao import dao_set_created_live_letter_api_notifications_to_pending + +from tests.app.db import create_notification + + +def test_should_only_get_letter_notifications( + sample_letter_notification, + sample_email_notification, + sample_notification +): + ret = dao_set_created_live_letter_api_notifications_to_pending() + + assert sample_letter_notification.status == NOTIFICATION_PENDING + assert sample_email_notification.status == NOTIFICATION_CREATED + assert sample_notification.status == NOTIFICATION_CREATED + assert ret == [sample_letter_notification] + + +def test_should_only_get_created_letters(sample_letter_template): + created_noti = create_notification(sample_letter_template, status=NOTIFICATION_CREATED) + create_notification(sample_letter_template, status=NOTIFICATION_PENDING) + create_notification(sample_letter_template, status=NOTIFICATION_SENDING) + + ret = dao_set_created_live_letter_api_notifications_to_pending() + + assert ret == [created_noti] + + +def test_should_only_get_api_letters(sample_letter_template, sample_letter_job): + api_noti = create_notification(sample_letter_template) + job_noti = create_notification(sample_letter_template, job=sample_letter_job) + + ret = dao_set_created_live_letter_api_notifications_to_pending() + + assert ret == [api_noti] + + +def test_should_only_get_normal_api_letters(sample_letter_template): + live_noti = create_notification(sample_letter_template, key_type=KEY_TYPE_NORMAL) + test_noti = create_notification(sample_letter_template, key_type=KEY_TYPE_TEST) + + ret = dao_set_created_live_letter_api_notifications_to_pending() + + assert ret == [live_noti] diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 7d191da56..ba8378827 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -40,9 +40,12 @@ from app.dao.notifications_dao import ( dao_delete_notifications_and_history_by_id, dao_timeout_notifications, is_delivery_slow_for_provider, - dao_update_notifications_sent_to_dvla, + dao_update_notifications_for_job_to_sent_to_dvla, dao_get_notifications_by_to_field, - dao_created_scheduled_notification, dao_get_scheduled_notifications, set_scheduled_notification_to_processed) + dao_created_scheduled_notification, + dao_get_scheduled_notifications, + set_scheduled_notification_to_processed +) from app.dao.services_dao import dao_update_service from tests.app.db import create_notification, create_api_key @@ -1713,11 +1716,11 @@ def test_slow_provider_delivery_does_not_return_for_standard_delivery_time( assert not slow_delivery -def test_dao_update_notifications_sent_to_dvla(notify_db, notify_db_session, sample_letter_template): +def test_dao_update_notifications_for_job_to_sent_to_dvla(notify_db, notify_db_session, sample_letter_template): job = sample_job(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_letter_template) notification = create_notification(template=sample_letter_template, job=job) - updated_count = dao_update_notifications_sent_to_dvla(job_id=job.id, provider='some provider') + updated_count = dao_update_notifications_for_job_to_sent_to_dvla(job_id=job.id, provider='some provider') assert updated_count == 1 updated_notification = Notification.query.get(notification.id) @@ -1732,7 +1735,7 @@ def test_dao_update_notifications_sent_to_dvla(notify_db, notify_db_session, sam assert history.updated_at -def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key(sample_letter_job): +def test_dao_update_notifications_for_job_to_sent_to_dvla_does_update_history_if_test_key(sample_letter_job): api_key = create_api_key(sample_letter_job.service, key_type=KEY_TYPE_TEST) notification = create_notification( sample_letter_job.template, @@ -1740,7 +1743,10 @@ def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key(s api_key=api_key ) - updated_count = dao_update_notifications_sent_to_dvla(job_id=sample_letter_job.id, provider='some provider') + updated_count = dao_update_notifications_for_job_to_sent_to_dvla( + job_id=sample_letter_job.id, + provider='some provider' + ) assert updated_count == 1 updated_notification = Notification.query.get(notification.id) diff --git a/tests/app/db.py b/tests/app/db.py index f271d70e9..b3b6ba61c 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,7 +1,7 @@ from datetime import datetime import uuid -from app import db +from app import db, create_random_identifier from app.dao.jobs_dao import dao_create_job from app.dao.service_inbound_api_dao import save_service_inbound_api from app.models import ( @@ -22,6 +22,7 @@ from app.models import ( InboundNumber, Organisation, EMAIL_TYPE, + LETTER_TYPE, SMS_TYPE, INBOUND_SMS_TYPE, KEY_TYPE_NORMAL, @@ -142,6 +143,9 @@ def create_notification( if not api_key: api_key = create_api_key(template.service, key_type=key_type) + if template.template_type == LETTER_TYPE and reference is None: + reference = create_random_identifier() + data = { 'id': uuid.uuid4(), 'to': to_field, diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py index 8d964b706..24f37c06f 100644 --- a/tests/app/notifications/test_process_letter_notifications.py +++ b/tests/app/notifications/test_process_letter_notifications.py @@ -1,53 +1,10 @@ -import pytest - -from app.dao.services_dao import dao_archive_service -from app.models import Job -from app.models import JOB_STATUS_READY_TO_SEND from app.models import LETTER_TYPE from app.models import Notification -from app.notifications.process_letter_notifications import create_letter_api_job +from app.models import NOTIFICATION_CREATED from app.notifications.process_letter_notifications import create_letter_notification -from app.v2.errors import InvalidRequest -from app.variables import LETTER_API_FILENAME - -from tests.app.db import create_service -from tests.app.db import create_template -def test_create_job_rejects_inactive_service(notify_db_session): - service = create_service() - template = create_template(service, template_type=LETTER_TYPE) - dao_archive_service(service.id) - - with pytest.raises(InvalidRequest) as exc_info: - create_letter_api_job(template) - - assert exc_info.value.message == 'Service {} is inactive'.format(service.id) - - -def test_create_job_rejects_archived_template(sample_letter_template): - sample_letter_template.archived = True - - with pytest.raises(InvalidRequest) as exc_info: - create_letter_api_job(sample_letter_template) - - assert exc_info.value.message == 'Template {} is deleted'.format(sample_letter_template.id) - - -def test_create_job_creates_job(sample_letter_template): - job = create_letter_api_job(sample_letter_template) - - assert job == Job.query.one() - assert job.original_file_name == LETTER_API_FILENAME - assert job.service == sample_letter_template.service - assert job.template_id == sample_letter_template.id - assert job.template_version == sample_letter_template.version - assert job.notification_count == 1 - assert job.job_status == JOB_STATUS_READY_TO_SEND - assert job.created_by is None - - -def test_create_letter_notification_creates_notification(sample_letter_job, sample_api_key): +def test_create_letter_notification_creates_notification(sample_letter_template, sample_api_key): data = { 'personalisation': { 'address_line_1': 'The Queen', @@ -56,20 +13,20 @@ def test_create_letter_notification_creates_notification(sample_letter_job, samp } } - notification = create_letter_notification(data, sample_letter_job, sample_api_key) + notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED) assert notification == Notification.query.one() - assert notification.job == sample_letter_job - assert notification.template == sample_letter_job.template + assert notification.job is None + assert notification.status == NOTIFICATION_CREATED + assert notification.template == sample_letter_template assert notification.api_key == sample_api_key assert notification.notification_type == LETTER_TYPE assert notification.key_type == sample_api_key.key_type - assert notification.job_row_number == 0 assert notification.reference is not None assert notification.client_reference is None -def test_create_letter_notification_sets_reference(sample_letter_job, sample_api_key): +def test_create_letter_notification_sets_reference(sample_letter_template, sample_api_key): data = { 'personalisation': { 'address_line_1': 'The Queen', @@ -79,6 +36,6 @@ def test_create_letter_notification_sets_reference(sample_letter_job, sample_api 'reference': 'foo' } - notification = create_letter_notification(data, sample_letter_job, sample_api_key) + notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED) assert notification.client_reference == 'foo' diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index b46f093ca..ccfa39b2f 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -11,12 +11,11 @@ from app.models import KEY_TYPE_TEAM from app.models import KEY_TYPE_TEST from app.models import LETTER_TYPE from app.models import Notification +from app.models import NOTIFICATION_SENDING from app.models import SMS_TYPE from app.schema_validation import validate from app.v2.errors import RateLimitError from app.v2.notifications.notification_schemas import post_letter_response -from app.variables import LETTER_TEST_API_FILENAME -from app.variables import LETTER_API_FILENAME from tests import create_authorization_header from tests.app.db import create_service, create_template @@ -45,7 +44,6 @@ 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): - mocked = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') data = { 'template_id': str(sample_letter_template.id), 'personalisation': { @@ -63,8 +61,7 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo resp_json = letter_request(client, data, service_id=sample_letter_template.service_id) assert validate(resp_json, post_letter_response) == resp_json - job = Job.query.one() - assert job.original_file_name == LETTER_API_FILENAME + assert Job.query.count() == 0 notification = Notification.query.one() notification_id = notification.id assert resp_json['id'] == str(notification_id) @@ -82,8 +79,6 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo ) assert not resp_json['scheduled_for'] - mocked.assert_called_once_with([str(job.id)], queue='job-tasks') - def test_post_letter_notification_returns_400_and_missing_template( client, @@ -220,15 +215,14 @@ def test_post_letter_notification_returns_403_if_not_allowed_to_send_notificatio (True, KEY_TYPE_NORMAL), (False, KEY_TYPE_TEST) ]) -def test_post_letter_notification_doesnt_queue_task( +def test_post_letter_notification_queues_success( client, notify_db_session, mocker, research_mode, key_type ): - real_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') - fake_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') + fake_task = mocker.patch('app.celery.tasks.update_letter_notifications_to_sent_to_dvla.apply_async') service = create_service(research_mode=research_mode, service_permissions=[LETTER_TYPE]) template = create_template(service, template_type=LETTER_TYPE) @@ -240,10 +234,12 @@ def test_post_letter_notification_doesnt_queue_task( letter_request(client, data, service_id=service.id, key_type=key_type) - job = Job.query.one() - assert job.original_file_name == LETTER_TEST_API_FILENAME - assert not real_task.called - fake_task.assert_called_once_with([str(job.id)], queue='research-mode-tasks') + notification = Notification.query.one() + assert notification.status == NOTIFICATION_SENDING + fake_task.assert_called_once_with( + kwargs={'notification_references': [notification.reference]}, + queue='research-mode-tasks' + ) def test_post_letter_notification_doesnt_accept_team_key(client, sample_letter_template): @@ -282,17 +278,23 @@ 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_calls_update_job_sent_to_dvla_when_service_is_in_trial_mode_but_using_test_key( - client, sample_trial_letter_template, mocker): - build_dvla_task = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') - update_job_task = mocker.patch('app.celery.tasks.update_job_to_sent_to_dvla.apply_async') +def test_post_letter_notification_fakes_dvla_when_service_is_in_trial_mode_but_using_test_key( + client, + sample_trial_letter_template, + mocker +): + update_task = mocker.patch('app.celery.tasks.update_letter_notifications_to_sent_to_dvla.apply_async') data = { "template_id": sample_trial_letter_template.id, "personalisation": {'address_line_1': 'Foo', 'address_line_2': 'Bar', 'postcode': 'Baz'} } - letter_request(client, data=data, service_id=sample_trial_letter_template.service_id, - key_type=KEY_TYPE_TEST) - job = Job.query.one() - update_job_task.assert_called_once_with([str(job.id)], queue='research-mode-tasks') - assert not build_dvla_task.called + + 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 + update_task.assert_called_once_with( + kwargs={'notification_references': [notification.reference]}, + queue='research-mode-tasks' + )