diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index a985370a3..9d5249e8a 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -9,7 +9,6 @@ from flask import current_app from notifications_utils.statsd_decorators import statsd from sqlalchemy import and_ from sqlalchemy.exc import SQLAlchemyError -from notifications_utils.s3 import s3upload from app.aws import s3 from app import notify_celery @@ -39,7 +38,6 @@ from app.dao.notifications_dao import ( dao_get_count_of_letters_to_process_for_date, dao_get_scheduled_notifications, 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 ( @@ -57,7 +55,6 @@ from app.models import ( ) from app.notifications.process_notifications import send_notification_to_queue from app.celery.tasks import ( - create_dvla_file_contents_for_notifications, process_job ) from app.config import QueueNames, TaskNames @@ -415,37 +412,6 @@ def trigger_letter_pdfs_for_day(): letter_pdfs_count, 'collate-letter-pdfs-for-day')) -@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 - ) - ) - - @notify_celery.task(name='check-job-status') @statsd(namespace="tasks") def check_job_status(): diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 39d0755bc..7ae59feda 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -20,7 +20,6 @@ from requests import ( RequestException ) from sqlalchemy.exc import SQLAlchemyError -from botocore.exceptions import ClientError as BotoClientError from app import ( create_uuid, @@ -36,8 +35,6 @@ from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id from app.dao.jobs_dao import ( dao_update_job, dao_get_job_by_id, - all_notifications_are_created_for_job, - dao_get_all_notifications_for_job, dao_update_job_status ) from app.dao.notifications_dao import ( @@ -60,7 +57,6 @@ from app.models import ( JOB_STATUS_FINISHED, JOB_STATUS_IN_PROGRESS, JOB_STATUS_PENDING, - JOB_STATUS_READY_TO_SEND, JOB_STATUS_SENT_TO_DVLA, JOB_STATUS_ERROR, KEY_TYPE_NORMAL, LETTER_TYPE, @@ -73,7 +69,6 @@ from app.models import ( ) from app.notifications.process_notifications import persist_notification from app.service.utils import service_allowed_to_send_to -from notifications_utils.s3 import s3upload @worker_process_shutdown.connect @@ -335,29 +330,6 @@ def save_letter( handle_exception(self, notification, notification_id, e) -@notify_celery.task(bind=True, name="build-dvla-file", countdown=60, max_retries=15, default_retry_delay=300) -@statsd(namespace="tasks") -def build_dvla_file(self, job_id): - try: - if all_notifications_are_created_for_job(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_BUCKETS']['job'], - file_location="{}-dvla-job.text".format(job_id) - ) - dao_update_job_status(job_id, JOB_STATUS_READY_TO_SEND) - else: - msg = "All notifications for job {} are not persisted".format(job_id) - current_app.logger.info(msg) - self.retry(queue=QueueNames.RETRY) - # specifically don't catch celery.retry errors - except (SQLAlchemyError, BotoClientError): - current_app.logger.exception("build_dvla_file threw exception") - self.retry(queue=QueueNames.RETRY) - - @notify_celery.task(bind=True, name='update-letter-job-to-sent') @statsd(namespace="tasks") def update_job_to_sent_to_dvla(self, job_id): @@ -414,12 +386,6 @@ def update_letter_notifications_to_error(self, notification_references): current_app.logger.debug("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( diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 73c4b0af2..595319c54 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -26,7 +26,6 @@ from app.celery.scheduled_tasks import ( run_scheduled_jobs, run_letter_jobs, trigger_letter_pdfs_for_day, - run_letter_api_notifications, populate_monthly_billing, s3, send_daily_performance_platform_stats, @@ -55,10 +54,7 @@ from app.models import ( JOB_STATUS_READY_TO_SEND, JOB_STATUS_IN_PROGRESS, JOB_STATUS_SENT_TO_DVLA, - KEY_TYPE_TEST, LETTER_TYPE, - NOTIFICATION_CREATED, - NOTIFICATION_PENDING, SMS_TYPE ) from app.utils import get_london_midnight_in_utc @@ -845,63 +841,6 @@ def test_run_letter_jobs_does_nothing_if_no_ready_jobs(client, mocker, sample_le 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( - mocker, - sample_letter_template, - sample_letter_job, - sample_api_key -): - letter_job_notification = create_notification( - sample_letter_template, - job=sample_letter_job - ) - 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 - - def test_check_job_status_task_raises_job_incomplete_error(mocker, sample_template): mock_celery = mocker.patch('app.celery.tasks.notify_celery.send_task') job = create_job(template=sample_template, notification_count=3, diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 6ad7f20fc..0ff66a4f7 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -5,12 +5,10 @@ from unittest.mock import Mock import pytest import requests_mock -from flask import current_app from freezegun import freeze_time from requests import RequestException from sqlalchemy.exc import SQLAlchemyError from celery.exceptions import Retry -from botocore.exceptions import ClientError from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, LetterDVLATemplate from app import (encryption, DATETIME_FORMAT) @@ -18,8 +16,6 @@ from app.celery import provider_tasks from app.celery import tasks from app.celery.scheduled_tasks import check_job_status from app.celery.tasks import ( - build_dvla_file, - create_dvla_file_contents_for_job, process_job, process_row, save_sms, @@ -43,7 +39,6 @@ from app.models import ( JOB_STATUS_FINISHED, JOB_STATUS_IN_PROGRESS, LETTER_TYPE, - SERVICE_PERMISSION_TYPES, SMS_TYPE ) @@ -1214,87 +1209,6 @@ def test_get_template_class(template_type, expected_class): assert get_template_class(template_type) == expected_class -def test_build_dvla_file(sample_letter_template, mocker): - job = create_job(template=sample_letter_template, notification_count=2) - create_notification(template=job.template, job=job) - create_notification(template=job.template, job=job) - mocked_upload = mocker.patch("app.celery.tasks.s3upload") - mocker.patch("app.celery.tasks.notify_celery.send_task") - mocker.patch("app.celery.tasks.LetterDVLATemplate", return_value='dvla|string') - build_dvla_file(job.id) - - mocked_upload.assert_called_once_with( - filedata="dvla|string\ndvla|string\n", - region=current_app.config['AWS_REGION'], - 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' - - -def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_letter_template, mocker): - job = create_job(template=sample_letter_template, notification_count=2, job_status='in progress') - create_notification(template=job.template, job=job) - - mocked = mocker.patch("app.celery.tasks.s3upload") - mocked_send_task = mocker.patch("app.celery.tasks.notify_celery.send_task") - mocker.patch('app.celery.tasks.build_dvla_file.retry', side_effect=Retry) - with pytest.raises(Retry): - build_dvla_file(job.id) - mocked.assert_not_called() - - tasks.build_dvla_file.retry.assert_called_with(queue="retry-tasks") - assert Job.query.get(job.id).job_status == 'in progress' - mocked_send_task.assert_not_called() - - -def test_build_dvla_file_retries_if_s3_err(sample_letter_template, mocker): - job = create_job(sample_letter_template, notification_count=1) - create_notification(job.template, job=job) - error_response = { - 'Error': { - 'Code': 'InvalidParameterValue', - 'Message': 'some error message from amazon', - 'Type': 'Sender' - } - } - mocker.patch('app.celery.tasks.LetterDVLATemplate', return_value='dvla|string') - mocker.patch('app.celery.tasks.s3upload', side_effect=ClientError(error_response, 'operation_name')) - retry_mock = mocker.patch('app.celery.tasks.build_dvla_file.retry', side_effect=Retry) - - with pytest.raises(Retry): - build_dvla_file(job.id) - - retry_mock.assert_called_once_with(queue='retry-tasks') - - -def test_create_dvla_file_contents(notify_db_session, mocker): - service = create_service(service_permissions=SERVICE_PERMISSION_TYPES) - letter_template = create_template(service=service, template_type=LETTER_TYPE) - job = create_job(template=letter_template, notification_count=2) - create_notification(template=job.template, job=job, reference=1, reply_to_text='London,\nNW1A 1AA') - create_notification(template=job.template, job=job, reference=2, reply_to_text='Not the default address') - mocked_letter_template = mocker.patch("app.celery.tasks.LetterDVLATemplate") - mocked_letter_template_instance = mocked_letter_template.return_value - mocked_letter_template_instance.__str__.return_value = "dvla|string" - - create_dvla_file_contents_for_job(job.id) - calls = mocked_letter_template.call_args_list - # Template - assert calls[0][0][0]['subject'] == 'Template subject' - assert calls[0][0][0]['content'] == 'Dear Sir/Madam, Hello. Yours Truly, The Government.' - - # Personalisation - assert not calls[0][0][1] - assert not calls[1][0][1] - # Named arguments - assert calls[0][1]['contact_block'] == 'London,\nNW1A 1AA' - assert calls[1][1]['contact_block'] == 'Not the default address' - assert calls[0][1]['notification_reference'] == '1' - assert calls[1][1]['notification_reference'] == '2' - assert calls[1][1]['org_id'] == '001' - - @freeze_time("2017-03-23 11:09:00.061258") def test_dvla_letter_template(sample_letter_notification): t = {"content": sample_letter_notification.template.content,