From 42447fcb8cc7d7bfc3f70b610f570e5947dc5b26 Mon Sep 17 00:00:00 2001 From: stvnrlly Date: Wed, 7 Dec 2022 22:04:36 -0500 Subject: [PATCH] remove letters from app/celery --- app/celery/reporting_tasks.py | 10 +- app/celery/tasks.py | 19 -- app/dao/returned_letters_dao.py | 39 ---- tests/app/celery/test_tasks.py | 42 ---- tests/app/dao/test_returned_letters_dao.py | 248 --------------------- tests/app/letters/test_returned_letters.py | 42 ---- 6 files changed, 3 insertions(+), 397 deletions(-) delete mode 100644 tests/app/dao/test_returned_letters_dao.py delete mode 100644 tests/app/letters/test_returned_letters.py diff --git a/app/celery/reporting_tasks.py b/app/celery/reporting_tasks.py index b754c4f6c..9c79b6ca0 100644 --- a/app/celery/reporting_tasks.py +++ b/app/celery/reporting_tasks.py @@ -12,7 +12,7 @@ from app.dao.fact_billing_dao import ( ) from app.dao.fact_notification_status_dao import update_fact_notification_status from app.dao.notifications_dao import get_service_ids_with_notifications_on_date -from app.models import EMAIL_TYPE, LETTER_TYPE, SMS_TYPE +from app.models import EMAIL_TYPE, SMS_TYPE @notify_celery.task(name="create-nightly-billing") @@ -72,10 +72,6 @@ def create_nightly_notification_status(): because all outstanding email / SMS are "timed out" after 3 days, and we reject delivery receipts after this point. - - Letter statuses don't change after 9 days. There's no "timeout" for - letters but this is the longest we've had to cope with in the past - due - to major issues with our print provider. - Because the time range of the task exceeds the minimum possible retention period (3 days), we need to choose which table to query for each service. @@ -89,8 +85,8 @@ def create_nightly_notification_status(): yesterday = convert_utc_to_local_timezone(datetime.utcnow()).date() - timedelta(days=1) - for notification_type in [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE]: - days = 10 if notification_type == LETTER_TYPE else 4 + for notification_type in [SMS_TYPE, EMAIL_TYPE]: + days = 4 for i in range(days): process_day = yesterday - timedelta(days=i) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index fca5a49cf..944b1b320 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -14,10 +14,8 @@ from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id from app.dao.jobs_dao import dao_get_job_by_id, dao_update_job from app.dao.notifications_dao import ( dao_get_last_notification_added_for_job_id, - dao_update_notifications_by_reference, get_notification_by_id, ) -from app.dao.returned_letters_dao import insert_or_update_returned_letters from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.service_sms_sender_dao import dao_get_service_sms_senders_by_id @@ -29,7 +27,6 @@ from app.models import ( JOB_STATUS_IN_PROGRESS, JOB_STATUS_PENDING, KEY_TYPE_NORMAL, - NOTIFICATION_RETURNED_LETTER, SMS_TYPE, ) from app.notifications.process_notifications import persist_notification @@ -426,19 +423,3 @@ def process_incomplete_job(job_id): process_row(row, template, job, job.service, sender_id=sender_id) job_complete(job, resumed=True) - - -@notify_celery.task(name='process-returned-letters-list') -def process_returned_letters_list(notification_references): - updated, updated_history = dao_update_notifications_by_reference( - notification_references, - {"status": NOTIFICATION_RETURNED_LETTER} - ) - - insert_or_update_returned_letters(notification_references) - - current_app.logger.info( - "Updated {} letter notifications ({} history notifications, from {} references) to returned-letter".format( - updated, updated_history, len(notification_references) - ) - ) diff --git a/app/dao/returned_letters_dao.py b/app/dao/returned_letters_dao.py index d118aa3c1..e802fd297 100644 --- a/app/dao/returned_letters_dao.py +++ b/app/dao/returned_letters_dao.py @@ -1,10 +1,6 @@ -from datetime import datetime - from sqlalchemy import desc, func -from sqlalchemy.dialects.postgresql import insert from app import db -from app.dao.dao_utils import autocommit from app.models import ( Job, Notification, @@ -16,41 +12,6 @@ from app.models import ( from app.utils import midnight_n_days_ago -def _get_notification_ids_for_references(references): - notification_ids = db.session.query(Notification.id, Notification.service_id).filter( - Notification.reference.in_(references) - ).all() - - notification_history_ids = db.session.query(NotificationHistory.id, NotificationHistory.service_id).filter( - NotificationHistory.reference.in_(references) - ).all() - - return notification_ids + notification_history_ids - - -@autocommit -def insert_or_update_returned_letters(references): - data = _get_notification_ids_for_references(references) - for row in data: - table = ReturnedLetter.__table__ - - stmt = insert(table).values( - reported_at=datetime.utcnow().date(), - service_id=row.service_id, - notification_id=row.id, - created_at=datetime.utcnow() - ) - - stmt = stmt.on_conflict_do_update( - index_elements=[table.c.notification_id], - set_={ - 'reported_at': datetime.utcnow().date(), - 'updated_at': datetime.utcnow() - } - ) - db.session.connection().execute(stmt) - - def fetch_recent_returned_letter_count(service_id): return db.session.query( func.count(ReturnedLetter.notification_id).label('returned_letter_count'), diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index a5965d84c..53f55b2f4 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -23,7 +23,6 @@ from app.celery.tasks import ( process_incomplete_job, process_incomplete_jobs, process_job, - process_returned_letters_list, process_row, s3, save_api_email, @@ -1363,47 +1362,6 @@ def test_process_incomplete_jobs_sets_status_to_in_progress_and_resets_processin assert mock_process_incomplete_job.mock_calls == [call(str(job1.id)), call(str(job2.id))] -@pytest.mark.skip(reason="Needs updating for TTS: Remove mail") -def test_process_returned_letters_list(sample_letter_template): - create_notification(sample_letter_template, reference='ref1') - create_notification(sample_letter_template, reference='ref2') - - process_returned_letters_list(['ref1', 'ref2', 'unknown-ref']) - - notifications = Notification.query.all() - - assert [n.status for n in notifications] == ['returned-letter', 'returned-letter'] - assert all(n.updated_at for n in notifications) - - -@pytest.mark.skip(reason="Needs updating for TTS: Remove mail") -def test_process_returned_letters_list_updates_history_if_notification_is_already_purged( - sample_letter_template -): - create_notification_history(sample_letter_template, reference='ref1') - create_notification_history(sample_letter_template, reference='ref2') - - process_returned_letters_list(['ref1', 'ref2', 'unknown-ref']) - - notifications = NotificationHistory.query.all() - - assert [n.status for n in notifications] == ['returned-letter', 'returned-letter'] - assert all(n.updated_at for n in notifications) - - -@pytest.mark.skip(reason="Needs updating for TTS: Remove mail") -def test_process_returned_letters_populates_returned_letters_table( - sample_letter_template -): - create_notification_history(sample_letter_template, reference='ref1') - create_notification_history(sample_letter_template, reference='ref2') - - process_returned_letters_list(['ref1', 'ref2', 'unknown-ref']) - - returned_letters = ReturnedLetter.query.all() - assert len(returned_letters) == 2 - - @freeze_time('2020-03-25 14:30') @pytest.mark.parametrize('notification_type', ['sms', 'email']) def test_save_api_email_or_sms(mocker, sample_service, notification_type): diff --git a/tests/app/dao/test_returned_letters_dao.py b/tests/app/dao/test_returned_letters_dao.py deleted file mode 100644 index ca3046831..000000000 --- a/tests/app/dao/test_returned_letters_dao.py +++ /dev/null @@ -1,248 +0,0 @@ -import uuid -from datetime import date, datetime, timedelta - -from freezegun import freeze_time - -from app.dao.returned_letters_dao import ( - fetch_most_recent_returned_letter, - fetch_recent_returned_letter_count, - fetch_returned_letter_summary, - fetch_returned_letters, - insert_or_update_returned_letters, -) -from app.models import NOTIFICATION_RETURNED_LETTER, ReturnedLetter -from tests.app.db import ( - create_notification, - create_notification_history, - create_returned_letter, - create_service, -) - - -def test_insert_or_update_returned_letters_inserts(sample_letter_template): - notification = create_notification(template=sample_letter_template, - reference='ref1') - history = create_notification_history(template=sample_letter_template, - reference='ref2') - - assert ReturnedLetter.query.count() == 0 - - insert_or_update_returned_letters(['ref1', 'ref2']) - - returned_letters = ReturnedLetter.query.all() - - assert len(returned_letters) == 2 - returned_letters_ = [x.notification_id for x in returned_letters] - assert notification.id in returned_letters_ - assert history.id in returned_letters_ - - -def test_insert_or_update_returned_letters_updates(sample_letter_template): - notification = create_notification(template=sample_letter_template, - reference='ref1') - history = create_notification_history(template=sample_letter_template, - reference='ref2') - - assert ReturnedLetter.query.count() == 0 - with freeze_time('2019-12-09 13:30'): - insert_or_update_returned_letters(['ref1', 'ref2']) - returned_letters = ReturnedLetter.query.all() - assert len(returned_letters) == 2 - for x in returned_letters: - assert x.reported_at == date(2019, 12, 9) - assert x.created_at == datetime(2019, 12, 9, 13, 30) - assert not x.updated_at - assert x.notification_id in [notification.id, history.id] - - with freeze_time('2019-12-10 14:20'): - insert_or_update_returned_letters(['ref1', 'ref2']) - returned_letters = ReturnedLetter.query.all() - assert len(returned_letters) == 2 - for x in returned_letters: - assert x.reported_at == date(2019, 12, 10) - assert x.created_at == datetime(2019, 12, 9, 13, 30) - assert x.updated_at == datetime(2019, 12, 10, 14, 20) - assert x.notification_id in [notification.id, history.id] - - -def test_insert_or_update_returned_letters_when_no_notification(notify_db_session): - insert_or_update_returned_letters(['ref1']) - assert ReturnedLetter.query.count() == 0 - - -def test_insert_or_update_returned_letters_for_history_only(sample_letter_template): - history_1 = create_notification_history(template=sample_letter_template, - reference='ref1') - history_2 = create_notification_history(template=sample_letter_template, - reference='ref2') - - assert ReturnedLetter.query.count() == 0 - insert_or_update_returned_letters(['ref1', 'ref2']) - returned_letters = ReturnedLetter.query.all() - assert len(returned_letters) == 2 - for x in returned_letters: - assert x.notification_id in [history_1.id, history_2.id] - - -def test_insert_or_update_returned_letters_with_duplicates_in_reference_list(sample_letter_template): - notification_1 = create_notification(template=sample_letter_template, - reference='ref1') - notification_2 = create_notification(template=sample_letter_template, - reference='ref2') - - assert ReturnedLetter.query.count() == 0 - insert_or_update_returned_letters(['ref1', 'ref2', 'ref1', 'ref2']) - returned_letters = ReturnedLetter.query.all() - assert len(returned_letters) == 2 - for x in returned_letters: - assert x.notification_id in [notification_1.id, notification_2.id] - - -def test_get_returned_letter_count(sample_service): - # Before 7 days – don’t count - create_returned_letter( - sample_service, - reported_at=datetime(2001, 1, 1) - ) - create_returned_letter( - sample_service, - reported_at=datetime(2010, 11, 1, 23, 59, 59), - ) - # In the last 7 days – count - create_returned_letter( - sample_service, - reported_at=datetime(2010, 11, 2, 0, 0, 0), - ) - create_returned_letter( - sample_service, - reported_at=datetime(2010, 11, 8, 10, 0), - ) - create_returned_letter( - sample_service, - reported_at=datetime(2010, 11, 8, 10, 0), - ) - # Different service – don’t count - create_returned_letter( - create_service(service_id=uuid.uuid4(), service_name='Other service'), - reported_at=datetime(2010, 11, 8, 10, 0), - ) - - with freeze_time('2010-11-08 10:10'): - result = fetch_recent_returned_letter_count(sample_service.id) - - assert result.returned_letter_count == 3 - - -def test_fetch_most_recent_returned_letter_for_service(sample_service): - # Older - create_returned_letter( - sample_service, - reported_at=datetime(2009, 9, 9, 9, 9), - ) - # Newer - create_returned_letter( - sample_service, - reported_at=datetime(2010, 10, 10, 10, 10), - ) - # Newest, but different service - create_returned_letter( - create_service(service_id=uuid.uuid4(), service_name='Other service'), - reported_at=datetime(2011, 11, 11, 11, 11), - ) - result = fetch_most_recent_returned_letter(sample_service.id) - - assert str(result.reported_at) == '2010-10-10' - - -def test_get_returned_letter_summary(sample_service): - now = datetime.utcnow() - create_returned_letter(sample_service, reported_at=now) - create_returned_letter(sample_service, reported_at=now) - - results = fetch_returned_letter_summary(sample_service.id) - - assert len(results) == 1 - - assert results[0].returned_letter_count == 2 - assert results[0].reported_at == now.date() - - -def test_get_returned_letter_summary_orders_by_reported_at(sample_service): - now = datetime.utcnow() - last_month = datetime.utcnow() - timedelta(days=30) - create_returned_letter(sample_service, reported_at=now) - create_returned_letter(sample_service, reported_at=now) - create_returned_letter(sample_service, reported_at=now) - create_returned_letter(sample_service, reported_at=last_month) - create_returned_letter(sample_service, reported_at=last_month) - create_returned_letter() # returned letter for a different service - - results = fetch_returned_letter_summary(sample_service.id) - - assert len(results) == 2 - assert results[0].reported_at == now.date() - assert results[0].returned_letter_count == 3 - assert results[1].reported_at == last_month.date() - assert results[1].returned_letter_count == 2 - - -def test_fetch_returned_letters_from_notifications_and_notification_history(sample_letter_template): - today = datetime.now() - last_month = datetime.now() - timedelta(days=30) - - letter_1 = create_notification(template=sample_letter_template, client_reference='letter_1', - status=NOTIFICATION_RETURNED_LETTER, - created_at=datetime.utcnow() - timedelta(days=1)) - returned_letter_1 = create_returned_letter(service=sample_letter_template.service, reported_at=today, - notification_id=letter_1.id) - letter_2 = create_notification_history(template=sample_letter_template, client_reference='letter_2', - status=NOTIFICATION_RETURNED_LETTER, created_at=datetime.utcnow()) - returned_letter_2 = create_returned_letter(service=sample_letter_template.service, reported_at=today, - notification_id=letter_2.id) - letter_3 = create_notification_history(template=sample_letter_template, client_reference='letter_3', - status=NOTIFICATION_RETURNED_LETTER) - create_returned_letter(service=sample_letter_template.service, reported_at=last_month, - notification_id=letter_3.id) - - results = fetch_returned_letters(service_id=sample_letter_template.service_id, report_date=today.date()) - - assert len(results) == 2 - assert results[0] == (letter_2.id, returned_letter_2.reported_at, letter_2.client_reference, letter_2.created_at, - sample_letter_template.name, letter_2.template_id, letter_2.template_version, False, None, - None, None, None, None, None) - assert results[1] == (letter_1.id, returned_letter_1.reported_at, letter_1.client_reference, letter_1.created_at, - sample_letter_template.name, letter_1.template_id, letter_1.template_version, False, - letter_1.api_key_id, None, None, None, None, None) - - -def test_fetch_returned_letters_with_jobs(sample_letter_job): - today = datetime.now() - letter_1 = create_notification_history(template=sample_letter_job.template, client_reference='letter_1', - status=NOTIFICATION_RETURNED_LETTER, - job=sample_letter_job, job_row_number=20, - created_at=datetime.utcnow() - timedelta(minutes=1)) - returned_letter_1 = create_returned_letter(service=sample_letter_job.service, reported_at=today, - notification_id=letter_1.id) - - results = fetch_returned_letters(service_id=sample_letter_job.service_id, report_date=today.date()) - assert len(results) == 1 - assert results[0] == (letter_1.id, returned_letter_1.reported_at, letter_1.client_reference, letter_1.created_at, - sample_letter_job.template.name, letter_1.template_id, letter_1.template_version, False, None, - None, None, None, sample_letter_job.original_file_name, 21) - - -def test_fetch_returned_letters_with_create_by_user(sample_letter_template): - today = datetime.now() - letter_1 = create_notification_history(template=sample_letter_template, client_reference='letter_1', - status=NOTIFICATION_RETURNED_LETTER, - created_at=datetime.utcnow() - timedelta(minutes=1), - created_by_id=sample_letter_template.service.users[0].id) - returned_letter_1 = create_returned_letter(service=sample_letter_template.service, reported_at=today, - notification_id=letter_1.id) - - results = fetch_returned_letters(service_id=sample_letter_template.service_id, report_date=today.date()) - assert len(results) == 1 - assert results[0] == (letter_1.id, returned_letter_1.reported_at, letter_1.client_reference, letter_1.created_at, - sample_letter_template.name, letter_1.template_id, letter_1.template_version, False, None, - letter_1.created_by_id, sample_letter_template.service.users[0].name, - sample_letter_template.service.users[0].email_address, None, None) diff --git a/tests/app/letters/test_returned_letters.py b/tests/app/letters/test_returned_letters.py deleted file mode 100644 index cdda37c33..000000000 --- a/tests/app/letters/test_returned_letters.py +++ /dev/null @@ -1,42 +0,0 @@ -import pytest - - -@pytest.mark.parametrize('status, references', [ - (200, ["1234567890ABCDEF", "1234567890ABCDEG"]), - (400, ["1234567890ABCDEFG", "1234567890ABCDEG"]), - (400, ["1234567890ABCDE", "1234567890ABCDEG"]), - (400, ["1234567890ABCDE\u26d4", "1234567890ABCDEG"]), - (400, ["NOTIFY0001234567890ABCDEF", "1234567890ABCDEG"]), -]) -def test_process_returned_letters(status, references, admin_request, mocker): - mock_celery = mocker.patch("app.letters.rest.process_returned_letters_list.apply_async") - - response = admin_request.post( - 'letter-job.create_process_returned_letters_job', - _data={"references": references}, - _expected_status=status - ) - - if status != 200: - assert '{} does not match'.format(references[0]) in response['errors'][0]['message'] - else: - mock_celery.assert_called_once_with(args=(references,), queue='database-tasks', compression='zlib') - - -def test_process_returned_letters_splits_tasks_up(admin_request, mocker): - mock_celery = mocker.patch("app.letters.rest.process_returned_letters_list.apply_async") - mocker.patch("app.letters.rest.MAX_REFERENCES_PER_TASK", 3) - - references = [f'{x:016}' for x in range(10)] - - admin_request.post( - 'letter-job.create_process_returned_letters_job', - _data={"references": references}, - ) - - assert mock_celery.call_count == 4 - - assert mock_celery.call_args_list[0][1]['args'][0] == ['0000000000000000', '0000000000000001', '0000000000000002'] - assert mock_celery.call_args_list[1][1]['args'][0] == ['0000000000000003', '0000000000000004', '0000000000000005'] - assert mock_celery.call_args_list[2][1]['args'][0] == ['0000000000000006', '0000000000000007', '0000000000000008'] - assert mock_celery.call_args_list[3][1]['args'][0] == ['0000000000000009']