Merge pull request #4163 from alphagov/branding-previews

Show a preview of GOV.UK and NHS email branding and apply straight away

app/main/views/service_settings.py

@@ -80,6 +80,8 @@ PLATFORM_ADMIN_SERVICE_PERMISSIONS = OrderedDict([
     ('international_letters', {'title': 'Send international letters', 'requires': 'letter'}),
 ])
 
+NHS_BRANDING_ID = 'a7dc4e56-660b-4db7-8cff-12c37b12b5ea'
+
 
 @main.route("/services/<uuid:service_id>/service-settings")
 @user_has_permissions('manage_service', 'manage_api_keys')
@@ -1196,9 +1198,9 @@ def email_branding_govuk(service_id):
     check_branding_allowed_for_service('govuk')
 
     if request.method == 'POST':
-        create_email_branding_zendesk_ticket(request.form['branding_choice'])
+        current_service.update(email_branding=None)
 
-        flash('Thanks for your branding request. We’ll get back to you within one working day.', 'default')
+        flash('You’ve updated your email branding', 'default')
         return redirect(url_for('.service_settings', service_id=current_service.id))
 
     return render_template('views/service-settings/branding/email-branding-govuk.html')
@@ -1210,12 +1212,12 @@ def email_branding_govuk_and_org(service_id):
     check_branding_allowed_for_service('govuk_and_org')
 
     if request.method == 'POST':
-        create_email_branding_zendesk_ticket(request.form['branding_choice'])
+        create_email_branding_zendesk_ticket('govuk_and_org')
 
         flash('Thanks for your branding request. We’ll get back to you within one working day.', 'default')
         return redirect(url_for('.service_settings', service_id=current_service.id))
 
-    return render_template('views/service-settings/branding/email-branding-govuk.html', with_org=True)
+    return render_template('views/service-settings/branding/email-branding-govuk-org.html')
 
 
 @main.route("/services/<uuid:service_id>/service-settings/email-branding/nhs", methods=['GET', 'POST'])
@@ -1224,12 +1226,12 @@ def email_branding_nhs(service_id):
     check_branding_allowed_for_service('nhs')
 
     if request.method == 'POST':
-        create_email_branding_zendesk_ticket('nhs')
+        current_service.update(email_branding=NHS_BRANDING_ID)
 
-        flash('Thanks for your branding request. We’ll get back to you within one working day.', 'default')
+        flash('You’ve updated your email branding', 'default')
         return redirect(url_for('.service_settings', service_id=current_service.id))
 
-    return render_template('views/service-settings/branding/email-branding-nhs.html')
+    return render_template('views/service-settings/branding/email-branding-nhs.html', nhs_branding_id=NHS_BRANDING_ID)
 
 
 @main.route("/services/<uuid:service_id>/service-settings/email-branding/organisation", methods=['GET', 'POST'])

app/navigation.py

@@ -220,6 +220,7 @@ class MainNavigation(Navigation):
             'add_organisation_from_gp_service',
             'add_organisation_from_nhs_local_service',
             'email_branding_govuk',
+            'email_branding_govuk_and_org',
             'email_branding_nhs',
             'email_branding_organisation',
             'email_branding_request',

app/templates/views/service-settings/branding/email-branding-govuk-org.html

@@ -0,0 +1,36 @@
+{% extends "withnav_template.html" %}
+{% from "components/form.html" import form_wrapper %}
+{% from "components/back-link/macro.njk" import govukBackLink %}
+{% from "components/page-footer.html" import page_footer %}
+{% from "components/page-header.html" import page_header %}
+
+{% block service_page_title %}
+  Before you request new branding
+{% endblock %}
+
+{% block backLink %}
+  {{ govukBackLink({
+    "href": url_for('.email_branding_request', service_id=current_service.id)
+  }) }}
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  {{ page_header('Before you request new branding') }}
+
+  <p class="govuk-body">You can only use GOV.UK branding if people go to GOV.UK to access your service.</p>
+
+  <p class="govuk-body">
+    You cannot use GOV.UK branding if your organisation is
+    <a class="govuk-link govuk-link--no-visited-state"
+      href="https://www.gov.uk/government/publications/govuk-proposition/govuk-proposition#organisations-independent-from-government">independent
+      from government</a>.
+  </p>
+
+  <p class="govuk-body">We’ll email you once your branding’s ready to use, or if we need any more information.</p>
+
+  {% call form_wrapper() %}
+    {{ page_footer('Request new branding') }}
+  {% endcall %}
+
+{% endblock %}

app/templates/views/service-settings/branding/email-branding-govuk.html

@@ -5,7 +5,7 @@
 {% from "components/page-header.html" import page_header %}
 
 {% block service_page_title %}
-  Before you request new branding
+  Check your new branding
 {% endblock %}
 
 {% block backLink %}
@@ -16,16 +16,17 @@
 
 {% block maincolumn_content %}
 
-  {{ page_header('Before you request new branding') }}
+  {{ page_header('Check your new branding') }}
 
-  <p class="govuk-body">Check that your new branding matches the rest of your service.</p>
+  <p class="govuk-body">
+    Emails from {{ current_service.name }} will look like this.
+  </p>
 
-  <p class="govuk-body">You can use the GOV.UK logo on your emails if:</p>
-  <ul class="list list-bullet">
-    <li>your website looks like GOV.UK</li>
-    <li>your email links to a website that looks like GOV.UK</li>
-    <li>people get an email from your service after using GOV.UK</li>
-  </ul>
+  <iframe src="{{ url_for('main.email_template', branding_style='__NONE__') }}" class="branding-preview"></iframe>
+
+  <h2 class="heading-medium">Before you continue</h2>
+
+  <p class="govuk-body">You can only use GOV.UK branding if people go to GOV.UK to access your service.</p>
 
   <p class="govuk-body">
     You cannot use GOV.UK branding if your organisation is
@@ -34,10 +35,8 @@
       from government</a>.
   </p>
 
-  <p class="govuk-body">We’ll email you once your branding’s ready to use, or if we need any more information.</p>
-
   {% call form_wrapper() %}
-    {{ page_footer('Request new branding', button_name='branding_choice', button_value=('govuk_and_org' if with_org else 'govuk')) }}
+    {{ page_footer('Use this branding') }}
   {% endcall %}
 
 {% endblock %}

app/templates/views/service-settings/branding/email-branding-nhs.html

@@ -5,7 +5,7 @@
 {% from "components/page-header.html" import page_header %}
 
 {% block service_page_title %}
-  Before you request new branding
+  Check your new branding
 {% endblock %}
 
 {% block backLink %}
@@ -16,18 +16,24 @@
 
 {% block maincolumn_content %}
 
-  {{ page_header('Before you request new branding') }}
+  {{ page_header('Check your new branding') }}
 
   <p class="govuk-body">
-    <a class="govuk-link govuk-link--no-visited-state" href="https://www.england.nhs.uk/nhsidentity/identity-guidelines/who-can-use-the-nhs-identity/">Check that your service is allowed to use the NHS identity</a>.
+    Emails from {{ current_service.name }} will look like this.
   </p>
 
-  <p class="govuk-body">Your new branding should match the rest of your service.</p>
+  <iframe src="{{ url_for('main.email_template', branding_style=nhs_branding_id) }}" class="branding-preview"></iframe>
 
-  <p class="govuk-body">We’ll email you once your branding’s ready to use, or if we need any more information.</p>
+  <h2 class="heading-medium">Before you continue</h2>
+
+  <p class="govuk-body">Make sure you’re allowed to use NHS branding.</p>
+
+  <p class="govuk-body">
+    If you’re not sure, <a class="govuk-link govuk-link--no-visited-state" href="https://www.england.nhs.uk/nhsidentity/identity-guidelines/who-can-use-the-nhs-identity/">check the guidance on the NHS website</a>.
+  </p>
 
   {% call form_wrapper() %}
-    {{ page_footer('Request new branding') }}
+    {{ page_footer('Use this branding') }}
   {% endcall %}
 
 {% endblock %}

app/templates/views/service-settings/branding/email-branding-options.html

@@ -25,9 +25,12 @@
     Your emails currently have {{ branding_name }} branding.
   </p>
 
+  {% set branding_id = current_service.email_branding_id if current_service.email_branding else '__NONE__' %}
+  <iframe src="{{ url_for('main.email_template', branding_style=branding_id) }}" class="branding-preview"></iframe>
+
   {% if current_service.needs_to_change_email_branding %}
     <p class="govuk-body">
-      You should be using your own branding instead. We can help you to set this up.
+      You should be using your own branding instead.
     </p>
   {% endif %}
 

tests/app/main/views/test_service_settings.py

@@ -14,6 +14,7 @@ from notifications_utils.clients.zendesk.zendesk_client import (
 )
 
 import app
+from app.main.views.service_settings import NHS_BRANDING_ID
 from tests import (
     find_element_by_tag_and_partial_text,
     invite_json,
@@ -4771,6 +4772,7 @@ def test_update_service_organisation_does_not_update_if_same_value(
 def test_show_email_branding_request_page_when_no_branding_is_set(
     service_one,
     client_request,
+    mocker,
     mock_get_email_branding,
     mock_get_letter_branding_by_id,
     organisation_type,
@@ -4779,11 +4781,18 @@ def test_show_email_branding_request_page_when_no_branding_is_set(
     service_one['email_branding'] = None
     service_one['organisation_type'] = organisation_type
 
+    mocker.patch(
+        'app.models.service.Service.email_branding_id',
+        new_callable=PropertyMock,
+        return_value=None,
+    )
+
     page = client_request.get(
         '.email_branding_request', service_id=SERVICE_ONE_ID
     )
 
     assert mock_get_email_branding.called is False
+    assert page.find('iframe')['src'] == url_for('main.email_template', branding_style='__NONE__')
     assert mock_get_letter_branding_by_id.called is False
 
     button_text = normalize_spaces(page.select_one('.page-footer button').text)
@@ -4983,17 +4992,22 @@ def test_show_email_branding_request_page_when_email_branding_is_set(
     client_request,
     mock_get_email_branding,
     mock_get_service_organisation,
-    active_user_with_permissions,
 ):
     service_one['email_branding'] = sample_uuid()
     mocker.patch(
         'app.organisations_client.get_organisation',
         return_value=organisation_json(),
     )
+    mocker.patch(
+        'app.models.service.Service.email_branding_id',
+        new_callable=PropertyMock,
+        return_value='1234-abcd',
+    )
 
     page = client_request.get(
         '.email_branding_request', service_id=SERVICE_ONE_ID
     )
+    assert page.find('iframe')['src'] == url_for('main.email_template', branding_style='1234-abcd')
     assert [
         (
             radio['value'],
@@ -5432,24 +5446,19 @@ def test_submit_branding_when_something_else_is_only_option(
     ) in mock_create_ticket.call_args_list[0][1]['message']
 
 
-@pytest.mark.parametrize('endpoint, service_org_type, expected_heading', [
-    ('main.email_branding_govuk', 'central', 'Before you request new branding'),
-    ('main.email_branding_govuk_and_org', 'central', 'Before you request new branding'),
-    ('main.email_branding_govuk', 'central', 'Before you request new branding'),
-    ('main.email_branding_nhs', 'nhs_local', 'Before you request new branding'),
-    ('main.email_branding_organisation', 'central', 'When you request new branding'),
+@pytest.mark.parametrize('endpoint, expected_heading', [
+    ('main.email_branding_govuk_and_org', 'Before you request new branding'),
+    ('main.email_branding_organisation', 'When you request new branding'),
 ])
-def test_get_email_branding_description_pages(
+def test_get_email_branding_description_pages_for_org_branding(
     client_request,
     mocker,
     service_one,
     organisation_one,
     mock_get_email_branding,
     endpoint,
-    service_org_type,
     expected_heading,
 ):
-    organisation_one['organisation_type'] = service_org_type
     service_one['email_branding'] = sample_uuid()
     service_one['organisation'] = organisation_one
 
@@ -5466,6 +5475,39 @@ def test_get_email_branding_description_pages(
     assert normalize_spaces(page.select_one('.page-footer button').text) == 'Request new branding'
 
 
+@pytest.mark.parametrize('endpoint, service_org_type, branding_preview_id', [
+    ('main.email_branding_govuk', 'central', '__NONE__'),
+    ('main.email_branding_nhs', 'nhs_local', NHS_BRANDING_ID),
+])
+def test_get_email_branding_govuk_and_nhs_pages(
+    client_request,
+    mocker,
+    service_one,
+    organisation_one,
+    mock_get_email_branding,
+    endpoint,
+    service_org_type,
+    branding_preview_id,
+):
+    organisation_one['organisation_type'] = service_org_type
+    service_one['email_branding'] = sample_uuid()
+    service_one['organisation'] = organisation_one
+
+    mocker.patch(
+        'app.organisations_client.get_organisation',
+        return_value=organisation_one,
+    )
+
+    page = client_request.get(
+        endpoint,
+        service_id=SERVICE_ONE_ID,
+    )
+    assert page.h1.text == 'Check your new branding'
+    assert 'Emails from service one will look like this' in normalize_spaces(page.text)
+    assert page.find('iframe')['src'] == url_for('main.email_template', branding_style=branding_preview_id)
+    assert normalize_spaces(page.select_one('.page-footer button').text) == 'Use this branding'
+
+
 def test_get_email_branding_something_else_page(client_request):
     page = client_request.get(
         'main.email_branding_something_else',
@@ -5495,11 +5537,43 @@ def test_get_email_branding_description_pages_give_404_if_selected_branding_not_
     )
 
 
-@pytest.mark.parametrize('branding_choice, branding_description', [
-    ('govuk', 'GOV.UK'),
-    ('govuk_and_org', 'GOV.UK and organisation one'),
-])
-def test_submit_email_branding_request_from_govuk_description_page(
+def test_update_email_branding_from_govuk_preview_page(
+    mocker,
+    client_request,
+    service_one,
+    organisation_one,
+    mock_get_service_settings_page_common,
+    no_reply_to_email_addresses,
+    mock_get_email_branding,
+    single_sms_sender,
+    mock_update_service,
+):
+    mocker.patch(
+        'app.organisations_client.get_organisation',
+        return_value=organisation_one,
+    )
+    mocker.patch(
+        'app.models.service.Service.organisation_id',
+        new_callable=PropertyMock,
+        return_value=ORGANISATION_ID,
+    )
+    service_one['email_branding'] = sample_uuid()
+
+    page = client_request.post(
+        '.email_branding_govuk',
+        service_id=SERVICE_ONE_ID,
+        _follow_redirects=True,
+    )
+
+    mock_update_service.assert_called_once_with(
+        SERVICE_ONE_ID,
+        email_branding=None,
+    )
+    assert page.h1.text == 'Settings'
+    assert normalize_spaces(page.select_one('.banner-default').text) == 'You’ve updated your email branding'
+
+
+def test_submit_email_branding_request_from_govuk_and_org_description_page(
     mocker,
     client_request,
     service_one,
@@ -5508,8 +5582,6 @@ def test_submit_email_branding_request_from_govuk_description_page(
     no_reply_to_email_addresses,
     mock_get_email_branding,
     single_sms_sender,
-    branding_choice,
-    branding_description,
 ):
     mocker.patch(
         'app.organisations_client.get_organisation',
@@ -5529,9 +5601,8 @@ def test_submit_email_branding_request_from_govuk_description_page(
     )
 
     page = client_request.post(
-        '.email_branding_govuk',
+        '.email_branding_govuk_and_org',
         service_id=SERVICE_ONE_ID,
-        _data={'branding_choice': branding_choice},
         _follow_redirects=True,
     )
 
@@ -5544,7 +5615,7 @@ def test_submit_email_branding_request_from_govuk_description_page(
             '',
             '---',
             'Current branding: Organisation name',
-            f'Branding requested: {branding_description}\n',
+            'Branding requested: GOV.UK and organisation one\n',
         ]),
         subject='Email branding request - service one',
         ticket_type='question',
@@ -5561,7 +5632,7 @@ def test_submit_email_branding_request_from_govuk_description_page(
     )
 
 
-def test_submit_email_branding_request_from_nhs_description_page(
+def test_update_email_branding_from_nhs_preview_page(
     mocker,
     client_request,
     service_one,
@@ -5570,46 +5641,23 @@ def test_submit_email_branding_request_from_nhs_description_page(
     no_reply_to_email_addresses,
     mock_get_email_branding,
     single_sms_sender,
+    mock_update_service,
 ):
     service_one['email_branding'] = sample_uuid()
     service_one['organisation_type'] = 'nhs_local'
 
-    mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__')
-    mock_send_ticket_to_zendesk = mocker.patch(
-        'app.main.views.service_settings.zendesk_client.send_ticket_to_zendesk',
-        autospec=True,
-    )
-
     page = client_request.post(
         '.email_branding_nhs',
         service_id=SERVICE_ONE_ID,
         _follow_redirects=True,
     )
 
-    mock_create_ticket.assert_called_once_with(
-        ANY,
-        message='\n'.join([
-            'Organisation: Can’t tell (domain is user.gov.uk)',
-            'Service: service one',
-            'http://localhost/services/596364a0-858e-42c8-9062-a8fe822260eb',
-            '',
-            '---',
-            'Current branding: Organisation name',
-            'Branding requested: NHS\n',
-        ]),
-        subject='Email branding request - service one',
-        ticket_type='question',
-        user_name='Test User',
-        user_email='test@user.gov.uk',
-        org_id=None,
-        org_type='nhs_local',
-        service_id=SERVICE_ONE_ID
-    )
-    mock_send_ticket_to_zendesk.assert_called_once()
-    assert normalize_spaces(page.select_one('.banner-default').text) == (
-        'Thanks for your branding request. We’ll get back to you '
-        'within one working day.'
+    mock_update_service.assert_called_once_with(
+        SERVICE_ONE_ID,
+        email_branding=NHS_BRANDING_ID,
     )
+    assert page.h1.text == 'Settings'
+    assert normalize_spaces(page.select_one('.banner-default').text) == 'You’ve updated your email branding'
 
 
 def test_submit_email_branding_request_from_organisation_description_page(