diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 3264126df..4845aa9cf 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -402,7 +402,16 @@ def service_add_email_reply_to(service_id): first_email_address=first_email_address) -@main.route("/services//service-settings/email-reply-to//edit", methods=['GET', 'POST']) +@main.route( + "/services//service-settings/email-reply-to//edit", + methods=['GET', 'POST'], + endpoint="service_edit_email_reply_to" +) +@main.route( + "/services//service-settings/email-reply-to//delete", + methods=['GET'], + endpoint="service_confirm_delete_email_reply_to" +) @login_required @user_has_permissions('manage_service') def service_edit_email_reply_to(service_id, reply_to_email_id): @@ -422,7 +431,21 @@ def service_edit_email_reply_to(service_id, reply_to_email_id): return render_template( 'views/service-settings/email-reply-to/edit.html', form=form, - reply_to_email_address_id=reply_to_email_address['id']) + reply_to_email_address_id=reply_to_email_id, + confirm_delete=(request.endpoint == "main.service_confirm_delete_email_reply_to"), + ) + + +@main.route("/services//service-settings/email-reply-to//delete", methods=['POST']) +@login_required +@user_has_permissions('manage_service') +def service_delete_email_reply_to(service_id, reply_to_email_id): + service_api_client.update_reply_to_email_address( + current_service['id'], + reply_to_email_id=reply_to_email_id, + active=False, + ) + return redirect(url_for('.service_email_reply_to', service_id=service_id)) @main.route("/services//service-settings/set-inbound-number", methods=['GET', 'POST']) @@ -653,7 +676,16 @@ def service_add_sms_sender(service_id): first_sms_sender=first_sms_sender) -@main.route("/services//service-settings/sms-sender//edit", methods=['GET', 'POST']) +@main.route( + "/services//service-settings/sms-sender//edit", + methods=['GET', 'POST'], + endpoint="service_edit_sms_sender" +) +@main.route( + "/services//service-settings/sms-sender//delete", + methods=['GET'], + endpoint="service_confirm_delete_sms_sender" +) @login_required @user_has_permissions('manage_service') def service_edit_sms_sender(service_id, sms_sender_id): @@ -678,10 +710,27 @@ def service_edit_sms_sender(service_id, sms_sender_id): 'views/service-settings/sms-sender/edit.html', form=form, sms_sender=sms_sender, - inbound_number=is_inbound_number + inbound_number=is_inbound_number, + sms_sender_id=sms_sender_id, + confirm_delete=(request.endpoint == "main.service_confirm_delete_sms_sender") ) +@main.route( + "/services//service-settings/sms-sender//delete", + methods=['POST'], +) +@login_required +@user_has_permissions('manage_service') +def service_delete_sms_sender(service_id, sms_sender_id): + service_api_client.update_sms_sender( + current_service['id'], + sms_sender_id=sms_sender_id, + active=False, + ) + return redirect(url_for('.service_sms_senders', service_id=service_id)) + + @main.route("/services//service-settings/set-letter-contact-block", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_service') diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 11c70a481..6fdd95c82 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -374,7 +374,25 @@ class ServiceAPIClient(NotifyAdminAPIClient): ) @cache.delete('service-{service_id}') - def update_reply_to_email_address(self, service_id, reply_to_email_id, email_address, is_default=False): + def update_reply_to_email_address( + self, + service_id, + reply_to_email_id, + email_address=None, + active=None, + is_default=False + ): + data = { + "is_default": is_default + } + if email_address is not None: + data.update({ + 'email_address': email_address + }) + if active is not None: + data.update({ + 'active': active + }) return self.post( "/service/{}/email-reply-to/{}".format( service_id, @@ -439,13 +457,21 @@ class ServiceAPIClient(NotifyAdminAPIClient): return self.post("/service/{}/sms-sender".format(service_id), data=data) @cache.delete('service-{service_id}') - def update_sms_sender(self, service_id, sms_sender_id, sms_sender, is_default=False): + def update_sms_sender(self, service_id, sms_sender_id, sms_sender=None, active=None, is_default=False): + data = { + "is_default": is_default, + } + if sms_sender is not None: + data.update({ + "sms_sender": sms_sender, + }) + if active is not None: + data.update({ + "active": active, + }) return self.post( "/service/{}/sms-sender/{}".format(service_id, sms_sender_id), - data={ - "sms_sender": sms_sender, - "is_default": is_default - } + data=data ) def get_service_callback_api(self, service_id, callback_api_id): diff --git a/app/templates/views/service-settings/email-reply-to/edit.html b/app/templates/views/service-settings/email-reply-to/edit.html index fb0fd6473..860e701be 100644 --- a/app/templates/views/service-settings/email-reply-to/edit.html +++ b/app/templates/views/service-settings/email-reply-to/edit.html @@ -1,4 +1,5 @@ {% extends "withnav_template.html" %} +{% from "components/banner.html" import banner_wrapper %} {% from "components/textbox.html" import textbox %} {% from "components/checkbox.html" import checkbox %} {% from "components/page-footer.html" import page_footer %} @@ -9,9 +10,20 @@ {% block maincolumn_content %} -

- Edit email reply to address -

+ {% if confirm_delete %} +
+ {% call banner_wrapper(type='dangerous', subhead="Are you sure you want to delete this email reply to address?") %} +
+ + +
+ {% endcall %} +
+ {% else %} +

+ Edit email reply to address +

+ {% endif %}
{{ textbox( form.email_address, @@ -22,16 +34,23 @@

This is the default reply to address for {{ current_service.name }} emails

+ {{ page_footer( + 'Save', + back_link=url_for('.service_email_reply_to', service_id=current_service.id), + back_link_text='Back' + ) }} {% else %}
{{ checkbox(form.is_default) }}
+ {{ page_footer( + 'Save', + back_link=url_for('.service_email_reply_to', service_id=current_service.id), + back_link_text='Back', + delete_link=url_for('.service_confirm_delete_email_reply_to', service_id=current_service.id, reply_to_email_id=reply_to_email_address_id), + delete_link_text='Delete' + ) }} {% endif %} - {{ page_footer( - 'Save', - back_link=url_for('.service_email_reply_to', service_id=current_service.id), - back_link_text='Back' - ) }}
{% endblock %} diff --git a/app/templates/views/service-settings/sms-sender/edit.html b/app/templates/views/service-settings/sms-sender/edit.html index cccc8ef12..d32250884 100644 --- a/app/templates/views/service-settings/sms-sender/edit.html +++ b/app/templates/views/service-settings/sms-sender/edit.html @@ -1,4 +1,5 @@ {% extends "withnav_template.html" %} +{% from "components/banner.html" import banner_wrapper %} {% from "components/textbox.html" import textbox %} {% from "components/checkbox.html" import checkbox %} {% from "components/page-footer.html" import page_footer %} @@ -9,9 +10,20 @@ {% block maincolumn_content %} -

- Edit text message sender -

+ {% if confirm_delete %} +
+ {% call banner_wrapper(type='dangerous', subhead="Are you sure you want to delete this text message sender?") %} +
+ + +
+ {% endcall %} +
+ {% else %} +

+ Edit text message sender +

+ {% endif %}
{% if inbound_number %}

@@ -27,18 +39,33 @@ {% endif %} {% if form.is_default.data %}

- This is currently your text message sender for {{ current_service.name }} + This is the default text message sender

+ {{ page_footer( + 'Save', + back_link=None if request.args.get('from_template') else url_for('.service_sms_senders', service_id=current_service.id), + back_link_text='Back' + ) }} {% else %}
{{ checkbox(form.is_default) }}
+ {% if inbound_number %} + {{ page_footer( + 'Save', + back_link=None if request.args.get('from_template') else url_for('.service_sms_senders', service_id=current_service.id), + back_link_text='Back' + ) }} + {% else %} + {{ page_footer( + 'Save', + back_link=None if request.args.get('from_template') else url_for('.service_sms_senders', service_id=current_service.id), + back_link_text='Back', + delete_link=url_for('.service_confirm_delete_sms_sender', service_id=current_service.id, sms_sender_id=sms_sender_id), + delete_link_text='Delete' + ) }} + {% endif %} {% endif %} - {{ page_footer( - 'Save', - back_link=None if request.args.get('from_template') else url_for('.service_sms_senders', service_id=current_service.id), - back_link_text='Back' - ) }}
-{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 005e032ef..cd2f8fd0a 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,4 +1,5 @@ import uuid +from functools import partial from unittest.mock import ANY, call import pytest @@ -15,6 +16,7 @@ from tests.conftest import ( active_user_no_api_key_permission, active_user_no_settings_permission, active_user_with_permissions, + fake_uuid, get_default_letter_contact_block, get_default_reply_to_email_address, get_default_sms_sender, @@ -1128,6 +1130,90 @@ def test_edit_reply_to_email_address( ) +fixed_fake_uuid = fake_uuid() + + +@pytest.mark.parametrize('fixture, expected_link_text, partial_href', [ + ( + get_non_default_reply_to_email_address, + 'Delete', + partial(url_for, 'main.service_confirm_delete_email_reply_to', reply_to_email_id=fixed_fake_uuid), + ), + ( + get_default_reply_to_email_address, + 'Back', + partial(url_for, '.service_email_reply_to'), + ), +]) +def test_shows_delete_link_for_email_reply_to_address( + mocker, + fixture, + expected_link_text, + partial_href, + fake_uuid, + client_request, +): + + fixture(mocker) + + page = client_request.get( + 'main.service_edit_email_reply_to', + service_id=SERVICE_ONE_ID, + reply_to_email_id=fixed_fake_uuid, + ) + + last_link = page.select('.page-footer a')[-1] + + assert normalize_spaces(last_link.text) == expected_link_text + assert last_link['href'] == partial_href(service_id=SERVICE_ONE_ID) + + +def test_confirm_delete_reply_to_email_address( + fake_uuid, + client_request, + get_non_default_reply_to_email_address +): + + page = client_request.get( + 'main.service_confirm_delete_email_reply_to', + service_id=SERVICE_ONE_ID, + reply_to_email_id=fake_uuid, + _test_page_title=False, + ) + + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( + 'Are you sure you want to delete this email reply to address?' + ) + assert 'action' not in page.select_one('.banner-dangerous form') + assert page.select_one('.banner-dangerous form')['method'] == 'post' + + +def test_delete_reply_to_email_address( + client_request, + service_one, + fake_uuid, + get_non_default_reply_to_email_address, + mock_update_reply_to_email_address, +): + + client_request.post( + '.service_delete_email_reply_to', + service_id=SERVICE_ONE_ID, + reply_to_email_id=fake_uuid, + _expected_redirect=url_for( + 'main.service_email_reply_to', + service_id=SERVICE_ONE_ID, + _external=True, + ) + ) + + mock_update_reply_to_email_address.assert_called_once_with( + SERVICE_ONE_ID, + active=False, + reply_to_email_id=fake_uuid, + ) + + @pytest.mark.parametrize('fixture, data, api_default_args', [ (get_default_letter_contact_block, {"is_default": "y"}, True), (get_default_letter_contact_block, {}, True), @@ -1223,14 +1309,14 @@ def test_edit_sms_sender( ( 'main.service_edit_sms_sender', get_default_sms_sender, - 'This is currently your text message sender for service one', + 'This is the default text message sender', 'sms_sender_id', False ), ( 'main.service_edit_sms_sender', get_non_default_sms_sender, - 'This is currently your text message sender for service one', + 'This is the default text message sender', 'sms_sender_id', True ) @@ -1264,6 +1350,112 @@ def test_default_box_shows_on_non_default_sender_details_while_editing( ) +@pytest.mark.parametrize('fixture, expected_link_text, partial_href', [ + ( + get_non_default_sms_sender, + 'Delete', + partial(url_for, 'main.service_confirm_delete_sms_sender', sms_sender_id=fixed_fake_uuid), + ), + ( + get_default_sms_sender, + 'Back', + partial(url_for, '.service_sms_senders'), + ), +]) +def test_shows_delete_link_for_sms_sender( + mocker, + fixture, + expected_link_text, + partial_href, + fake_uuid, + client_request, +): + + fixture(mocker) + + page = client_request.get( + 'main.service_edit_sms_sender', + service_id=SERVICE_ONE_ID, + sms_sender_id=fixed_fake_uuid, + ) + + last_link = page.select('.page-footer a')[-1] + + assert normalize_spaces(last_link.text) == expected_link_text + assert last_link['href'] == partial_href(service_id=SERVICE_ONE_ID) + + +def test_confirm_delete_sms_sender( + fake_uuid, + client_request, + get_non_default_sms_sender, +): + + page = client_request.get( + 'main.service_confirm_delete_sms_sender', + service_id=SERVICE_ONE_ID, + sms_sender_id=fake_uuid, + _test_page_title=False, + ) + + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( + 'Are you sure you want to delete this text message sender?' + ) + assert 'action' not in page.select_one('.banner-dangerous form') + assert page.select_one('.banner-dangerous form')['method'] == 'post' + + +@pytest.mark.parametrize('fixture, expected_link_text', [ + (get_inbound_number_sms_sender, 'Back'), + (get_default_sms_sender, 'Back'), + (get_non_default_sms_sender, 'Delete'), +]) +def test_inbound_sms_sender_is_not_deleteable( + client_request, + service_one, + fake_uuid, + fixture, + expected_link_text, + mocker +): + fixture(mocker) + + page = client_request.get( + '.service_edit_sms_sender', + service_id=SERVICE_ONE_ID, + sms_sender_id='1234', + ) + + last_link = page.select('.page-footer a')[-1] + assert normalize_spaces(last_link.text) == expected_link_text + + +def test_delete_sms_sender( + client_request, + service_one, + fake_uuid, + get_non_default_sms_sender, + mock_update_sms_sender, +): + + client_request.post( + '.service_delete_sms_sender', + service_id=SERVICE_ONE_ID, + sms_sender_id='1234', + _expected_redirect=url_for( + 'main.service_sms_senders', + service_id=SERVICE_ONE_ID, + _external=True, + ) + ) + + mock_update_sms_sender.assert_called_once_with( + SERVICE_ONE_ID, + active=False, + sms_sender_id='1234', + ) + + @pytest.mark.parametrize('fixture, hide_textbox, fixture_sender_id', [ (get_inbound_number_sms_sender, True, '1234'), (get_default_sms_sender, False, '1234'), diff --git a/tests/conftest.py b/tests/conftest.py index e14f9c34c..f66e07713 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -147,7 +147,7 @@ def mock_add_reply_to_email_address(mocker): @pytest.fixture(scope='function') def mock_update_reply_to_email_address(mocker): - def _update_reply_to(service_id, reply_to_email_id, email_address, is_default=False): + def _update_reply_to(service_id, reply_to_email_id, email_address=None, active=None, is_default=False): return return mocker.patch('app.service_api_client.update_reply_to_email_address', side_effect=_update_reply_to) @@ -450,7 +450,7 @@ def mock_add_sms_sender(mocker): @pytest.fixture(scope='function') def mock_update_sms_sender(mocker): - def _update_sms_sender(service_id, sms_sender_id, sms_sender, is_default=False): + def _update_sms_sender(service_id, sms_sender_id, sms_sender=None, active=None, is_default=False): return return mocker.patch('app.service_api_client.update_sms_sender', side_effect=_update_sms_sender)