Merge pull request #3043 from alphagov/add-first-letter-contact-experience

Make adding a ‘letter contact block’ for the first time make sense

app/main/views/service_settings.py

@@ -706,21 +706,32 @@ def service_letter_contact_details(service_id):
 def service_add_letter_contact(service_id):
     form = ServiceLetterContactBlockForm()
     first_contact_block = current_service.count_letter_contact_details == 0
+    from_template = request.args.get('from_template')
     if form.validate_on_submit():
-        service_api_client.add_letter_contact(
+        new_letter_contact = service_api_client.add_letter_contact(
             current_service.id,
             contact_block=form.letter_contact_block.data.replace('\r', '') or None,
             is_default=first_contact_block if first_contact_block else form.is_default.data
         )
-        if request.args.get('from_template'):
+        if from_template:
+            service_api_client.update_service_template_sender(
+                service_id,
+                from_template,
+                new_letter_contact['data']['id'],
+            )
             return redirect(
-                url_for('.set_template_sender', service_id=service_id, template_id=request.args.get('from_template'))
+                url_for('.view_template', service_id=service_id, template_id=from_template)
             )
         return redirect(url_for('.service_letter_contact_details', service_id=service_id))
     return render_template(
         'views/service-settings/letter-contact/add.html',
         form=form,
         first_contact_block=first_contact_block,
+        back_link=(
+            url_for('main.view_template', template_id=from_template, service_id=current_service.id)
+            if from_template
+            else url_for('.service_letter_contact_details', service_id=current_service.id)
+        ),
     )
 
 

app/main/views/templates.py

@@ -57,13 +57,7 @@ def view_template(service_id, template_id):
         return redirect(url_for(
             '.send_one_off', service_id=service_id, template_id=template_id
         ))
-    if template["template_type"] == "letter":
+
-        letter_contact_details = service_api_client.get_letter_contacts(service_id)
-        default_letter_contact_block_id = next(
-            (x['id'] for x in letter_contact_details if x['is_default']), None
-        )
-    else:
-        default_letter_contact_block_id = None
     return render_template(
         'views/templates/template.html',
         template=get_template(
@@ -81,7 +75,6 @@ def view_template(service_id, template_id):
         ),
         template_postage=template["postage"],
         user_has_template_permission=user_has_template_permission,
-        default_letter_contact_block_id=default_letter_contact_block_id,
     )
 
 

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

@@ -13,7 +13,7 @@
 
   {{ page_header(
     'Add a new address',
-    back_link=None if request.args.get('from_template') else url_for('.service_letter_contact_details', service_id=current_service.id)
+    back_link=back_link
   ) }}
   <div class="grid-row">
     <div class="column-whole">

app/templates/views/templates/_template.html

@@ -49,7 +49,11 @@
     {% endif %}
     <a href="{{ url_for(".edit_template_postage", service_id=current_service.id, template_id=template.id) }}" class="edit-template-link-letter-postage">Change</a>
     <a href="{{ url_for(".edit_service_template", service_id=current_service.id, template_id=template.id) }}" class="edit-template-link-letter-body">Edit</a>
-    <a href="{{ url_for(".set_template_sender", service_id=current_service.id, template_id=template.id) }}" class="edit-template-link-letter-contact">Edit</a>
+    {% if current_service.count_letter_contact_details %}
+      <a href="{{ url_for(".set_template_sender", service_id=current_service.id, template_id=template.id) }}" class="edit-template-link-letter-contact">Edit</a>
+    {% else %}
+      <a href="{{ url_for(".service_add_letter_contact", service_id=current_service.id, from_template=template.id) }}" class="edit-template-link-letter-contact">Edit</a>
+    {% endif %}
   {% endif %}
   {{ template|string }}
 </div>

app/templates/views/templates/set-template-sender.html

@@ -23,9 +23,7 @@
           hide_legend=True
         ) }}
         {{ page_footer('Continue') }}
-        {% if no_senders %}
+        <a href="{{ url_for('.service_add_letter_contact', service_id=current_service.id, from_template=template_id) }}">Add new sender</a>
-          <a href="{{ url_for('.service_add_letter_contact', service_id=current_service.id, from_template=template_id) }}">Add new sender</a>
-        {% endif %}
       {% endcall %}
     </div>
   </div>

tests/app/main/views/test_service_settings.py

@@ -2168,26 +2168,45 @@ def test_add_letter_contact_when_coming_from_template(
     mock_add_letter_contact,
     fake_uuid,
     mock_get_service_letter_template,
+    mock_update_service_template_sender,
 ):
-    data = {
+    page = client_request.get(
-        'letter_contact_block': "1 Example Street"
-    }
-
-    page = client_request.post(
         'main.service_add_letter_contact',
         service_id=SERVICE_ONE_ID,
-        _data=data,
         from_template=fake_uuid,
-        _follow_redirects=True
+    )
+
+    assert page.select_one('.govuk-back-link')['href'] == url_for(
+        'main.view_template',
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+    )
+
+    client_request.post(
+        'main.service_add_letter_contact',
+        service_id=SERVICE_ONE_ID,
+        _data={
+            'letter_contact_block': '1 Example Street',
+        },
+        from_template=fake_uuid,
+        _expected_redirect=url_for(
+            'main.view_template',
+            service_id=SERVICE_ONE_ID,
+            template_id=fake_uuid,
+            _external=True,
+        ),
     )
 
     mock_add_letter_contact.assert_called_once_with(
         SERVICE_ONE_ID,
         contact_block="1 Example Street",
-        is_default=True
+        is_default=True,
+    )
+    mock_update_service_template_sender.assert_called_once_with(
+        SERVICE_ONE_ID,
+        fake_uuid,
+        '1234',
     )
-
-    assert page.find('h1').text == 'Set letter contact block'
 
 
 @pytest.mark.parametrize('fixture, data, api_default_args', [

tests/app/main/views/test_templates.py

@@ -27,6 +27,7 @@ from tests.conftest import (
     ElementNotFound,
     active_caseworking_user,
     active_user_view_permissions,
+    fake_uuid,
     mock_get_service_email_template,
     mock_get_service_letter_template,
     mock_get_service_template,
@@ -687,17 +688,26 @@ def test_should_show_sms_template_with_downgraded_unicode_characters(
     assert rendered_msg in page.text
 
 
+@pytest.mark.parametrize('mock_contact_block, expected_partial_url', (
+    (no_letter_contact_blocks, partial(
+        url_for, 'main.service_add_letter_contact', from_template=fake_uuid(),
+    )),
+    (single_letter_contact_block, partial(
+        url_for, 'main.set_template_sender', template_id=fake_uuid(),
+    )),
+))
 def test_should_let_letter_contact_block_be_changed_for_the_template(
     mocker,
     mock_get_service_letter_template,
     mock_get_template_folders,
-    no_letter_contact_blocks,
     client_request,
     service_one,
     fake_uuid,
+    mock_contact_block,
+    expected_partial_url
 ):
-    service_one['permissions'].append('letter')
     mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=1)
+    mock_contact_block(mocker)
 
     page = client_request.get(
         'main.view_template',
@@ -706,11 +716,9 @@ def test_should_let_letter_contact_block_be_changed_for_the_template(
         _test_page_title=False,
     )
 
-    assert page.find('a', {'class': 'edit-template-link-letter-contact'})['href'] == url_for(
+    assert page.select_one(
-        'main.set_template_sender',
+        'a.edit-template-link-letter-contact'
-        service_id=SERVICE_ONE_ID,
+    )['href'] == expected_partial_url(service_id=SERVICE_ONE_ID)
-        template_id=fake_uuid,
-    )
 
 
 def test_should_show_page_template_with_priority_select_if_platform_admin(
@@ -2153,16 +2161,15 @@ def test_set_template_sender(
     )
 
 
-@pytest.mark.parametrize('fixture, add_button_is_on_page', [
+@pytest.mark.parametrize('fixture', [
-    (no_letter_contact_blocks, True),
+    no_letter_contact_blocks,
-    (single_letter_contact_block, False),
+    single_letter_contact_block,
 ])
 def test_add_sender_link_only_appears_on_services_with_no_senders(
     client_request,
     fake_uuid,
     mocker,
     fixture,
-    add_button_is_on_page,
     mock_get_service_letter_template,
     no_letter_contact_blocks
 ):
@@ -2173,4 +2180,8 @@ def test_add_sender_link_only_appears_on_services_with_no_senders(
         template_id=fake_uuid,
     )
 
-    assert (page.select_one('.column-three-quarters form > a') is not None) == add_button_is_on_page
+    assert page.select_one('.column-three-quarters form > a')['href'] == url_for(
+        'main.service_add_letter_contact',
+        service_id=SERVICE_ONE_ID,
+        from_template=fake_uuid,
+    )

tests/conftest.py

@@ -270,7 +270,14 @@ def get_non_default_letter_contact_block(mocker):
 @pytest.fixture(scope='function')
 def mock_add_letter_contact(mocker):
     def _add_letter_contact(service_id, contact_block, is_default=False):
-        return
+        return {'data': {
+            'id': '1234',
+            'service_id': service_id,
+            'contact_block': '1 Example Street',
+            'is_default': True,
+            'created_at': str(datetime.utcnow()),
+            'updated_at': None
+        }}
 
     return mocker.patch('app.service_api_client.add_letter_contact', side_effect=_add_letter_contact)