Allow delete email reply to address, SMS senders

For both SMS senders and email reply to addresses this commit adds:
- a delete link
- a confirmation loop

It doesn’t let users delete:
- default SMS senders or reply to addresses (they always have to have
  one)
- inbound numbers

It assumes that the API will allow updating of an attribute named
`active` on the respective database rows. It could work in a different
way. We can’t do complete deletion though because these will still be
keyed to notifications.

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_id>/service-settings/email-reply-to/<reply_to_email_id>/edit", methods=['GET', 'POST'])
+@main.route(
+    "/services/<service_id>/service-settings/email-reply-to/<reply_to_email_id>/edit",
+    methods=['GET', 'POST'],
+    endpoint="service_edit_email_reply_to"
+)
+@main.route(
+    "/services/<service_id>/service-settings/email-reply-to/<reply_to_email_id>/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_id>/service-settings/email-reply-to/<reply_to_email_id>/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_id>/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_id>/service-settings/sms-sender/<sms_sender_id>/edit", methods=['GET', 'POST'])
+@main.route(
+    "/services/<service_id>/service-settings/sms-sender/<sms_sender_id>/edit",
+    methods=['GET', 'POST'],
+    endpoint="service_edit_sms_sender"
+)
+@main.route(
+    "/services/<service_id>/service-settings/sms-sender/<sms_sender_id>/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_id>/service-settings/sms-sender/<sms_sender_id>/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_id>/service-settings/set-letter-contact-block", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('manage_service')

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={
+            data=data
-                "sms_sender": sms_sender,
-                "is_default": is_default
-            }
         )
 
     def get_service_callback_api(self, service_id, callback_api_id):

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 %}
 
-  <h1 class="heading-large">
+  {% if confirm_delete %}
-    Edit email reply to address
+    <div class="bottom-gutter">
-  </h1>
+      {% call banner_wrapper(type='dangerous', subhead="Are you sure you want to delete this email reply to address?") %}
+        <form method='post'>
+          <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+          <input type="submit" class="button" name="delete" value="Confirm" />
+        </form>
+      {% endcall %}
+    </div>
+  {% else %}
+    <h1 class="heading-large">
+      Edit email reply to address
+    </h1>
+  {% endif %}
   <form method="post">
     {{ textbox(
       form.email_address,
@@ -22,16 +34,23 @@
       <p class="form-group">
         This is the default reply to address for {{ current_service.name }} emails
       </p>
+      {{ page_footer(
+        'Save',
+        back_link=url_for('.service_email_reply_to', service_id=current_service.id),
+        back_link_text='Back'
+      ) }}
     {% else %}
       <div class="form-group">
         {{ checkbox(form.is_default) }}
       </div>
+      {{ 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'
-  ) }}
   </form>
 
 {% endblock %}

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 %}
 
-  <h1 class="heading-large">
+  {% if confirm_delete %}
-    Edit text message sender
+    <div class="bottom-gutter">
-  </h1>
+      {% call banner_wrapper(type='dangerous', subhead="Are you sure you want to delete this text message sender?") %}
+        <form method='post'>
+          <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+          <input type="submit" class="button" name="delete" value="Confirm" />
+        </form>
+      {% endcall %}
+    </div>
+  {% else %}
+    <h1 class="heading-large">
+      Edit text message sender
+    </h1>
+  {% endif %}
   <form method="post">
     {% if inbound_number %}
       <p>
@@ -27,18 +39,33 @@
     {% endif %}
     {% if form.is_default.data %}
       <p class="form-group">
-        This is currently your text message sender for {{ current_service.name }}
+        This is the default text message sender
       </p>
+      {{ 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 %}
       <div class="form-group">
         {{ checkbox(form.is_default) }}
       </div>
+      {% 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'
-    ) }}
   </form>
 
-{% endblock %}
+{% endblock %}

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'),

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)