From ebb43082d51b9f27b17190abb0537a710a544408 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 10 Aug 2018 16:22:25 +0100 Subject: [PATCH] Deal with letters that have not been sent, this can be test letters or letters in tech-failure. --- app/dao/notifications_dao.py | 30 ++++++++++--------- ...t_notification_dao_delete_notifications.py | 10 +++++++ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 7ecd86352..465c9379a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -334,7 +334,7 @@ def delete_notifications_created_more_than_a_week_ago_by_type(notification_type) services_with_data_retention)) if notification_type == LETTER_TYPE: _delete_letters_from_s3(query=query) - deleted = query.delete(synchronize_session='fetch') + deleted += query.delete(synchronize_session='fetch') return deleted @@ -342,19 +342,21 @@ def _delete_letters_from_s3(query): letters_to_delete_from_s3 = query.all() for letter in letters_to_delete_from_s3: bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] - sent_at = str(letter.sent_at.date()) - prefix = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format( - folder=sent_at, - reference=letter.reference, - duplex="D", - letter_class="2", - colour="C", - crown="C" if letter.service.crown else "N", - date='' - ).upper()[:-5] - s3_objects = get_s3_object_by_prefix(bucket_name=bucket_name, prefix=prefix) - for s3_object in s3_objects: - s3_object.delete() + # If the letter has not been sent there isn't a letter to delete from S3 + if letter.sent_at: + sent_at = str(letter.sent_at.date()) + prefix = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format( + folder=sent_at, + reference=letter.reference, + duplex="D", + letter_class="2", + colour="C", + crown="C" if letter.service.crown else "N", + date='' + ).upper()[:-5] + s3_objects = get_s3_object_by_prefix(bucket_name=bucket_name, prefix=prefix) + for s3_object in s3_objects: + s3_object.delete() @statsd(namespace="dao") 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 7d56ea35c..703a69024 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 @@ -176,6 +176,16 @@ def test_delete_notifications_delete_notification_type_for_default_time_if_no_da assert len(Notification.query.filter_by(notification_type='email').all()) == 1 +def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_been_sent(sample_service, mocker): + mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_object_by_prefix") + letter_template = create_template(service=sample_service, template_type='letter') + + create_notification(template=letter_template, status='sending', + reference='LETTER_REF') + delete_notifications_created_more_than_a_week_ago_by_type('email') + mock_get_s3.assert_not_called() + + def _create_templates(sample_service): email_template = create_template(service=sample_service, template_type='email') sms_template = create_template(service=sample_service)