diff --git a/app/celery/nightly_tasks.py b/app/celery/nightly_tasks.py index f71f7fb14..b812b2915 100644 --- a/app/celery/nightly_tasks.py +++ b/app/celery/nightly_tasks.py @@ -24,7 +24,7 @@ from app.dao.notifications_dao import ( from app.dao.service_data_retention_dao import ( fetch_service_data_retention_for_all_services_by_notification_type, ) -from app.models import EMAIL_TYPE, LETTER_TYPE, SMS_TYPE, FactProcessingTime +from app.models import EMAIL_TYPE, SMS_TYPE, FactProcessingTime from app.utils import get_local_midnight_in_utc @@ -34,12 +34,6 @@ def remove_sms_email_csv_files(): _remove_csv_files([EMAIL_TYPE, SMS_TYPE]) -@notify_celery.task(name="remove_letter_jobs") -@cronitor("remove_letter_jobs") -def remove_letter_csv_files(): - _remove_csv_files([LETTER_TYPE]) - - def _remove_csv_files(job_types): jobs = dao_get_jobs_older_than_data_retention(notification_types=job_types) for job in jobs: @@ -52,7 +46,6 @@ def _remove_csv_files(job_types): def delete_notifications_older_than_retention(): delete_email_notifications_older_than_retention.apply_async(queue=QueueNames.REPORTING) delete_sms_notifications_older_than_retention.apply_async(queue=QueueNames.REPORTING) - delete_letter_notifications_older_than_retention.apply_async(queue=QueueNames.REPORTING) @notify_celery.task(name="delete-sms-notifications") @@ -67,12 +60,6 @@ def delete_email_notifications_older_than_retention(): _delete_notifications_older_than_retention_by_type('email') -@notify_celery.task(name="delete-letter-notifications") -@cronitor("delete-letter-notifications") -def delete_letter_notifications_older_than_retention(): - _delete_notifications_older_than_retention_by_type('letter') - - def _delete_notifications_older_than_retention_by_type(notification_type): flexible_data_retention = fetch_service_data_retention_for_all_services_by_notification_type(notification_type) diff --git a/app/config.py b/app/config.py index 7baeed5a6..491ec24ec 100644 --- a/app/config.py +++ b/app/config.py @@ -20,10 +20,8 @@ class QueueNames(object): RETRY = 'retry-tasks' NOTIFY = 'notify-internal-tasks' PROCESS_FTP = 'process-ftp-tasks' - CREATE_LETTERS_PDF = 'create-letters-pdf-tasks' CALLBACKS = 'service-callbacks' CALLBACKS_RETRY = 'service-callbacks-retry' - LETTERS = 'letter-tasks' SMS_CALLBACKS = 'sms-callbacks' ANTIVIRUS = 'antivirus-tasks' SAVE_API_EMAIL = 'save-api-email-tasks' @@ -42,10 +40,8 @@ class QueueNames(object): QueueNames.JOBS, QueueNames.RETRY, QueueNames.NOTIFY, - QueueNames.CREATE_LETTERS_PDF, QueueNames.CALLBACKS, QueueNames.CALLBACKS_RETRY, - QueueNames.LETTERS, QueueNames.SMS_CALLBACKS, QueueNames.SAVE_API_EMAIL, QueueNames.SAVE_API_SMS, @@ -270,12 +266,6 @@ class Config(object): 'schedule': crontab(hour=4, minute=0), 'options': {'queue': QueueNames.PERIODIC}, }, - 'remove_letter_jobs': { - 'task': 'remove_letter_jobs', - 'schedule': crontab(hour=4, minute=20), - # since we mark jobs as archived - 'options': {'queue': QueueNames.PERIODIC}, - }, 'check-for-services-with-high-failure-rates-or-sending-to-tv-numbers': { 'task': 'check-for-services-with-high-failure-rates-or-sending-to-tv-numbers', 'schedule': crontab(day_of_week='mon-fri', hour=10, minute=30), diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 87e77a7d0..919eab075 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -361,10 +361,6 @@ def move_notifications_to_notification_history( qry_limit=50000 ): deleted = 0 - if notification_type == LETTER_TYPE: - _delete_letters_from_s3( - notification_type, service_id, timestamp_to_delete_backwards_from, qry_limit - ) delete_count_per_call = 1 while delete_count_per_call > 0: delete_count_per_call = insert_notification_history_delete_notifications( @@ -387,32 +383,6 @@ def move_notifications_to_notification_history( return deleted -def _delete_letters_from_s3( - notification_type, service_id, date_to_delete_from, query_limit -): - letters_to_delete_from_s3 = db.session.query( - Notification - ).filter( - Notification.notification_type == notification_type, - Notification.created_at < date_to_delete_from, - Notification.service_id == service_id, - # although letters in non completed statuses do have PDFs in s3, they do not exist in the - # production-letters-pdf bucket as they never made it that far so we do not try and delete - # them from it - Notification.status.in_(NOTIFICATION_STATUS_TYPES_COMPLETED) - ).limit(query_limit).all() - for letter in letters_to_delete_from_s3: - try: - letter_pdf = find_letter_pdf_in_s3(letter) - letter_pdf.delete() - except ClientError: - current_app.logger.exception( - "Error deleting S3 object for letter: {}".format(letter.id)) - except LetterPDFNotFound: - current_app.logger.warning( - "No S3 object to delete for letter: {}".format(letter.id)) - - @autocommit def dao_delete_notifications_by_id(notification_id): db.session.query(Notification).filter( diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index 070d23b09..6cd9c16f4 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/tests/app/celery/test_nightly_tasks.py @@ -9,9 +9,7 @@ from app.celery.nightly_tasks import ( _delete_notifications_older_than_retention_by_type, delete_email_notifications_older_than_retention, delete_inbound_sms, - delete_letter_notifications_older_than_retention, delete_sms_notifications_older_than_retention, - remove_letter_csv_files, remove_sms_email_csv_files, s3, save_daily_notification_processing_time, @@ -128,8 +126,6 @@ def test_remove_csv_files_filters_by_type(mocker, sample_service): job_to_delete = create_job(template=letter_template, created_at=eight_days_ago) create_job(template=sms_template, created_at=eight_days_ago) - remove_letter_csv_files() - assert s3.remove_job_from_s3.call_args_list == [ call(job_to_delete.service_id, job_to_delete.id), ] @@ -148,12 +144,6 @@ def test_delete_email_notifications_older_than_retentions_calls_child_task(notif mocked_notifications.assert_called_once_with('email') -def test_delete_letter_notifications_older_than_retention_calls_child_task(notify_api, mocker): - mocked = mocker.patch('app.celery.nightly_tasks._delete_notifications_older_than_retention_by_type') - delete_letter_notifications_older_than_retention() - mocked.assert_called_once_with('letter') - - def test_should_not_update_status_of_letter_notifications(client, sample_letter_template): created_at = datetime.utcnow() - timedelta(days=5) not1 = create_notification(template=sample_letter_template, status='sending', created_at=created_at) diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index ca141c445..291d72141 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -156,29 +156,6 @@ def test_move_notifications_deletes_letters_sent_and_in_final_state_from_table_a s3.get_object(Bucket=bucket_name, Key=filename) -@pytest.mark.parametrize('notification_status', ['pending-virus-check', 'created', 'sending']) -@pytest.mark.skip(reason="Skipping letter-related functionality for now") -def test_move_notifications_does_not_delete_letters_not_yet_in_final_state( - sample_service, mocker, notification_status -): - mock_s3_object = mocker.patch("app.dao.notifications_dao.find_letter_pdf_in_s3").return_value - letter_template = create_template(service=sample_service, template_type='letter') - create_notification( - template=letter_template, - status=notification_status, - reference='LETTER_REF', - created_at=datetime.utcnow() - timedelta(days=8), - ) - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 0 - - move_notifications_to_notification_history('letter', sample_service.id, datetime.utcnow()) - - assert Notification.query.count() == 1 - assert NotificationHistory.query.count() == 0 - mock_s3_object.assert_not_called() - - def test_move_notifications_only_moves_notifications_older_than_provided_timestamp(sample_template): delete_time = datetime(2020, 6, 1, 12) one_second_before = delete_time - timedelta(seconds=1) diff --git a/tests/app/test_config.py b/tests/app/test_config.py index 1f77278d1..fe2fef296 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -4,7 +4,7 @@ from app.config import QueueNames 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) == 17 + assert len(queues) == 15 assert set([ QueueNames.PRIORITY, QueueNames.PERIODIC, @@ -16,10 +16,8 @@ def test_queue_names_all_queues_correct(): QueueNames.JOBS, QueueNames.RETRY, QueueNames.NOTIFY, - QueueNames.CREATE_LETTERS_PDF, QueueNames.CALLBACKS, QueueNames.CALLBACKS_RETRY, - QueueNames.LETTERS, QueueNames.SMS_CALLBACKS, QueueNames.SAVE_API_EMAIL, QueueNames.SAVE_API_SMS,