Remove the list from S3 once we don’t need it

Once a contact list is gone from the database there’s no way to
reference it again. Any jobs have made their own copy.

So we can clean it up, meaning we’re not storing personal data longer
than we need to.

app/aws/s3.py

@@ -43,6 +43,13 @@ def get_job_location(service_id, job_id):
     )
 
 
+def get_contact_list_location(service_id, contact_list_id):
+    return (
+        current_app.config['CONTACT_LIST_BUCKET_NAME'],
+        FILE_LOCATION_STRUCTURE.format(service_id, contact_list_id),
+    )
+
+
 def get_job_and_metadata_from_s3(service_id, job_id):
     obj = get_s3_object(*get_job_location(service_id, job_id))
     return obj.get()['Body'].read().decode('utf-8'), obj.get()['Metadata']
@@ -62,6 +69,10 @@ def remove_job_from_s3(service_id, job_id):
     return remove_s3_object(*get_job_location(service_id, job_id))
 
 
+def remove_contact_list_from_s3(service_id, contact_list_id):
+    return remove_s3_object(*get_contact_list_location(service_id, contact_list_id))
+
+
 def get_s3_bucket_objects(bucket_name, subfolder=''):
     boto_client = client('s3', current_app.config['AWS_REGION'])
     paginator = boto_client.get_paginator('list_objects_v2')

app/config.py

@@ -364,6 +364,7 @@ class Development(Config):
     SQLALCHEMY_ECHO = False
 
     CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload'
+    CONTACT_LIST_BUCKET_NAME = 'development-contact-list'
     TEST_LETTERS_BUCKET_NAME = 'development-test-letters'
     DVLA_RESPONSE_BUCKET_NAME = 'notify.tools-ftp'
     LETTERS_PDF_BUCKET_NAME = 'development-letters-pdf'
@@ -405,6 +406,7 @@ class Test(Development):
     TESTING = True
 
     CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload'
+    CONTACT_LIST_BUCKET_NAME = 'test-contact-list'
     TEST_LETTERS_BUCKET_NAME = 'test-test-letters'
     DVLA_RESPONSE_BUCKET_NAME = 'test.notify.com-ftp'
     LETTERS_PDF_BUCKET_NAME = 'test-letters-pdf'
@@ -440,6 +442,7 @@ class Preview(Config):
     NOTIFY_EMAIL_DOMAIN = 'notify.works'
     NOTIFY_ENVIRONMENT = 'preview'
     CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload'
+    CONTACT_LIST_BUCKET_NAME = 'preview-contact-list'
     TEST_LETTERS_BUCKET_NAME = 'preview-test-letters'
     DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp'
     LETTERS_PDF_BUCKET_NAME = 'preview-letters-pdf'
@@ -456,6 +459,7 @@ class Staging(Config):
     NOTIFY_EMAIL_DOMAIN = 'staging-notify.works'
     NOTIFY_ENVIRONMENT = 'staging'
     CSV_UPLOAD_BUCKET_NAME = 'staging-notifications-csv-upload'
+    CONTACT_LIST_BUCKET_NAME = 'staging-contact-list'
     TEST_LETTERS_BUCKET_NAME = 'staging-test-letters'
     DVLA_RESPONSE_BUCKET_NAME = 'staging-notify.works-ftp'
     LETTERS_PDF_BUCKET_NAME = 'staging-letters-pdf'
@@ -473,6 +477,7 @@ class Live(Config):
     NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk'
     NOTIFY_ENVIRONMENT = 'live'
     CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload'
+    CONTACT_LIST_BUCKET_NAME = 'production-contact-list'
     TEST_LETTERS_BUCKET_NAME = 'production-test-letters'
     DVLA_RESPONSE_BUCKET_NAME = 'notifications.service.gov.uk-ftp'
     LETTERS_PDF_BUCKET_NAME = 'production-letters-pdf'
@@ -497,6 +502,7 @@ class Sandbox(CloudFoundryConfig):
     NOTIFY_EMAIL_DOMAIN = 'notify.works'
     NOTIFY_ENVIRONMENT = 'sandbox'
     CSV_UPLOAD_BUCKET_NAME = 'cf-sandbox-notifications-csv-upload'
+    CONTACT_LIST_BUCKET_NAME = 'cf-sandbox-contact-list'
     LETTERS_PDF_BUCKET_NAME = 'cf-sandbox-letters-pdf'
     TEST_LETTERS_BUCKET_NAME = 'cf-sandbox-test-letters'
     DVLA_RESPONSE_BUCKET_NAME = 'notify.works-ftp'

app/service/rest.py

@@ -13,6 +13,7 @@ from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm.exc import NoResultFound
 
 from app import DATE_FORMAT, DATETIME_FORMAT_NO_TIMEZONE
+from app.aws import s3
 from app.config import QueueNames
 from app.dao import fact_notification_status_dao, notifications_dao
 from app.dao.dao_utils import dao_rollback
@@ -1047,6 +1048,7 @@ def delete_contact_list_by_id(service_id, contact_list_id):
     )
     dao_unlink_jobs_from_contact_list_id(contact_list.id)
     dao_delete_contact_list(contact_list)
+    s3.remove_contact_list_from_s3(service_id, contact_list_id)
 
     return '', 204
 

tests/app/service/test_service_contact_list_rest.py

@@ -120,7 +120,8 @@ def test_dao_get_contact_list_by_id_does_not_return_if_contact_list_id_for_anoth
     assert response['message'] == "No result found"
 
 
-def test_dao_delete_contact_list_by_id(admin_request, sample_service):
+def test_dao_delete_contact_list_by_id(mocker, admin_request, sample_service):
+    mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
     service_1 = create_service(service_name='Service under test')
     template_1 = create_template(service=service_1)
     expected_list = create_service_contact_list(service=service_1)
@@ -140,8 +141,14 @@ def test_dao_delete_contact_list_by_id(admin_request, sample_service):
     assert job_2.contact_list_id == other_list.id
     assert job_3.contact_list_id is None
 
+    mock_s3.assert_called_once_with(
+        expected_list.service.id,
+        expected_list.id,
+    )
 
-def test_dao_delete_contact_list_when_unused(admin_request, sample_service):
+
+def test_dao_delete_contact_list_when_unused(mocker, admin_request, sample_service):
+    mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
     service = create_service(service_name='Service under test')
     expected_list = create_service_contact_list(service=service)
 
@@ -155,8 +162,16 @@ def test_dao_delete_contact_list_when_unused(admin_request, sample_service):
 
     assert ServiceContactList.query.count() == 0
 
+    mock_s3.assert_called_once_with(
+        expected_list.service.id,
+        expected_list.id,
+    )
+
+
+def test_dao_delete_contact_list_by_id_for_different_service(mocker, admin_request, sample_service):
+
+    mock_s3 = mocker.patch('app.service.rest.s3.remove_contact_list_from_s3')
 
-def test_dao_delete_contact_list_by_id_for_different_service(admin_request, sample_service):
     service_1 = create_service(service_name='Service under test')
     service_2 = create_service(service_name='Other service')
 
@@ -171,3 +186,4 @@ def test_dao_delete_contact_list_by_id_for_different_service(admin_request, samp
     )
 
     assert ServiceContactList.query.count() == 1
+    assert mock_s3.called is False