From 61470391bf1fa3f51a4b943b0e17303d63c04d36 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 19 Apr 2017 16:11:28 +0100
Subject: [PATCH] Add platform admin setting for letter branding
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Does two main things:
- defines what ‘brands’ we support, in terms of the ID that DVLA use
- adds a form to choose which branding a service uses (currently
  platform admin only, like email branding)

By doing this we will be able to (with some more work) preview and send
letters with a variety of different branding.

Story: https://www.pivotaltracker.com/story/show/143506905
---
 app/config.py                                 |  5 ++
 app/main/forms.py                             | 14 ++++
 app/main/views/service_settings.py            | 30 ++++++-
 app/notify_client/service_api_client.py       |  3 +-
 app/templates/views/service-settings.html     |  5 ++
 .../service-settings/set-letter-branding.html | 25 ++++++
 tests/app/main/views/test_service_settings.py | 78 +++++++++++++++++--
 7 files changed, 149 insertions(+), 11 deletions(-)
 create mode 100644 app/templates/views/service-settings/set-letter-branding.html

diff --git a/app/config.py b/app/config.py
index 53eb1e8ac..5899d6880 100644
--- a/app/config.py
+++ b/app/config.py
@@ -83,6 +83,11 @@ class Config(object):
         r"assembly\.wales",
     ]
 
+    LETTER_BRANDING = [
+        ('001', 'HM Government'),
+        ('500', 'Land Registry'),
+    ]
+
 
 class Development(Config):
     DEBUG = True
diff --git a/app/main/forms.py b/app/main/forms.py
index d303b3673..60ee4df05 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -538,6 +538,20 @@ class ServiceBrandingOrg(Form):
     )
 
 
+class LetterBranding(Form):
+
+    def __init__(self, choices=[], *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.dvla_org_id.choices = choices
+
+    dvla_org_id = RadioField(
+        'Which logo should this service’s letter have?',
+        validators=[
+            DataRequired()
+        ]
+    )
+
+
 class Whitelist(Form):
 
     def populate(self, email_addresses, phone_numbers):
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index abc6dd620..3441c3c66 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -26,7 +26,8 @@ from app.main.forms import (
     ServiceReplyToEmailFrom,
     ServiceSmsSender,
     ServiceLetterContactBlock,
-    ServiceBrandingOrg
+    ServiceBrandingOrg,
+    LetterBranding,
 )
 from app import user_api_client, current_service, organisations_client
 
@@ -41,7 +42,10 @@ def service_settings(service_id):
         organisation = None
     return render_template(
         'views/service-settings.html',
-        organisation=organisation
+        organisation=organisation,
+        letter_branding=dict(
+            current_app.config['LETTER_BRANDING']
+        ).get(current_service.get('dvla_org_id', '001'))
     )
 
 
@@ -314,6 +318,28 @@ def service_set_branding_and_org(service_id):
     )
 
 
+@main.route("/services/<service_id>/service-settings/set-letter-branding", methods=['GET', 'POST'])
+@login_required
+@user_has_permissions(admin_override=True)
+def set_letter_branding(service_id):
+
+    form = LetterBranding(choices=current_app.config['LETTER_BRANDING'])
+
+    if form.validate_on_submit():
+        service_api_client.update_service(
+            service_id,
+            dvla_organisation=form.dvla_org_id.data
+        )
+        return redirect(url_for('.service_settings', service_id=service_id))
+
+    form.dvla_org_id.data = current_service.get('dvla_organisation', '001')
+
+    return render_template(
+        'views/service-settings/set-letter-branding.html',
+        form=form,
+    )
+
+
 def get_branding_as_value_and_label(organisations):
     return [
         (organisation['id'], organisation['name'])
diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py
index 72d489a2d..d172176cd 100644
--- a/app/notify_client/service_api_client.py
+++ b/app/notify_client/service_api_client.py
@@ -91,7 +91,8 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             'created_by',
             'branding',
             'organisation',
-            'letter_contact_block'
+            'letter_contact_block',
+            'dvla_org_id',
         }
         if disallowed_attributes:
             raise TypeError('Not allowed to update service attributes: {}'.format(
diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html
index b25b93875..78f0db7a1 100644
--- a/app/templates/views/service-settings.html
+++ b/app/templates/views/service-settings.html
@@ -103,6 +103,11 @@
           {% endcall %}
           {{ edit_field('Change', url_for('.service_set_branding_and_org', service_id=current_service.id)) }}
         {% endcall %}
+        {% call row() %}
+          {{ text_field('Letter branding')}}
+          {{ text_field(letter_branding) }}
+          {{ edit_field('Change', url_for('.set_letter_branding', service_id=current_service.id)) }}
+        {% endcall %}
       {% endcall %}
 
       <ul>
diff --git a/app/templates/views/service-settings/set-letter-branding.html b/app/templates/views/service-settings/set-letter-branding.html
new file mode 100644
index 000000000..ecee934c1
--- /dev/null
+++ b/app/templates/views/service-settings/set-letter-branding.html
@@ -0,0 +1,25 @@
+{% extends "withnav_template.html" %}
+{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block service_page_title %}
+  Set letter branding
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">Set letter branding</h1>
+  <div class="grid-row">
+    <div class="column-three-quarters">
+      <form method="post">
+        {{ radios(form.dvla_org_id) }}
+        {{ page_footer(
+          'Save',
+          back_link=url_for('.service_settings', service_id=current_service.id),
+          back_link_text='Back to settings'
+        ) }}
+      </form>
+    </div>
+  </div>
+
+{% endblock %}
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index dfcbac884..435ec1257 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -10,23 +10,45 @@ import app
 from app.utils import email_safe
 from tests import validate_route_permission, service_json
 
+from tests.conftest import active_user_with_permissions, platform_admin_user
 
+
+@pytest.mark.parametrize('user, expected_rows', [
+    (active_user_with_permissions, [
+        'Label Value Action',
+        'Service name service one Change',
+        'Email reply to address None Change',
+        'Text message sender 40604 Change'
+    ]),
+    (platform_admin_user, [
+        'Label Value Action',
+        'Service name service one Change',
+        'Email reply to address None Change',
+        'Text message sender 40604 Change',
+        'Label Value Action',
+        'Email branding GOV.UK Change',
+        'Letter branding HM Government Change',
+    ]),
+])
 def test_should_show_overview(
-    logged_in_client,
+    client,
+    mocker,
     service_one,
+    fake_uuid,
+    user,
+    expected_rows,
 ):
-    response = logged_in_client.get(url_for(
+    client.login(user(fake_uuid), mocker, service_one)
+    response = client.get(url_for(
         'main.service_settings', service_id=service_one['id']
     ))
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
     assert page.find('h1').text == 'Settings'
-    for index, row in enumerate([
-        'Service name service one Change',
-        'Email reply to address None Change',
-        'Text message sender 40604 Change'
-    ]):
-        assert row == " ".join(page.find_all('tr')[index + 1].text.split())
+    rows = page.find_all('tr')
+    assert len(rows) == len(expected_rows)
+    for index, row in enumerate(expected_rows):
+        assert row == " ".join(rows[index].text.split())
     app.service_api_client.get_service.assert_called_with(service_one['id'])
 
 
@@ -660,6 +682,46 @@ def test_set_letter_contact_block_has_max_10_lines(
     assert error_message == 'Contains 11 lines, maximum is 10'
 
 
+def test_set_letter_branding_platform_admin_only(
+    logged_in_client,
+    service_one,
+):
+    response = logged_in_client.get(url_for('main.set_letter_branding', service_id=service_one['id']))
+    assert response.status_code == 403
+
+
+@pytest.mark.parametrize('current_dvla_org_id, expected_selected', [
+    (None, '001'),
+    ('500', '500'),
+])
+def test_set_letter_branding_prepopulates(
+    logged_in_platform_admin_client,
+    service_one,
+    current_dvla_org_id,
+    expected_selected,
+):
+    if current_dvla_org_id:
+        service_one['dvla_organisation'] = current_dvla_org_id
+    response = logged_in_platform_admin_client.get(url_for('main.set_letter_branding', service_id=service_one['id']))
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert page.select('input[checked]')[0]['value'] == expected_selected
+
+
+def test_set_letter_contact_block_saves(
+    logged_in_platform_admin_client,
+    service_one,
+    mock_update_service,
+):
+    response = logged_in_platform_admin_client.post(
+        url_for('main.set_letter_branding', service_id=service_one['id']),
+        data={'dvla_org_id': '500'}
+    )
+    assert response.status_code == 302
+    assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True)
+    mock_update_service.assert_called_once_with(service_one['id'], dvla_organisation='500')
+
+
 def test_should_show_branding(
     logged_in_platform_admin_client,
     service_one,