From 441651bbd1f4482d5a7394f142ca12181f0a9652 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 18 Dec 2017 16:12:17 +0000 Subject: [PATCH 1/5] Add get_count_of_letters_to_process to notifications_dao - will get the letter notifications from day before >= letter processing deadline (17:30) - letters_as_pdf permission is required in the service --- app/config.py | 11 ++- app/dao/notifications_dao.py | 31 +++++++ .../notification_dao/test_notification_dao.py | 89 +++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 0c4b477c6..f92355352 100644 --- a/app/config.py +++ b/app/config.py @@ -1,4 +1,4 @@ -from datetime import timedelta +from datetime import timedelta, time import os import json @@ -32,6 +32,7 @@ class QueueNames(object): PROCESS_FTP = 'process-ftp-tasks' CREATE_LETTERS_PDF = 'create-letters-pdf-tasks' CALLBACKS = 'service-callbacks' + LETTERS = 'letter-tasks' @staticmethod def all_queues(): @@ -48,6 +49,7 @@ class QueueNames(object): QueueNames.NOTIFY, QueueNames.CREATE_LETTERS_PDF, QueueNames.CALLBACKS, + QueueNames.LETTERS, ] @@ -238,6 +240,11 @@ class Config(object): 'schedule': crontab(hour=17, minute=30), 'options': {'queue': QueueNames.PERIODIC} }, + 'run-letter-pdfs': { + 'task': 'run-letter-pdfs', + 'schedule': crontab(hour=17, minute=50), + 'options': {'queue': QueueNames.PERIODIC} + }, 'run-letter-api-notifications': { 'task': 'run-letter-api-notifications', 'schedule': crontab(hour=17, minute=40), @@ -313,6 +320,8 @@ class Config(object): TEMPLATE_PREVIEW_API_HOST = os.environ.get('TEMPLATE_PREVIEW_API_HOST', 'http://localhost:6013') TEMPLATE_PREVIEW_API_KEY = os.environ.get('TEMPLATE_PREVIEW_API_KEY', 'my-secret-key') + LETTER_PROCESSING_DEADLINE = time(17, 30) + ###################### # Config overrides ### diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 29b49ba7c..fc6d2b32e 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -584,3 +584,34 @@ def dao_get_last_notification_added_for_job_id(job_id): ).first() return last_notification_added + + +def dao_get_count_of_letters_to_process_for_date(date_to_process=date.today()): + """ + Returns a count of letter notifications for services with letters_as_pdf permission set + to be processed today if no argument passed in otherwise will return the count for + the date passed in. + Records processed today = yesterday 17:30 to today 17:29:59 + + Note - services without letters_as_pdf permission will be ignored + """ + day_before = date_to_process - timedelta(days=1) + letter_deadline_time = current_app.config.get('LETTER_PROCESSING_DEADLINE') + + start_datetime = datetime.combine(day_before, letter_deadline_time) + end_datetime = start_datetime + timedelta(days=1) + + count_of_letters_to_process_for_date = Notification.query.join( + Service + ).filter( + Notification.created_at >= start_datetime, + Notification.created_at < end_datetime, + Notification.notification_type == LETTER_TYPE, + Notification.status == NOTIFICATION_CREATED, + Notification.key_type != KEY_TYPE_TEST, + Service.permissions.any( + ServicePermission.permission == 'letters_as_pdf' + ) + ).count() + + return count_of_letters_to_process_for_date diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 8af596199..7d5083307 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -10,6 +10,7 @@ from app.dao.notifications_dao import ( dao_create_notification, dao_created_scheduled_notification, dao_delete_notifications_and_history_by_id, + dao_get_count_of_letters_to_process_for_date, dao_get_last_notification_added_for_job_id, dao_get_last_template_usage, dao_get_notifications_by_to_field, @@ -33,6 +34,7 @@ from app.dao.notifications_dao import ( dao_get_notifications_by_references ) from app.dao.services_dao import dao_update_service +from app.dao.service_permissions_dao import dao_add_service_permission from app.models import ( Job, Notification, @@ -2001,3 +2003,90 @@ def test_dao_get_notifications_by_reference(sample_template): assert len(notifications) == 2 assert notifications[0].id in [notification_1.id, notification_2.id] assert notifications[1].id in [notification_1.id, notification_2.id] + + +@freeze_time("2017-12-18 17:50") +def test_dao_get_count_of_letters_to_process_for_today(sample_letter_template): + dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') + + # expected + 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') + + # not expected + create_notification(template=sample_letter_template, created_at='2017-12-17 17:29:59') + create_notification(template=sample_letter_template, created_at='2017-12-18 17:30:00') + + count_for_date = dao_get_count_of_letters_to_process_for_date() + + assert count_for_date == 2 + + +@freeze_time("2017-12-18 17:50") +def test_dao_get_count_of_letters_to_process_for_date_in_past(sample_letter_template): + dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') + + # expected + create_notification(template=sample_letter_template, created_at='2017-12-15 17:29:59') + + # not expected + create_notification(template=sample_letter_template, created_at='2017-12-15 17:30:00') + create_notification(template=sample_letter_template, created_at='2017-12-18 17:29:00') + + count_for_date = dao_get_count_of_letters_to_process_for_date(date(2017, 12, 15)) + + assert count_for_date == 1 + + +@freeze_time("2017-12-18 17:50") +def test_dao_get_count_of_letters_to_process_for_date_in_future_does_not_raise_error(sample_letter_template): + dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') + + # not expected + create_notification(template=sample_letter_template, created_at='2017-12-18 17:30:00') + create_notification(template=sample_letter_template, created_at='2017-12-19 17:29:59') + + count_for_date = dao_get_count_of_letters_to_process_for_date(date(2017, 12, 20)) + + assert count_for_date == 0 + + +def test_dao_get_count_of_letters_to_process_for_today_without_notis_does_not_raise_error(sample_letter_template): + count_for_date = dao_get_count_of_letters_to_process_for_date() + + assert count_for_date == 0 + + +@freeze_time("2017-12-18 17:50") +def test_dao_get_count_of_letters_to_process_for_date_ignores_service_not_letters_as_pdf( + sample_letter_template, sample_template): + # not expected + create_notification(template=sample_letter_template, created_at='2017-12-18 17:29:00') + + dao_add_service_permission(sample_template.service.id, 'letters_as_pdf') + sample_template.template_type = 'letter' + + # expected + create_notification(template=sample_template, created_at='2017-12-18 17:29:00') + create_notification(template=sample_template, created_at='2017-12-18 17:29:10') + + count_for_date = dao_get_count_of_letters_to_process_for_date() + + assert 'letters_as_pdf' not in [p.permission for p in sample_letter_template.service.permissions] + assert count_for_date == 2 + + +@freeze_time("2017-12-18 17:50") +def test_dao_get_count_of_letters_to_process_for_date_ignores_test_keys(sample_letter_template): + dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') + + # not expected + create_notification(template=sample_letter_template, key_type=KEY_TYPE_TEST, created_at='2017-12-18 17:29:00') + + # expected + create_notification(template=sample_letter_template, created_at='2017-12-18 17:29:10') + create_notification(template=sample_letter_template, created_at='2017-12-18 17:29:20') + + count_for_date = dao_get_count_of_letters_to_process_for_date() + + assert count_for_date == 2 From 88f3c17463247fe4d06edb1abdc07346880c7662 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 18 Dec 2017 16:19:27 +0000 Subject: [PATCH 2/5] Trigger collate-letter-pdfs-for-day task if notis to process --- app/celery/scheduled_tasks.py | 16 ++++++++++++ tests/app/celery/test_scheduled_tasks.py | 31 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 7b28b0fb4..c773fe739 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -1,4 +1,5 @@ from datetime import ( + date, datetime, timedelta ) @@ -33,6 +34,7 @@ from app.dao.notifications_dao import ( dao_timeout_notifications, is_delivery_slow_for_provider, delete_notifications_created_more_than_a_week_ago_by_type, + 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, @@ -355,6 +357,20 @@ def run_letter_jobs(): current_app.logger.info("Queued {} ready letter job ids onto {}".format(len(job_ids), QueueNames.PROCESS_FTP)) +@notify_celery.task(name="run-letter-pdfs") +@statsd(namespace="tasks") +def run_letter_pdfs(): + letter_pdfs_count = dao_get_count_of_letters_to_process_for_date() + if letter_pdfs_count: + notify_celery.send_task( + name='collate-letter-pdfs-for-day', + args=(date.today().strftime("%Y-%m-%d"),), + queue=QueueNames.LETTERS + ) + current_app.logger.info("{} letter pdfs to be process by {} task".format( + letter_pdfs_count, 'collate-letter-pdfs-for-day')) + + @notify_celery.task(name="run-letter-api-notifications") @statsd(namespace="tasks") def run_letter_api_notifications(): diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index bf1e242f3..33909e4c4 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -24,6 +24,7 @@ from app.celery.scheduled_tasks import ( remove_transformed_dvla_files, run_scheduled_jobs, run_letter_jobs, + run_letter_pdfs, run_letter_api_notifications, populate_monthly_billing, s3, @@ -43,6 +44,7 @@ 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, @@ -713,6 +715,35 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): queue=QueueNames.PROCESS_FTP) +@freeze_time("2017-12-18 17:50") +def test_run_letter_pdfs(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') + + mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") + + run_letter_pdfs() + + mock_celery.assert_called_once_with(name='collate-letter-pdfs-for-day', + args=('2017-12-18',), + queue=QueueNames.LETTERS) + + +@freeze_time("2017-12-18 17:50") +def test_run_letter_pdfs_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") + + run_letter_pdfs() + + assert not mock_celery.called + + def test_run_letter_jobs_does_nothing_if_no_ready_jobs(client, mocker, sample_letter_template): create_job(sample_letter_template, job_status=JOB_STATUS_IN_PROGRESS) create_job(sample_letter_template, job_status=JOB_STATUS_SENT_TO_DVLA) From 3a4b8673bc654b18a507171bd011ba6840ef64fa Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 18 Dec 2017 16:20:57 +0000 Subject: [PATCH 3/5] Update upload_letters_pdf to use config setting for letter processing deadline --- app/aws/s3.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index 5380544c8..5ffb24068 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -1,4 +1,4 @@ -from datetime import datetime, timedelta, time +from datetime import datetime, timedelta from flask import current_app @@ -85,7 +85,7 @@ def upload_letters_pdf(reference, crown, filedata): now = datetime.utcnow() print_datetime = now - if now.time() > time(17, 30): + if now.time() > current_app.config.get('LETTER_PROCESSING_DEADLINE'): print_datetime = now + timedelta(days=1) upload_file_name = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format( From b70bf1b541dc7b93db5637da620ec8236985524b Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 18 Dec 2017 16:22:22 +0000 Subject: [PATCH 4/5] Add letter tasks to manifest and queue name to config test --- manifest-delivery-base.yml | 2 +- tests/app/test_config.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 8838ad40b..26a12ea0e 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -51,7 +51,7 @@ applications: NOTIFY_APP_NAME: delivery-worker-priority - name: notify-delivery-worker - command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q job-tasks,retry-tasks,notify-internal-tasks,create-letters-pdf-tasks + command: scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 -Q job-tasks,retry-tasks,notify-internal-tasks,create-letters-pdf-tasks,letter-tasks env: NOTIFY_APP_NAME: delivery-worker diff --git a/tests/app/test_config.py b/tests/app/test_config.py index 2f643ba5c..4e17390de 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -63,7 +63,7 @@ def test_cloudfoundry_config_has_different_defaults(): def test_queue_names_all_queues_correct(): # Need to ensure that all_queues() only returns queue names used in API queues = QueueNames.all_queues() - assert len(queues) == 12 + assert len(queues) == 13 assert set([ QueueNames.PRIORITY, QueueNames.PERIODIC, @@ -76,5 +76,6 @@ def test_queue_names_all_queues_correct(): QueueNames.RETRY, QueueNames.NOTIFY, QueueNames.CREATE_LETTERS_PDF, - QueueNames.CALLBACKS + QueueNames.CALLBACKS, + QueueNames.LETTERS, ]) == set(queues) From 8103540261205654e1174ee4904d15c946bfee02 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 19 Dec 2017 13:18:56 +0000 Subject: [PATCH 5/5] Renamed run-letter-pdfs to trigger-letter-pdfs-for-day - also set optional date_to_process argument for dao_get_count_of_letters_to_process_for_date to None, so it's set in the code instead --- app/celery/scheduled_tasks.py | 4 ++-- app/config.py | 4 ++-- app/dao/notifications_dao.py | 5 ++++- tests/app/celery/test_scheduled_tasks.py | 11 ++++++----- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index c773fe739..5bac860a6 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -357,9 +357,9 @@ def run_letter_jobs(): current_app.logger.info("Queued {} ready letter job ids onto {}".format(len(job_ids), QueueNames.PROCESS_FTP)) -@notify_celery.task(name="run-letter-pdfs") +@notify_celery.task(name="trigger-letter-pdfs-for-day") @statsd(namespace="tasks") -def run_letter_pdfs(): +def trigger_letter_pdfs_for_day(): letter_pdfs_count = dao_get_count_of_letters_to_process_for_date() if letter_pdfs_count: notify_celery.send_task( diff --git a/app/config.py b/app/config.py index f92355352..295adab1c 100644 --- a/app/config.py +++ b/app/config.py @@ -240,8 +240,8 @@ class Config(object): 'schedule': crontab(hour=17, minute=30), 'options': {'queue': QueueNames.PERIODIC} }, - 'run-letter-pdfs': { - 'task': 'run-letter-pdfs', + 'trigger-letter-pdfs-for-day': { + 'task': 'trigger-letter-pdfs-for-day', 'schedule': crontab(hour=17, minute=50), 'options': {'queue': QueueNames.PERIODIC} }, diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index fc6d2b32e..0d1c6e72a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -586,7 +586,7 @@ def dao_get_last_notification_added_for_job_id(job_id): return last_notification_added -def dao_get_count_of_letters_to_process_for_date(date_to_process=date.today()): +def dao_get_count_of_letters_to_process_for_date(date_to_process=None): """ Returns a count of letter notifications for services with letters_as_pdf permission set to be processed today if no argument passed in otherwise will return the count for @@ -595,6 +595,9 @@ def dao_get_count_of_letters_to_process_for_date(date_to_process=date.today()): Note - services without letters_as_pdf permission will be ignored """ + if date_to_process is None: + date_to_process = date.today() + day_before = date_to_process - timedelta(days=1) letter_deadline_time = current_app.config.get('LETTER_PROCESSING_DEADLINE') diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 33909e4c4..699f1fa9e 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -24,7 +24,7 @@ from app.celery.scheduled_tasks import ( remove_transformed_dvla_files, run_scheduled_jobs, run_letter_jobs, - run_letter_pdfs, + trigger_letter_pdfs_for_day, run_letter_api_notifications, populate_monthly_billing, s3, @@ -716,7 +716,7 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): @freeze_time("2017-12-18 17:50") -def test_run_letter_pdfs(client, mocker, sample_letter_template): +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') @@ -724,7 +724,7 @@ def test_run_letter_pdfs(client, mocker, sample_letter_template): mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") - run_letter_pdfs() + trigger_letter_pdfs_for_day() mock_celery.assert_called_once_with(name='collate-letter-pdfs-for-day', args=('2017-12-18',), @@ -732,14 +732,15 @@ def test_run_letter_pdfs(client, mocker, sample_letter_template): @freeze_time("2017-12-18 17:50") -def test_run_letter_pdfs_send_task_not_called_if_no_notis_for_day(client, mocker, sample_letter_template): +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") - run_letter_pdfs() + trigger_letter_pdfs_for_day() assert not mock_celery.called