Merge pull request #2230 from alphagov/add-branding-name-to-ui

Show the 'name' of the email branding in the adding and editing screens

app/main/forms.py

@@ -729,7 +729,8 @@ class ServiceSelectEmailBranding(StripWhitespaceForm):
 
 class ServiceUpdateEmailBranding(StripWhitespaceForm):
 
-    name = StringField('Name')
+    name = StringField('Name of brand')
+    text = StringField('Text')
     colour = StringField(
         'Colour',
         render_kw={'onchange': 'update_colour(this)'},
@@ -742,7 +743,8 @@ class ServiceUpdateEmailBranding(StripWhitespaceForm):
 
 class ServiceCreateEmailBranding(StripWhitespaceForm):
 
-    name = StringField('Name')
+    name = StringField('Name of brand')
+    text = StringField('Text')
     colour = StringField(
         'Colour',
         render_kw={'onchange': 'update_colour(this)'},

app/main/views/email_branding.py

@@ -78,12 +78,14 @@ def update_email_branding(branding_id, logo=None):
             branding_id=branding_id,
             logo=logo,
             name=form.name.data,
+            text=form.text.data,
             colour=form.colour.data
         )
 
         return redirect(url_for('.email_branding', branding_id=branding_id))
 
     form.name.data = email_branding['name']
+    form.text.data = email_branding['text']
     form.colour.data = email_branding['colour']
 
     return render_template(
@@ -124,6 +126,7 @@ def create_email_branding(logo=None):
         email_branding_client.create_email_branding(
             logo=logo,
             name=form.name.data,
+            text=form.text.data,
             colour=form.colour.data
         )
 

app/notify_client/email_branding_client.py

@@ -15,18 +15,20 @@ class EmailBrandingClient(NotifyAdminAPIClient):
     def get_letter_email_branding(self):
         return self.get(url='/dvla_organisations')
 
-    def create_email_branding(self, logo, name, colour):
+    def create_email_branding(self, logo, name, text, colour):
         data = {
             "logo": logo,
             "name": name,
+            "text": text,
             "colour": colour
         }
         return self.post(url="/email-branding", data=data)
 
-    def update_email_branding(self, branding_id, logo, name, colour):
+    def update_email_branding(self, branding_id, logo, name, text, colour):
         data = {
             "logo": logo,
             "name": name,
+            "text": text,
             "colour": colour
         }
         return self.post(url="/email-branding/{}".format(branding_id), data=data)

app/templates/components/radios.html

@@ -91,51 +91,6 @@
 {% endmacro %}
 
 
-{% macro branding_radios(
-  field,
-  hint=None,
-  branding_dict={},
-  show_header=True
-) %}
-  <div class="form-group {% if field.errors %} form-group-error{% endif %}">
-    <fieldset>
-      <legend class="form-label">
-        {% if show_header %}
-          {{ field.label.text }}
-        {% endif %}
-        {% if field.errors %}
-            <span class="error-message" data-module="track-error" data-error-type="{{ field.errors[0] }}" data-error-label="{{ field.name }}">
-              {{ field.errors[0] }}
-            </span>
-        {% endif %}
-      </legend>
-      {% for value, option, checked in field.iter_choices() %}
-        <div class="multiple-choice branding-radio">
-          <input
-            type="radio"
-            name="{{ field.name }}"
-            id="{{ field.name }}-{{ loop.index }}"
-            value="{{ value }}"
-            {% if checked %}checked="checked"{% endif %}
-          />
-          <label class="block-label" for="{{ field.name }}-{{ loop.index }}">
-            {% if branding_dict.get(value, {}).get('colour') %}
-              <span style="background: {{ branding_dict[value].colour }}; display: inline-block; width: 3px; height: 27px;"></span>
-            {% endif %}
-            {% if branding_dict.get(value, {}).get('logo') %}
-              <img
-                src="{{ branding_dict[value].logo }}"
-                height="27"
-              />
-            {% endif %}
-            {{option}}
-          </label>
-        </div>
-      {% endfor %}
-    </fieldset>
-  </div>
-{% endmacro %}
-
 {% macro conditional_radio_panel(id) %}
   <div class="conditional-radios-panel" id="panel-{{ id }}">
     {{ caller() }}

app/templates/views/email-branding/manage-branding.html

@@ -22,6 +22,7 @@
       <form method="post">
         <div class="form-group">
           <div style='margin-top:15px;'>{{textbox(form.name)}}</div>
+          <div style='margin-top:15px;'>{{textbox(form.text)}}</div>
           {{colour_textbox(form.colour, width='1-4', colour=email_branding.colour if email_branding)}}
           {{ page_footer(
             'Save',

app/templates/views/email-branding/select-branding.html

@@ -1,5 +1,5 @@
 {% extends "views/platform-admin/_base_template.html" %}
-{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}
@@ -15,7 +15,7 @@
   <div class="grid-row">
     <div class="column-three-quarters">
       <form method="post">
-        {{ branding_radios(form.email_branding, branding_dict=branding_dict, show_header=False) }}
+        {{ radios(form.email_branding) }}
         {{ page_footer(
           'Next'
         ) }}

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

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}
@@ -15,7 +15,7 @@
         {{ radios(form.branding_type) }}
       </div>
       <div class="column-one-half">
-        {{ branding_radios(form.branding_style, branding_dict=branding_dict) }}
+        {{ radios(form.branding_style) }}
       </div>
     </div>
     <div class="grid-row">

app/templates/views/service-settings/set-letter-branding.html

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}

app/templates/views/service-settings/set-organisation-type.html

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}

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

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}

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

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/radios.html" import radios, branding_radios %}
+{% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
 
 {% block service_page_title %}

tests/app/main/views/test_email_branding.py

@@ -26,9 +26,7 @@ def test_email_branding_page_shows_full_branding_list(
     ) == "Select an email branding to update or create a new email branding"
 
     first_label = page.select('div.multiple-choice > label')[0]
-    assert 'background: red;' in first_label.find('span')['style']
     assert normalize_spaces(first_label.text) == 'org 1'
-    assert first_label.find('img')['src'].endswith('/logo1.png')
 
     assert normalize_spaces((page.select('div.multiple-choice > label')[-1]).text) == 'Create a new email branding'
 
@@ -47,6 +45,7 @@ def test_edit_email_branding_shows_the_correct_branding_info(
 
     assert page.select_one('#logo-img > img')['src'].endswith('/example.png')
     assert page.select_one('#name').attrs.get('value') == 'Organisation name'
+    assert page.select_one('#text').attrs.get('value') == 'Organisation text'
     assert page.select_one('#colour').attrs.get('value') == '#f00'
 
 
@@ -64,6 +63,7 @@ def test_create_email_branding_does_not_show_any_branding_info(
 
     assert page.select_one('#logo-img > img') is None
     assert page.select_one('#name').attrs.get('value') == ''
+    assert page.select_one('#text').attrs.get('value') == ''
     assert page.select_one('#colour').attrs.get('value') == ''
 
 
@@ -76,7 +76,8 @@ def test_create_new_email_branding_without_logo(
     data = {
         'logo': None,
         'colour': '#ff0000',
-        'name': 'new name'
+        'text': 'new text',
+        'name': 'new name',
     }
 
     mock_persist = mocker.patch('app.main.views.email_branding.persist_logo')
@@ -92,6 +93,7 @@ def test_create_new_email_branding_without_logo(
     assert mock_create_email_branding.call_args == call(
         logo=data['logo'],
         name=data['name'],
+        text=data['text'],
         colour=data['colour']
     )
     assert mock_persist.call_args_list == []
@@ -109,7 +111,8 @@ def test_create_new_email_branding_when_branding_saved(
     data = {
         'logo': 'test.png',
         'colour': '#ff0000',
-        'name': 'new name'
+        'text': 'new text',
+        'name': 'new name',
     }
 
     temp_filename = LOGO_LOCATION_STRUCTURE.format(
@@ -127,6 +130,7 @@ def test_create_new_email_branding_when_branding_saved(
         data={
             'colour': data['colour'],
             'name': data['name'],
+            'text': data['text'],
             'cdn_url': 'https://static-logos.cdn.com'
         }
     )
@@ -135,6 +139,7 @@ def test_create_new_email_branding_when_branding_saved(
     assert mock_create_email_branding.call_args == call(
         logo=data['logo'],
         name=data['name'],
+        text=data['text'],
         colour=data['colour']
     )
 
@@ -199,6 +204,7 @@ def test_update_exisiting_branding(
     data = {
         'logo': 'test.png',
         'colour': '#0000ff',
+        'text': 'new text',
         'name': 'new name'
     }
 
@@ -214,7 +220,8 @@ def test_update_exisiting_branding(
     logged_in_platform_admin_client.post(
         url_for('.update_email_branding', logo=temp_filename, branding_id=fake_uuid),
         content_type='multipart/form-data',
-        data={'colour': data['colour'], 'name': data['name'], 'cdn_url': 'https://static-logos.cdn.com'}
+        data={'colour': data['colour'], 'name': data['name'], 'text': data['text'],
+              'cdn_url': 'https://static-logos.cdn.com'}
     )
 
     assert mock_update_email_branding.called
@@ -222,6 +229,7 @@ def test_update_exisiting_branding(
         branding_id=fake_uuid,
         logo=data['logo'],
         name=data['name'],
+        text=data['text'],
         colour=data['colour']
     )
 
@@ -303,6 +311,7 @@ def test_colour_regex_validation(
     data = {
         'logo': None,
         'colour': colour_hex,
+        'text': 'new text',
         'name': 'new name'
     }
 

tests/app/notify_client/test_email_branding_client.py

@@ -26,10 +26,11 @@ def test_get_letter_email_branding(mocker):
 
 
 def test_create_email_branding(mocker):
-    org_data = {'logo': 'test.png', 'name': 'test name', 'colour': 'red'}
+    org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red'}
 
     mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post')
-    EmailBrandingClient().create_email_branding(logo=org_data['logo'], name=org_data['name'], colour=org_data['colour'])
+    EmailBrandingClient().create_email_branding(
+        logo=org_data['logo'], name=org_data['name'], text=org_data['text'], colour=org_data['colour'])
 
     mock_post.assert_called_once_with(
         url='/email-branding',
@@ -38,11 +39,12 @@ def test_create_email_branding(mocker):
 
 
 def test_update_email_branding(mocker, fake_uuid):
-    org_data = {'logo': 'test.png', 'name': 'test name', 'colour': 'red'}
+    org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red'}
 
     mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post')
     EmailBrandingClient().update_email_branding(
-        branding_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], colour=org_data['colour'])
+        branding_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], text=org_data['text'],
+        colour=org_data['colour'])
 
     mock_post.assert_called_once_with(
         url='/email-branding/{}'.format(fake_uuid),

tests/conftest.py

@@ -2432,11 +2432,11 @@ def mock_send_already_registered_email(mocker):
 def mock_get_all_email_branding(mocker):
     def _get_all_email_branding():
         return [
-            {'id': '1', 'name': 'org 1', 'colour': 'red', 'logo': 'logo1.png'},
-            {'id': '2', 'name': 'org 2', 'colour': 'orange', 'logo': 'logo2.png'},
-            {'id': '3', 'name': None, 'colour': None, 'logo': 'logo3.png'},
-            {'id': '4', 'name': 'org 4', 'colour': None, 'logo': 'logo4.png'},
-            {'id': '5', 'name': None, 'colour': 'blue', 'logo': 'logo5.png'},
+            {'id': '1', 'name': 'org 1', 'text': 'org 1', 'colour': 'red', 'logo': 'logo1.png'},
+            {'id': '2', 'name': 'org 2', 'text': 'org 2', 'colour': 'orange', 'logo': 'logo2.png'},
+            {'id': '3', 'name': 'org 3', 'text': None, 'colour': None, 'logo': 'logo3.png'},
+            {'id': '4', 'name': 'org 4', 'text': 'org 4', 'colour': None, 'logo': 'logo4.png'},
+            {'id': '5', 'name': 'org 5', 'text': None, 'colour': 'blue', 'logo': 'logo5.png'},
         ]
 
     return mocker.patch(
@@ -2474,6 +2474,7 @@ def mock_get_email_branding(mocker, fake_uuid):
             'email_branding': {
                 'logo': 'example.png',
                 'name': 'Organisation name',
+                'text': 'Organisation text',
                 'id': fake_uuid,
                 'colour': '#f00'
             }
@@ -2486,7 +2487,7 @@ def mock_get_email_branding(mocker, fake_uuid):
 
 @pytest.fixture(scope='function')
 def mock_create_email_branding(mocker):
-    def _create_email_branding(logo, name, colour):
+    def _create_email_branding(logo, name, text, colour):
         return
 
     return mocker.patch(
@@ -2496,7 +2497,7 @@ def mock_create_email_branding(mocker):
 
 @pytest.fixture(scope='function')
 def mock_update_email_branding(mocker):
-    def _update_email_branding(branding_id, logo, name, colour):
+    def _update_email_branding(branding_id, logo, name, text, colour):
         return
 
     return mocker.patch(