diff --git a/app/aws/s3.py b/app/aws/s3.py index 5c91bf8fc..cfba0f265 100644 --- a/app/aws/s3.py +++ b/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') diff --git a/app/config.py b/app/config.py index dacda4ff7..fab5faf24 100644 --- a/app/config.py +++ b/app/config.py @@ -363,6 +363,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' @@ -406,6 +407,7 @@ class Test(Development): HIGH_VOLUME_SERVICE = ['941b6f9a-50d7-4742-8d50-f365ca74bf27'] 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' @@ -441,6 +443,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' @@ -457,6 +460,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' @@ -474,6 +478,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' @@ -498,6 +503,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' diff --git a/app/dao/service_contact_list_dao.py b/app/dao/service_contact_list_dao.py index d4e2826b9..6a55cf405 100644 --- a/app/dao/service_contact_list_dao.py +++ b/app/dao/service_contact_list_dao.py @@ -5,7 +5,8 @@ from app.models import ServiceContactList def dao_get_contact_list_by_id(service_id, contact_list_id): contact_list = ServiceContactList.query.filter_by( service_id=service_id, - id=contact_list_id + id=contact_list_id, + archived=False, ).one() return contact_list @@ -13,7 +14,8 @@ def dao_get_contact_list_by_id(service_id, contact_list_id): def dao_get_contact_lists(service_id): contact_lists = ServiceContactList.query.filter_by( - service_id=service_id + service_id=service_id, + archived=False, ).order_by( ServiceContactList.created_at.desc() ) @@ -23,3 +25,9 @@ def dao_get_contact_lists(service_id): def save_service_contact_list(service_contact_list): db.session.add(service_contact_list) db.session.commit() + + +def dao_archive_contact_list(service_contact_list): + service_contact_list.archived = True + db.session.add(service_contact_list) + db.session.commit() diff --git a/app/models.py b/app/models.py index 802567046..64beaca52 100644 --- a/app/models.py +++ b/app/models.py @@ -2135,6 +2135,7 @@ class ServiceContactList(db.Model): created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=True) created_at = db.Column(db.DateTime, nullable=False) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + archived = db.Column(db.Boolean, nullable=False, default=False) def serialize(self): created_at_in_bst = convert_utc_to_bst(self.created_at) diff --git a/app/service/rest.py b/app/service/rest.py index 86920d297..a1300f462 100644 --- a/app/service/rest.py +++ b/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 @@ -36,8 +37,12 @@ from app.dao.returned_letters_dao import ( fetch_returned_letter_summary, fetch_returned_letters, ) -from app.dao.service_contact_list_dao import dao_get_contact_lists, save_service_contact_list, \ - dao_get_contact_list_by_id +from app.dao.service_contact_list_dao import ( + dao_archive_contact_list, + dao_get_contact_lists, + dao_get_contact_list_by_id, + save_service_contact_list, +) from app.dao.service_data_retention_dao import ( fetch_service_data_retention, fetch_service_data_retention_by_id, @@ -1034,6 +1039,18 @@ def get_contact_list_by_id(service_id, contact_list_id): return jsonify(contact_list.serialize()) +@service_blueprint.route('//contact-list/', methods=['DELETE']) +def delete_contact_list_by_id(service_id, contact_list_id): + contact_list = dao_get_contact_list_by_id( + service_id=service_id, + contact_list_id=contact_list_id, + ) + dao_archive_contact_list(contact_list) + s3.remove_contact_list_from_s3(service_id, contact_list_id) + + return '', 204 + + @service_blueprint.route('//contact-list', methods=['POST']) def create_contact_list(service_id): service_contact_list = validate(request.get_json(), create_service_contact_list_schema) diff --git a/migrations/versions/0319_contact_list_archived.py b/migrations/versions/0319_contact_list_archived.py new file mode 100644 index 000000000..7316651f5 --- /dev/null +++ b/migrations/versions/0319_contact_list_archived.py @@ -0,0 +1,26 @@ +""" + +Revision ID: 0319_contact_list_archived +Revises: 0318_service_contact_list +Create Date: 2020-03-26 11:16:12.389524 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '0319_contact_list_archived' +down_revision = '0318_service_contact_list' + + +def upgrade(): + op.add_column( + 'service_contact_list', + sa.Column('archived', sa.Boolean(), nullable=False, server_default=sa.false()), + ) + + +def downgrade(): + op.drop_column( + 'service_contact_list', + 'archived', + ) diff --git a/tests/app/dao/test_service_contact_list_dao.py b/tests/app/dao/test_service_contact_list_dao.py index df09fe0a4..a38b9923a 100644 --- a/tests/app/dao/test_service_contact_list_dao.py +++ b/tests/app/dao/test_service_contact_list_dao.py @@ -4,6 +4,11 @@ from tests.app.db import create_service_contact_list def test_dao_get_contact_lists(notify_db_session): contact_list = create_service_contact_list() + create_service_contact_list( + service=contact_list.service, + archived=True, + ) + fetched_list = dao_get_contact_lists(contact_list.service_id) assert len(fetched_list) == 1 diff --git a/tests/app/db.py b/tests/app/db.py index f9ed21efe..fb248a633 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -384,7 +384,8 @@ def create_job( processing_started=None, processing_finished=None, original_file_name='some.csv', - archived=False + archived=False, + contact_list_id=None, ): data = { 'id': uuid.uuid4(), @@ -400,7 +401,8 @@ def create_job( 'scheduled_for': scheduled_for, 'processing_started': processing_started, 'processing_finished': processing_finished, - 'archived': archived + 'archived': archived, + 'contact_list_id': contact_list_id, } job = Job(**data) dao_create_job(job) @@ -974,7 +976,8 @@ def create_service_contact_list( original_file_name='EmergencyContactList.xls', row_count=100, template_type='email', - created_by_id=None + created_by_id=None, + archived=False, ): if not service: service = create_service(service_name='service for contact list', user=create_user()) @@ -986,6 +989,7 @@ def create_service_contact_list( template_type=template_type, created_by_id=created_by_id or service.users[0].id, created_at=datetime.utcnow(), + archived=archived, ) db.session.add(contact_list) db.session.commit() diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 4ddb50d6f..98605b0d3 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -238,12 +238,21 @@ def test_create_scheduled_job(client, sample_template, mocker, fake_uuid): assert resp_json['data']['notification_count'] == 1 -def test_create_job_with_contact_list_id(client, mocker, sample_template, fake_uuid): +@pytest.mark.parametrize('contact_list_archived', ( + True, False, +)) +def test_create_job_with_contact_list_id( + client, + mocker, + sample_template, + fake_uuid, + contact_list_archived, +): mocker.patch('app.celery.tasks.process_job.apply_async') mocker.patch('app.job.rest.get_job_metadata_from_s3', return_value={ 'template_id': str(sample_template.id) }) - contact_list = create_service_contact_list() + contact_list = create_service_contact_list(archived=contact_list_archived) data = { 'id': fake_uuid, 'valid': 'True', diff --git a/tests/app/service/test_service_contact_list_rest.py b/tests/app/service/test_service_contact_list_rest.py index 387d47a7c..5b351c23b 100644 --- a/tests/app/service/test_service_contact_list_rest.py +++ b/tests/app/service/test_service_contact_list_rest.py @@ -1,7 +1,12 @@ import uuid from app.models import ServiceContactList -from tests.app.db import create_service_contact_list, create_service +from tests.app.db import ( + create_job, + create_service_contact_list, + create_service, + create_template, +) def test_create_service_contact_list(sample_service, admin_request): @@ -70,6 +75,7 @@ def test_get_contact_list_returns_for_service(admin_request, notify_db_session): expected_list_2 = create_service_contact_list(service=service_1) # not included in results create_service_contact_list(service=service_2) + create_service_contact_list(service=service_1, archived=True) response = admin_request.get( 'service.get_contact_list', @@ -96,6 +102,16 @@ def test_dao_get_contact_list_by_id(admin_request, sample_service): assert response == expected_list_1.serialize() +def test_dao_get_archived_contact_list_by_id(admin_request): + contact_list = create_service_contact_list(archived=True) + admin_request.get( + 'service.get_contact_list_by_id', + service_id=contact_list.service.id, + contact_list_id=contact_list.id, + _expected_status=404, + ) + + def test_dao_get_contact_list_by_id_does_not_return_if_contact_list_id_for_another_service( admin_request, sample_service ): @@ -113,3 +129,79 @@ 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_archive_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) + other_list = create_service_contact_list(service=service_1) + + # Job linked to the contact list we’re deleting + job_1 = create_job(template=template_1, contact_list_id=expected_list.id) + # Other jobs and lists shouldn’t be affected + job_2 = create_job(template=template_1, contact_list_id=other_list.id) + job_3 = create_job(template=template_1) + + admin_request.delete( + 'service.delete_contact_list_by_id', + service_id=service_1.id, + contact_list_id=expected_list.id, + ) + + assert job_1.contact_list_id == expected_list.id + assert expected_list.archived is True + + assert job_2.contact_list_id == other_list.id + assert other_list.archived is False + + assert job_3.contact_list_id is None + + mock_s3.assert_called_once_with( + expected_list.service.id, + expected_list.id, + ) + + +def test_archive_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) + + assert ServiceContactList.query.count() == 1 + + admin_request.delete( + 'service.delete_contact_list_by_id', + service_id=service.id, + contact_list_id=expected_list.id + ) + + assert ServiceContactList.query.count() == 1 + assert expected_list.archived is True + + mock_s3.assert_called_once_with( + expected_list.service.id, + expected_list.id, + ) + + +def test_archive_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') + + service_1 = create_service(service_name='Service under test') + service_2 = create_service(service_name='Other service') + + contact_list = create_service_contact_list(service=service_1) + assert ServiceContactList.query.count() == 1 + + admin_request.delete( + 'service.delete_contact_list_by_id', + service_id=service_2.id, + contact_list_id=contact_list.id, + _expected_status=404, + ) + + assert ServiceContactList.query.count() == 1 + assert mock_s3.called is False