Allow deleting default letter contact blocks

It’s possible to delete default letter contact blocks because there is a
fallback – having a blank letter contact block. This is different to SMS
senders and reply to addresses.

For this to make sense it also means:
- adding the ‘blank’ letter contact block to the list of letter contact
  blocks
- having a way of setting the default back to being blank

app/main/views/service_settings.py

@@ -755,11 +755,10 @@ def service_edit_letter_contact(service_id, letter_contact_id):
     if request.method == 'GET':
         form.is_default.data = letter_contact_block['is_default']
     if form.validate_on_submit():
-        service_api_client.update_letter_contact(
+        current_service.edit_letter_contact_block(
-            current_service.id,
+            id=letter_contact_id,
-            letter_contact_id=letter_contact_id,
             contact_block=form.letter_contact_block.data.replace('\r', '') or None,
-            is_default=True if letter_contact_block['is_default'] else form.is_default.data
+            is_default=letter_contact_block['is_default'] or form.is_default.data
         )
         return redirect(url_for('.service_letter_contact_details', service_id=service_id))
 
@@ -771,6 +770,13 @@ def service_edit_letter_contact(service_id, letter_contact_id):
         letter_contact_id=letter_contact_block['id'])
 
 
+@main.route("/services/<service_id>/service-settings/letter-contact/make-blank-default")
+@user_has_permissions('manage_service')
+def service_make_blank_default_letter_contact(service_id):
+    current_service.remove_default_letter_contact_block()
+    return redirect(url_for('.service_letter_contact_details', service_id=service_id))
+
+
 @main.route(
     "/services/<service_id>/service-settings/letter-contact/<letter_contact_id>/delete",
     methods=['POST'],

app/models/service.py

@@ -1,5 +1,7 @@
-from flask import abort, current_app
+from flask import Markup, abort, current_app
 from notifications_utils.field import Field
+from notifications_utils.formatters import nl2br
+from notifications_utils.take import Take
 from werkzeug.utils import cached_property
 
 from app.models import JSONModel
@@ -324,11 +326,36 @@ class Service(JSONModel):
     def default_letter_contact_block(self):
         return next(
             (
-                Field(x['contact_block'], html='escape')
+                letter_contact_block
-                for x in self.letter_contact_details if x['is_default']
+                for letter_contact_block in self.letter_contact_details
+                if letter_contact_block['is_default']
             ), None
         )
 
+    @property
+    def default_letter_contact_block_html(self):
+        if self.default_letter_contact_block:
+            return Markup(Take(Field(
+                self.default_letter_contact_block['contact_block'],
+                html='escape',
+            )).then(
+                nl2br
+            ))
+        return ''
+
+    def edit_letter_contact_block(self, id, contact_block, is_default):
+        service_api_client.update_letter_contact(
+            self.id, letter_contact_id=id, contact_block=contact_block, is_default=is_default,
+        )
+
+    def remove_default_letter_contact_block(self):
+        if self.default_letter_contact_block:
+            self.edit_letter_contact_block(
+                self.default_letter_contact_block['id'],
+                self.default_letter_contact_block['contact_block'],
+                is_default=False,
+            )
+
     def get_letter_contact_block(self, id):
         return service_api_client.get_letter_contact(self.id, id)
 

app/navigation.py

@@ -260,6 +260,7 @@ class HeaderNavigation(Navigation):
         'service_email_reply_to',
         'service_letter_contact_details',
         'service_letter_validation_preview',
+        'service_make_blank_default_letter_contact',
         'service_name_change',
         'service_name_change_confirm',
         'service_preview_email_branding',
@@ -390,6 +391,7 @@ class MainNavigation(Navigation):
             'service_edit_sms_sender',
             'service_email_reply_to',
             'service_letter_contact_details',
+            'service_make_blank_default_letter_contact',
             'service_name_change',
             'service_name_change_confirm',
             'service_preview_email_branding',
@@ -787,6 +789,7 @@ class CaseworkNavigation(Navigation):
         'service_email_reply_to',
         'service_letter_contact_details',
         'service_letter_validation_preview',
+        'service_make_blank_default_letter_contact',
         'service_name_change',
         'service_name_change_confirm',
         'service_preview_email_branding',
@@ -1059,6 +1062,7 @@ class OrgNavigation(Navigation):
         'service_email_reply_to',
         'service_letter_contact_details',
         'service_letter_validation_preview',
+        'service_make_blank_default_letter_contact',
         'service_name_change',
         'service_name_change_confirm',
         'service_preview_email_branding',

app/templates/views/service-settings.html

@@ -213,8 +213,14 @@
 
         {% call settings_row(if_has_permission='letter') %}
           {{ text_field('Sender addresses') }}
-          {% call field(status='default' if current_service.default_letter_contact_block == None else '') %}
+          {% call field(status='' if current_service.count_letter_contact_details else 'default') %}
-            {{ current_service.default_letter_contact_block | string | nl2br | safe if current_service.default_letter_contact_block else 'Not set'}}
+            {% if current_service.default_letter_contact_block %}
+              {{ current_service.default_letter_contact_block_html }}
+            {% elif current_service.count_letter_contact_details %}
+              Blank
+            {% else %}
+              Not set
+            {% endif %}
             {% if current_service.count_letter_contact_details > 1 %}
               <div class="hint">
                 {{ '…and %d more' | format(current_service.count_letter_contact_details - 1) }}

app/templates/views/service-settings/letter-contact-details.html

@@ -17,11 +17,18 @@
     ) }}
   </div>
   <div class="user-list">
-    {% if not letter_contact_details %}
+    <div class="user-list-item">
-      <div class="user-list-item">
+      <span class="hint">
-        <span class="hint">You haven’t added any letter contact details yet</span>
+        Blank
-      </div>
+        {% if current_service.default_letter_contact_block %}
-    {% endif %}
+          {% if current_user.has_permissions('manage_service') %}
+            <a class="user-list-edit-link" href="{{ url_for('.service_make_blank_default_letter_contact', service_id =current_service.id) }}">Make default</a>
+          {% endif %}
+        {% else %}
+          (default)
+        {% endif %}
+      </span>
+    </div>
     {% for item in letter_contact_details %}
       <div class="user-list-item">
         <p>

app/templates/views/service-settings/letter-contact/edit.html

@@ -24,20 +24,24 @@
       rows=10,
       highlight_tags=True
     ) }}
+
+
     {% if form.is_default.data %}
       <p class="form-group">
-        This is currently your default address for {{ current_service.name }}
+        This is the default address for {{ current_service.name }}
       </p>
     {% else %}
       <div class="form-group">
         {{ checkbox(form.is_default) }}
       </div>
     {% endif %}
+
     {{ 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 %}

tests/app/main/views/test_service_settings.py

@@ -1852,10 +1852,10 @@ def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender(
     assert get_row(page, 11) == "Sender addresses 1 Example Street …and 2 more Manage"
 
 
-@pytest.mark.parametrize('sender_list_page, expected_output', [
+@pytest.mark.parametrize('sender_list_page, index, expected_output', [
-    ('main.service_email_reply_to', 'test@example.com (default) Change'),
+    ('main.service_email_reply_to', 0, 'test@example.com (default) Change'),
-    ('main.service_letter_contact_details', '1 Example Street (default) Change'),
+    ('main.service_letter_contact_details', 1, '1 Example Street (default) Change'),
-    ('main.service_sms_senders', 'GOVUK (default) Change')
+    ('main.service_sms_senders', 0, 'GOVUK (default) Change')
 ])
 def test_api_ids_dont_show_on_option_pages_with_a_single_sender(
     client_request,
@@ -1864,7 +1864,8 @@ def test_api_ids_dont_show_on_option_pages_with_a_single_sender(
     mock_get_service_organisation,
     single_sms_sender,
     sender_list_page,
-    expected_output
+    index,
+    expected_output,
 ):
     rows = client_request.get(
         sender_list_page,
@@ -1873,34 +1874,41 @@ def test_api_ids_dont_show_on_option_pages_with_a_single_sender(
         '.user-list-item'
     )
 
-    assert normalize_spaces(rows[0].text) == expected_output
+    assert normalize_spaces(rows[index].text) == expected_output
-    assert len(rows) == 1
+    assert len(rows) == index + 1
 
 
 @pytest.mark.parametrize(
-    'sender_list_page, \
+    (
-    sample_data, \
+        'sender_list_page,'
-    expected_default_sender_output, \
+        'sample_data,'
-    expected_second_sender_output, \
+        'expected_items,'
-    expected_third_sender_output',
+    ),
     [(
         'main.service_email_reply_to',
         multiple_reply_to_email_addresses,
-        'test@example.com (default) Change 1234',
+        [
-        'test2@example.com Change 5678',
+            'test@example.com (default) Change 1234',
-        'test3@example.com Change 9457'
+            'test2@example.com Change 5678',
+            'test3@example.com Change 9457',
+        ],
     ), (
         'main.service_letter_contact_details',
         multiple_letter_contact_blocks,
-        '1 Example Street (default) Change 1234',
+        [
-        '2 Example Street Change 5678',
+            'Blank Make default',
-        '3 Example Street Change 9457'
+            '1 Example Street (default) Change 1234',
+            '2 Example Street Change 5678',
+            '3 Example Street Change 9457',
+        ],
     ), (
         'main.service_sms_senders',
         multiple_sms_senders,
-        'Example (default and receives replies) Change 1234',
+        [
-        'Example 2 Change 5678',
+            'Example (default and receives replies) Change 1234',
-        'Example 3 Change 9457'
+            'Example 2 Change 5678',
+            'Example 3 Change 9457',
+        ],
     ),
     ]
 )
@@ -1909,9 +1917,7 @@ def test_default_option_shows_for_default_sender(
     mocker,
     sender_list_page,
     sample_data,
-    expected_default_sender_output,
+    expected_items,
-    expected_second_sender_output,
-    expected_third_sender_output
 ):
     sample_data(mocker)
 
@@ -1922,10 +1928,40 @@ def test_default_option_shows_for_default_sender(
         '.user-list-item'
     )
 
-    assert normalize_spaces(rows[0].text) == expected_default_sender_output
+    assert [normalize_spaces(row.text) for row in rows] == expected_items
-    assert normalize_spaces(rows[1].text) == expected_second_sender_output
+
-    assert normalize_spaces(rows[2].text) == expected_third_sender_output
+
-    assert len(rows) == 3
+def test_remove_default_from_default_letter_contact_block(
+    client_request,
+    mocker,
+    multiple_letter_contact_blocks,
+    mock_update_letter_contact,
+):
+    letter_contact_details_page = url_for(
+        'main.service_letter_contact_details',
+        service_id=SERVICE_ONE_ID,
+        _external=True,
+    )
+
+    link = client_request.get_url(letter_contact_details_page).select_one('.user-list-item a')
+    assert link.text == 'Make default'
+    assert link['href'] == url_for(
+        '.service_make_blank_default_letter_contact',
+        service_id=SERVICE_ONE_ID,
+    )
+
+    client_request.get_url(
+        link['href'],
+        _expected_status=302,
+        _expected_redirect=letter_contact_details_page,
+    )
+
+    mock_update_letter_contact.assert_called_once_with(
+        SERVICE_ONE_ID,
+        letter_contact_id='1234',
+        contact_block='1 Example Street',
+        is_default=False,
+    )
 
 
 @pytest.mark.parametrize('sender_list_page, sample_data, expected_output', [
@@ -1937,7 +1973,7 @@ def test_default_option_shows_for_default_sender(
     (
         'main.service_letter_contact_details',
         no_letter_contact_blocks,
-        'You haven’t added any letter contact details yet'
+        'Blank (default)'
     ),
     (
         'main.service_sms_senders',
@@ -2558,14 +2594,14 @@ def test_edit_sms_sender(
     (
         'main.service_edit_letter_contact',
         get_default_letter_contact_block,
-        'This is currently your default address for service one',
+        'This is the default address for service one',
         'letter_contact_id',
         False
     ),
     (
         'main.service_edit_letter_contact',
         get_non_default_letter_contact_block,
-        'This is the default contact details for service one letters',
+        'THIS TEXT WONT BE TESTED',
         'letter_contact_id',
         True
     ),