From bffc4863dbf4799c3701cdd2ba7a18f8a15c9341 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 28 Feb 2018 11:42:23 +0000 Subject: [PATCH] Remove all methods no longer used now that we only send pdf files to DVLA. --- app/celery/tasks.py | 15 ----- app/config.py | 1 - app/dao/jobs_dao.py | 49 +++++++------- app/dao/notifications_dao.py | 37 ---------- tests/app/celery/test_scheduled_tasks.py | 5 -- tests/app/conftest.py | 2 +- .../test_letter_api_notification_dao.py | 67 ------------------- tests/app/dao/test_jobs_dao.py | 29 -------- 8 files changed, 24 insertions(+), 181 deletions(-) delete mode 100644 tests/app/dao/notification_dao/test_letter_api_notification_dao.py diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 7ae59feda..cbfe21787 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -12,7 +12,6 @@ from notifications_utils.statsd_decorators import statsd from notifications_utils.template import ( SMSMessageTemplate, WithSubjectTemplate, - LetterDVLATemplate ) from requests import ( HTTPError, @@ -386,20 +385,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_notifications(notifications): - file_contents = '\n'.join( - str(LetterDVLATemplate( - notification.template.__dict__, - notification.personalisation, - notification_reference=notification.reference, - contact_block=notification.reply_to_text, - org_id=notification.service.dvla_organisation.id, - )) - for notification in notifications - ) - return file_contents - - def handle_exception(task, notification, notification_id, exc): if not get_notification_by_id(notification_id): retry_msg = '{task} notification for job {job} row number {row} and notification id {noti}'.format( diff --git a/app/config.py b/app/config.py index 65dee5bc3..7b2a5a75e 100644 --- a/app/config.py +++ b/app/config.py @@ -54,7 +54,6 @@ class QueueNames(object): class TaskNames(object): DVLA_JOBS = 'send-jobs-to-dvla' - DVLA_NOTIFICATIONS = 'send-api-notifications-to-dvla' PROCESS_INCOMPLETE_JOBS = 'process-incomplete-jobs' ZIP_AND_SEND_LETTER_PDFS = 'zip-and-send-letter-pdfs' diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 6eb2f449e..73a9a23e3 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -3,14 +3,24 @@ from datetime import datetime, timedelta from flask import current_app from notifications_utils.statsd_decorators import statsd -from sqlalchemy import func, desc, asc, cast, Date as sql_date +from sqlalchemy import ( + Date as sql_date, + asc, + cast, + desc, + func, +) from app import db from app.dao import days_ago from app.models import ( - Job, JobStatistics, Notification, NotificationHistory, Template, - JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, - LETTER_TYPE + Job, + JobStatistics, + JOB_STATUS_PENDING, + JOB_STATUS_SCHEDULED, + LETTER_TYPE, + NotificationHistory, + Template, ) from app.variables import LETTER_TEST_API_FILENAME @@ -22,28 +32,15 @@ def dao_get_notification_outcomes_for_job(service_id, job_id): NotificationHistory.status ) - return query \ - .filter(NotificationHistory.service_id == service_id) \ - .filter(NotificationHistory.job_id == job_id)\ - .group_by(NotificationHistory.status) \ - .order_by(asc(NotificationHistory.status)) \ - .all() - - -@statsd(namespace="dao") -def all_notifications_are_created_for_job(job_id): - query = db.session.query(func.count(Notification.id), Job.id)\ - .join(Job)\ - .filter(Job.id == job_id)\ - .group_by(Job.id)\ - .having(func.count(Notification.id) == Job.notification_count).all() - - return query - - -@statsd(namespace="dao") -def dao_get_all_notifications_for_job(job_id): - return db.session.query(Notification).filter(Notification.job_id == job_id).order_by(Notification.created_at).all() + return query.filter( + NotificationHistory.service_id == service_id + ).filter( + NotificationHistory.job_id == job_id + ).group_by( + NotificationHistory.status + ).order_by( + asc(NotificationHistory.status) + ).all() def dao_get_job_by_service_id_and_job_id(service_id, job_id): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 494e31f17..fdb2d5358 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -28,7 +28,6 @@ from app.models import ( NotificationHistory, ScheduledNotification, Service, - ServicePermission, Template, TemplateHistory, KEY_TYPE_NORMAL, @@ -547,42 +546,6 @@ def dao_get_total_notifications_sent_per_day_for_performance_platform(start_date ).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. - - Note - do not process services that have letters_as_pdf permission as they - will get processed when the letters PDF zip task is created - """ - notifications = db.session.query( - Notification - ).join( - Service - ).filter( - Notification.notification_type == LETTER_TYPE, - Notification.status == NOTIFICATION_CREATED, - Notification.key_type == KEY_TYPE_NORMAL, - Notification.api_key != None, # noqa - # Ignore services that have letters_as_pdf permission - ~Service.permissions.any( - ServicePermission.permission == 'letters_as_pdf' - ) - ).with_for_update( - ).all() - - for notification in notifications: - notification.status = NOTIFICATION_PENDING - - db.session.add_all(notifications) - db.session.commit() - - return notifications - - @statsd(namespace="dao") def dao_get_last_notification_added_for_job_id(job_id): last_notification_added = Notification.query.filter( diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 595319c54..6c2094cf8 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -45,7 +45,6 @@ from app.dao.provider_details_dao import ( dao_update_provider_details, get_current_provider ) -from app.dao.service_permissions_dao import dao_add_service_permission from app.models import ( MonthlyBilling, NotificationHistory, @@ -803,8 +802,6 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): @freeze_time("2017-12-18 17:50") def test_trigger_letter_pdfs_for_day(client, mocker, sample_letter_template): - dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') - create_notification(template=sample_letter_template, created_at='2017-12-17 17:30:00') create_notification(template=sample_letter_template, created_at='2017-12-18 17:29:59') @@ -820,8 +817,6 @@ def test_trigger_letter_pdfs_for_day(client, mocker, sample_letter_template): @freeze_time("2017-12-18 17:50") def test_trigger_letter_pdfs_for_day_send_task_not_called_if_no_notis_for_day( client, mocker, sample_letter_template): - dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') - create_notification(template=sample_letter_template, created_at='2017-12-15 17:30:00') mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") diff --git a/tests/app/conftest.py b/tests/app/conftest.py index f096b04b7..1d52c0db6 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -198,7 +198,7 @@ def sample_service_full_permissions(notify_db, notify_db_session): notify_db_session, # ensure name doesn't clash with regular sample service service_name="sample service full permissions", - permissions=set(SERVICE_PERMISSION_TYPES) - {'letters_as_pdf'} + permissions=set(SERVICE_PERMISSION_TYPES) ) 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 deleted file mode 100644 index c52de01e1..000000000 --- a/tests/app/dao/notification_dao/test_letter_api_notification_dao.py +++ /dev/null @@ -1,67 +0,0 @@ -from app.models import ( - Notification, - NOTIFICATION_CREATED, - NOTIFICATION_PENDING, - NOTIFICATION_SENDING, - KEY_TYPE_TEST, - KEY_TYPE_NORMAL, - LETTER_TYPE -) -from app.dao.notifications_dao import dao_set_created_live_letter_api_notifications_to_pending - -from tests.app.db import create_notification, create_service, create_template - - -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_ignore_letters_as_pdf(sample_letter_notification): - service = create_service(service_permissions=[LETTER_TYPE, 'letters_as_pdf']) - template = create_template(service, template_type=LETTER_TYPE) - create_notification(template) - - all_noti = Notification.query.all() - assert len(all_noti) == 2 - - ret = dao_set_created_live_letter_api_notifications_to_pending() - - assert sample_letter_notification.status == NOTIFICATION_PENDING - 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) - 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) - 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/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 16899adb1..285983174 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -13,9 +13,7 @@ from app.dao.jobs_dao import ( dao_set_scheduled_jobs_to_pending, dao_get_future_scheduled_job_by_id_and_service_id, dao_get_notification_outcomes_for_job, - all_notifications_are_created_for_job, dao_update_job_status, - dao_get_all_notifications_for_job, dao_get_jobs_older_than_limited_by, dao_get_job_statistics_for_job, dao_get_job_stats_for_service, @@ -372,33 +370,6 @@ def test_get_jobs_for_service_doesnt_return_test_messages( assert jobs == [sample_job] -def test_all_notifications_are_created_for_job_returns_true(notify_db, notify_db_session): - job = create_job(notify_db=notify_db, notify_db_session=notify_db_session, notification_count=2) - create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=job) - create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=job) - job_is_complete = all_notifications_are_created_for_job(job.id) - assert job_is_complete - - -def test_all_notifications_are_created_for_job_returns_false(notify_db, notify_db_session): - job = create_job(notify_db=notify_db, notify_db_session=notify_db_session, notification_count=2) - job_is_complete = all_notifications_are_created_for_job(job.id) - assert not job_is_complete - - -def test_are_all_notifications_created_for_job_returns_false_when_job_does_not_exist(): - job_is_complete = all_notifications_are_created_for_job(uuid.uuid4()) - assert not job_is_complete - - -def test_dao_get_all_notifications_for_job(notify_db, notify_db_session, sample_job): - create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=sample_job) - create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=sample_job) - create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=sample_job) - - assert len(dao_get_all_notifications_for_job(sample_job.id)) == 3 - - def test_dao_update_job_status(sample_job): dao_update_job_status(sample_job.id, 'sent to dvla') updated_job = Job.query.get(sample_job.id)