From f965322f25e37f8125463223eb5b2473e246e8d2 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Mon, 13 Aug 2018 14:09:51 +0100
Subject: [PATCH] Fixes to the delete letter notifications. If there are no
 files to delete we won't get an excpetion. Wrap the delete file in a
 try/except to avoid stopping the entire task. Fix the missing slash for the
 file name.

---
 app/aws/s3.py                                            | 3 ++-
 app/dao/notifications_dao.py                             | 9 +++++++--
 .../test_notification_dao_delete_notifications.py        | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/app/aws/s3.py b/app/aws/s3.py
index a440745da..a926bb101 100644
--- a/app/aws/s3.py
+++ b/app/aws/s3.py
@@ -61,7 +61,8 @@ def get_s3_bucket_objects(bucket_name, subfolder='', older_than=7, limit_days=2)
 
     all_objects_in_bucket = []
     for page in page_iterator:
-        all_objects_in_bucket.extend(page['Contents'])
+        if page.get('Contents'):
+            all_objects_in_bucket.extend(page['Contents'])
 
     return all_objects_in_bucket
 
diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py
index 89526260a..27d609562 100644
--- a/app/dao/notifications_dao.py
+++ b/app/dao/notifications_dao.py
@@ -6,6 +6,7 @@ from datetime import (
     date
 )
 
+from boto.exception import BotoClientError
 from flask import current_app
 
 from notifications_utils.recipients import (
@@ -345,7 +346,7 @@ def _delete_letters_from_s3(query):
         if letter.sent_at:
             sent_at = str(letter.sent_at.date())
             prefix = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format(
-                folder=sent_at,
+                folder=sent_at + "/",
                 reference=letter.reference,
                 duplex="D",
                 letter_class="2",
@@ -355,7 +356,11 @@ def _delete_letters_from_s3(query):
             ).upper()[:-5]
             s3_objects = get_s3_bucket_objects(bucket_name=bucket_name, subfolder=prefix)
             for s3_object in s3_objects:
-                remove_s3_object(bucket_name, s3_object['Key'])
+                try:
+                    remove_s3_object(bucket_name, s3_object['Key'])
+                except BotoClientError:
+                    current_app.logger.exception(
+                        "Could not delete S3 object with filename: {}".format(s3_object['Key']))
 
 
 @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 55ef4a659..1479921bd 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
@@ -114,7 +114,7 @@ def test_delete_notifications_for_days_of_retention(sample_service, notification
     assert len(Notification.query.filter_by(notification_type=notification_type).all()) == 1
     if notification_type == 'letter':
         mock_get_s3.assert_called_with(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'],
-                                       subfolder="{}NOTIFY.LETTER_REF.D.2.C.C".format(str(datetime.utcnow().date()))
+                                       subfolder="{}/NOTIFY.LETTER_REF.D.2.C.C".format(str(datetime.utcnow().date()))
                                        )
         assert mock_get_s3.call_count == 2
     else: