From b50dbab8fd9e4f16ac91859a9ffa5202a96510d4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 18 Mar 2020 13:00:03 +0000 Subject: [PATCH] Add an endpoint to delete a contact list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was one of things we de-scoped when we first shipped this feature. In order to safely delete a list, we first need to make sure any jobs aren’t referencing it. --- app/dao/jobs_dao.py | 8 +++ app/dao/service_contact_list_dao.py | 5 ++ app/service/rest.py | 21 ++++++- tests/app/db.py | 6 +- .../service/test_service_contact_list_rest.py | 60 ++++++++++++++++++- 5 files changed, 95 insertions(+), 5 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index baff3bbb5..73b6430ec 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -135,6 +135,14 @@ def dao_update_job(job): db.session.commit() +def dao_unlink_jobs_from_contact_list_id(contact_list_id): + Job.query.filter( + Job.contact_list_id == contact_list_id, + ).update({ + 'contact_list_id': None, + }) + + def dao_get_jobs_older_than_data_retention(notification_types): flexible_data_retention = ServiceDataRetention.query.filter( ServiceDataRetention.notification_type.in_(notification_types) diff --git a/app/dao/service_contact_list_dao.py b/app/dao/service_contact_list_dao.py index d4e2826b9..10045bc80 100644 --- a/app/dao/service_contact_list_dao.py +++ b/app/dao/service_contact_list_dao.py @@ -23,3 +23,8 @@ 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_delete_contact_list(service_contact_list): + db.session.delete(service_contact_list) + db.session.commit() diff --git a/app/service/rest.py b/app/service/rest.py index 86920d297..6f950f087 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -29,6 +29,7 @@ from app.dao.fact_notification_status_dao import ( fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service ) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service +from app.dao.jobs_dao import dao_unlink_jobs_from_contact_list_id from app.dao.organisation_dao import dao_get_organisation_by_service_id from app.dao.returned_letters_dao import ( fetch_most_recent_returned_letter, @@ -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_get_contact_lists, + save_service_contact_list, + dao_get_contact_list_by_id, + dao_delete_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_unlink_jobs_from_contact_list_id(contact_list.id) + dao_delete_contact_list(contact_list) + + 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/tests/app/db.py b/tests/app/db.py index f9ed21efe..1397fa4d5 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) diff --git a/tests/app/service/test_service_contact_list_rest.py b/tests/app/service/test_service_contact_list_rest.py index 387d47a7c..238c97688 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): @@ -113,3 +118,56 @@ 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): + 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_1 = create_job(template=template_1, contact_list_id=expected_list.id) + 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 is None + assert job_2.contact_list_id == other_list.id + assert job_3.contact_list_id is None + + +def test_dao_delete_contact_list_when_unused(admin_request, sample_service): + 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() == 0 + + +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') + + 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