From 17bf06d04c4bb1c9948efee6235afb1dda349321 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Jul 2019 16:11:19 +0100 Subject: [PATCH] Let users delete letter contact blocks Because they can delete email reply to addresses and text message senders. --- app/main/views/service_settings.py | 27 ++++++++++- app/navigation.py | 8 ++++ app/notify_client/service_api_client.py | 7 +++ .../service-settings/letter-contact/edit.html | 6 ++- tests/app/main/views/test_service_settings.py | 45 +++++++++++++++++++ .../notify_client/test_service_api_client.py | 1 + 6 files changed, 92 insertions(+), 2 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 720c67155..c8903d11b 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -736,7 +736,16 @@ def service_add_letter_contact(service_id): ) -@main.route("/services//service-settings/letter-contact//edit", methods=['GET', 'POST']) +@main.route( + "/services//service-settings/letter-contact//edit", + methods=['GET', 'POST'], + endpoint="service_edit_letter_contact", +) +@main.route( + "/services//service-settings/letter-contact//delete", + methods=['GET'], + endpoint="service_confirm_delete_letter_contact", +) @user_has_permissions('manage_service') def service_edit_letter_contact(service_id, letter_contact_id): letter_contact_block = current_service.get_letter_contact_block(letter_contact_id) @@ -753,12 +762,28 @@ def service_edit_letter_contact(service_id, letter_contact_id): is_default=True if letter_contact_block['is_default'] else form.is_default.data ) return redirect(url_for('.service_letter_contact_details', service_id=service_id)) + + if (request.endpoint == "main.service_confirm_delete_letter_contact"): + flash("Are you sure you want to delete this contact block?", 'delete') return render_template( 'views/service-settings/letter-contact/edit.html', form=form, letter_contact_id=letter_contact_block['id']) +@main.route( + "/services//service-settings/letter-contact//delete", + methods=['POST'], +) +@user_has_permissions('manage_service') +def service_delete_letter_contact(service_id, letter_contact_id): + service_api_client.delete_letter_contact( + service_id=current_service.id, + letter_contact_id=letter_contact_id, + ) + return redirect(url_for('.service_letter_contact_details', service_id=current_service.id)) + + @main.route("/services//service-settings/sms-sender", methods=['GET']) @user_has_permissions('manage_service', 'manage_api_keys') def service_sms_senders(service_id): diff --git a/app/navigation.py b/app/navigation.py index 5d236f4ae..d19441fcf 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -246,10 +246,12 @@ class HeaderNavigation(Navigation): 'service_accept_agreement', 'service_confirm_agreement', 'service_confirm_delete_email_reply_to', + 'service_confirm_delete_letter_contact', 'service_confirm_delete_sms_sender', 'service_dashboard', 'service_dashboard_updates', 'service_delete_email_reply_to', + 'service_delete_letter_contact', 'service_delete_sms_sender', 'service_download_agreement', 'service_edit_email_reply_to', @@ -381,6 +383,7 @@ class MainNavigation(Navigation): 'service_accept_agreement', 'service_confirm_agreement', 'service_confirm_delete_email_reply_to', + 'service_confirm_delete_letter_contact', 'service_confirm_delete_sms_sender', 'service_edit_email_reply_to', 'service_edit_letter_contact', @@ -535,6 +538,7 @@ class MainNavigation(Navigation): 'send_notification', 'service_dashboard_updates', 'service_delete_email_reply_to', + 'service_delete_letter_contact', 'service_delete_sms_sender', 'service_download_agreement', 'service_letter_validation_preview', @@ -769,10 +773,12 @@ class CaseworkNavigation(Navigation): 'service_accept_agreement', 'service_confirm_agreement', 'service_confirm_delete_email_reply_to', + 'service_confirm_delete_letter_contact', 'service_confirm_delete_sms_sender', 'service_dashboard', 'service_dashboard_updates', 'service_delete_email_reply_to', + 'service_delete_letter_contact', 'service_delete_sms_sender', 'service_download_agreement', 'service_edit_email_reply_to', @@ -1039,10 +1045,12 @@ class OrgNavigation(Navigation): 'service_accept_agreement', 'service_confirm_agreement', 'service_confirm_delete_email_reply_to', + 'service_confirm_delete_letter_contact', 'service_confirm_delete_sms_sender', 'service_dashboard', 'service_dashboard_updates', 'service_delete_email_reply_to', + 'service_delete_letter_contact', 'service_delete_sms_sender', 'service_download_agreement', 'service_edit_email_reply_to', diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 8adbee048..519e268d3 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -458,6 +458,13 @@ class ServiceAPIClient(NotifyAdminAPIClient): } ) + @cache.delete('service-{service_id}') + def delete_letter_contact(self, service_id, letter_contact_id): + return self.post( + "/service/{}/letter-contact/{}/archive".format(service_id, letter_contact_id), + data=None + ) + def get_sms_senders(self, service_id): return self.get( "/service/{}/sms-sender".format(service_id) diff --git a/app/templates/views/service-settings/letter-contact/edit.html b/app/templates/views/service-settings/letter-contact/edit.html index 3dc8dc4d9..0c2839928 100644 --- a/app/templates/views/service-settings/letter-contact/edit.html +++ b/app/templates/views/service-settings/letter-contact/edit.html @@ -33,7 +33,11 @@ {{ checkbox(form.is_default) }} {% endif %} - {{ page_footer('Save') }} + {{ page_footer( + 'Save', + delete_link=url_for('.service_confirm_delete_letter_contact', service_id=current_service.id, letter_contact_id=letter_contact_id), + delete_link_text='Delete' + ) }} {% endcall %} {% endblock %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index f20ec3f93..3ace19a89 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -2464,6 +2464,51 @@ def test_edit_letter_contact_block( ) +def test_confirm_delete_letter_contact_block( + fake_uuid, + client_request, + get_default_letter_contact_block, +): + + page = client_request.get( + 'main.service_confirm_delete_letter_contact', + service_id=SERVICE_ONE_ID, + letter_contact_id=fake_uuid, + _test_page_title=False, + ) + + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( + 'Are you sure you want to delete this contact block? ' + 'Yes, delete' + ) + assert 'action' not in page.select_one('.banner-dangerous form') + assert page.select_one('.banner-dangerous form')['method'] == 'post' + + +def test_delete_letter_contact_block( + client_request, + service_one, + fake_uuid, + get_default_letter_contact_block, + mocker, +): + mock_delete = mocker.patch('app.service_api_client.delete_letter_contact') + client_request.post( + '.service_delete_letter_contact', + service_id=SERVICE_ONE_ID, + letter_contact_id=fake_uuid, + _expected_redirect=url_for( + 'main.service_letter_contact_details', + service_id=SERVICE_ONE_ID, + _external=True, + ) + ) + mock_delete.assert_called_once_with( + service_id=SERVICE_ONE_ID, + letter_contact_id=fake_uuid, + ) + + @pytest.mark.parametrize('fixture, data, api_default_args', [ (get_default_sms_sender, {"is_default": "y", "sms_sender": "test"}, True), (get_default_sms_sender, {"sms_sender": "test"}, True), diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 24d350146..d12e16b01 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -355,6 +355,7 @@ def test_returns_value_from_cache( (service_api_client, 'delete_reply_to_email_address', [SERVICE_ONE_ID, ''], {}), (service_api_client, 'add_letter_contact', [SERVICE_ONE_ID, ''], {}), (service_api_client, 'update_letter_contact', [SERVICE_ONE_ID] + [''] * 2, {}), + (service_api_client, 'delete_letter_contact', [SERVICE_ONE_ID, ''], {}), (service_api_client, 'add_sms_sender', [SERVICE_ONE_ID, ''], {}), (service_api_client, 'update_sms_sender', [SERVICE_ONE_ID] + [''] * 2, {}), (service_api_client, 'delete_sms_sender', [SERVICE_ONE_ID, ''], {}),