Merge branch 'master' into add-brand-type-to-email-branding

app/main/forms.py

@@ -713,20 +713,6 @@ class ServicePreviewBranding(StripWhitespaceForm):
     branding_style = HiddenField('branding_style')
 
 
-class ServiceSelectEmailBranding(StripWhitespaceForm):
-
-    def __init__(self, email_brandings=[], *args, **kwargs):
-        self.email_branding.choices = email_brandings
-        super(ServiceSelectEmailBranding, self).__init__(*args, **kwargs)
-
-    email_branding = RadioField(
-        'Email branding',
-        validators=[
-            DataRequired()
-        ]
-    )
-
-
 class ServiceUpdateEmailBranding(StripWhitespaceForm):
     name = StringField('Name of brand')
     text = StringField('Text')

app/main/views/email_branding.py

@@ -3,10 +3,7 @@ from flask_login import login_required
 
 from app import email_branding_client
 from app.main import main
-from app.main.forms import (
-    ServiceSelectEmailBranding,
-    ServiceUpdateEmailBranding,
-)
+from app.main.forms import SearchTemplatesForm, ServiceUpdateEmailBranding
 from app.main.s3_client import (
     TEMP_TAG,
     delete_temp_file,
@@ -14,10 +11,7 @@ from app.main.s3_client import (
     persist_logo,
     upload_logo,
 )
-from app.main.views.service_settings import (
-    get_branding_as_dict,
-    get_branding_as_value_and_label,
-)
+from app.main.views.service_settings import get_branding_as_value_and_label
 from app.utils import get_cdn_domain, user_is_platform_admin
 
 
@@ -27,19 +21,11 @@ from app.utils import get_cdn_domain, user_is_platform_admin
 def email_branding():
     brandings = email_branding_client.get_all_email_branding(sort_key='name')
 
-    form = ServiceSelectEmailBranding()
-    email_brandings = get_branding_as_value_and_label(brandings)
-    form.email_branding.choices = email_brandings + [('None', 'Create a new email branding')]
-    if form.validate_on_submit():
-        if form.email_branding.data != 'None':
-            return redirect(url_for('.update_email_branding', branding_id=form.email_branding.data))
-        else:
-            return redirect(url_for('.create_email_branding'))
-
     return render_template(
         'views/email-branding/select-branding.html',
-        form=form,
-        branding_dict=get_branding_as_dict(brandings),
+        email_brandings=get_branding_as_value_and_label(brandings),
+        search_form=SearchTemplatesForm(),
+        show_search_box=len(brandings) > 9,
     )
 
 
@@ -106,6 +92,7 @@ def update_email_branding(branding_id, logo=None):
 @user_is_platform_admin
 def create_email_branding(logo=None):
     form = ServiceUpdateEmailBranding()
+
     if form.validate_on_submit():
         if form.file.data:
             upload_filename = upload_logo(

app/main/views/service_settings.py

@@ -34,6 +34,7 @@ from app.main.forms import (
     OrganisationTypeForm,
     RenameServiceForm,
     RequestToGoLiveForm,
+    SearchTemplatesForm,
     ServiceContactDetailsForm,
     ServiceDataRetentionEditForm,
     ServiceDataRetentionForm,
@@ -886,7 +887,9 @@ def service_set_email_branding(service_id):
     return render_template(
         'views/service-settings/set-email-branding.html',
         form=form,
-        branding_dict=get_branding_as_dict(email_branding)
+        branding_dict=get_branding_as_dict(email_branding),
+        search_form=SearchTemplatesForm(),
+        show_search_box=(len(email_branding) > 6)
     )
 
 

app/templates/components/live-search.html

@@ -0,0 +1,21 @@
+{% from "components/textbox.html" import textbox %}
+
+{% macro live_search(
+    target_selector=None,
+    show=False,
+    form=None,
+    label=None
+) %}
+    {%- set search_label = label or form.search.label.text %}
+    {% if show %}
+      <div data-module="autofocus">
+        <div class="live-search" data-module="live-search" data-targets="{{ target_selector }}">
+          {{ textbox(
+            form.search,
+            width='1-1',
+            label=search_label
+          ) }}
+        </div>
+      </div>
+    {% endif %}
+{% endmacro %}

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

@@ -10,7 +10,7 @@
 
 {% block platform_admin_content %}
 
-  <h1 class="heading-large">{{ '{} email branding'.format('Update' if email_branding else 'Create')}}</h1>
+  <h1 class="heading-large">{{ '{} email branding'.format('Update' if email_branding else 'Add')}}</h1>
   <div class="grid-row">
     <div class="column-three-quarters">
       {% if logo %}

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

@@ -1,26 +1,31 @@
 {% extends "views/platform-admin/_base_template.html" %}
 {% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
+{% from "components/live-search.html" import live_search %}
 
 {% block service_page_title %}
-  Select email branding
+  Email branding
 {% endblock %}
 
 {% block platform_admin_content %}
 
-  <h1 class="heading-large">
-    <div>Select an email branding to update</div>
-    <div>or create a new email branding</div>
-  </h1>
-  <div class="grid-row">
-    <div class="column-three-quarters">
-      <form method="post">
-        {{ radios(form.email_branding) }}
-        {{ page_footer(
-          'Next'
-        ) }}
-      </form>
+  <div class="grid-row bottom-gutter-2-3">
+    <div class="column-two-thirds">
+      <h1 class="heading-large">Email branding</h1>
+    </div>
+    <div class="column-one-third">
+      <a href="{{ url_for('.create_email_branding') }}" class="button align-with-heading">Add new brand</a>
     </div>
   </div>
+  {{ live_search(target_selector='.message-name', show=show_search_box, form=search_form) }}
+  <nav>
+    {% for id, name in email_brandings %}
+      <div class="message-name">
+        <a href="{{ url_for('.update_email_branding', branding_id=id) }}">
+          {{name}}
+        </a>
+      </div>
+    {% endfor %}
+  </nav>
 
 {% endblock %}

app/templates/views/manage-users.html

@@ -3,6 +3,7 @@
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/tick-cross.html" import tick_cross %}
 {% from "components/textbox.html" import textbox %}
+{% from "components/live-search.html" import live_search %}
 
 {% set table_options = {
   'field_headings': [
@@ -33,12 +34,7 @@
 
   {% if show_search_box %}
     <div data-module="autofocus">
-      <div class="live-search" data-module="live-search" data-targets=".user-list-item">
-        {{ textbox(
-          form.search,
-          width='1-1'
-        ) }}
-      </div>
+      {{ live_search(target_selector='.user-list-item', show=True, form=form) }}
     </div>
   {% endif %}
 

app/templates/views/organisations/organisation/users/index.html

@@ -3,6 +3,7 @@
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/tick-cross.html" import tick_cross %}
 {% from "components/textbox.html" import textbox %}
+{% from "components/live-search.html" import live_search %}
 
 {% block org_page_title %}
   Team members
@@ -23,12 +24,7 @@
 
   {% if show_search_box %}
     <div data-module="autofocus">
-      <div class="live-search" data-module="live-search" data-targets=".user-list-item">
-        {{ textbox(
-          form.search,
-          width='1-1'
-        ) }}
-      </div>
+      {{ live_search(target_selector='.user-list-item', show=True, form=form) }}
     </div>
   {% endif %}
 

app/templates/views/pricing.html

@@ -1,5 +1,6 @@
 {% from "components/table.html" import mapping_table, row, text_field, field, row_heading %}
 {% from "components/textbox.html" import textbox %}
+{% from "components/live-search.html" import live_search %}
 
 {% extends "withoutnav_template.html" %}
 
@@ -62,13 +63,7 @@
     <details>
       <summary>International text message rates</summary>
 
-      <div class="live-search" data-module="live-search" data-targets="#international-pricing .table-row">
-        {{ textbox(
-          search_form.search,
-          width='1-1',
-          label='Search by country name or code'
-        ) }}
-      </div>
+      {{ live_search(target_selector='#international-pricing .table-row', show=True, form=search_form, label='Search by country name or code') }}
 
       <div id="international-pricing" class="bottom-gutter-3-2">
         {% call mapping_table(

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

@@ -1,6 +1,7 @@
 {% extends "withnav_template.html" %}
 {% from "components/radios.html" import radios %}
 {% from "components/page-footer.html" import page_footer %}
+{% from "components/live-search.html" import live_search %}
 
 {% block service_page_title %}
   Set email branding
@@ -10,11 +11,16 @@
 
   <h1 class="heading-large">Set email branding</h1>
   <form method="post">
+    <div class="grid-row">
+      <div class="column-one-whole">
+      </div>
+    </div>
     <div class="grid-row">
       <div class="column-one-half">
         {{ radios(form.branding_type) }}
       </div>
-      <div class="column-one-half">
+      <div class="column-one-half brand_styles">
+        {{ live_search(target_selector='.brand_styles .multiple-choice', show=show_search_box, form=search_form, label='Search branding styles by name') }}
         {{ radios(form.branding_style) }}
       </div>
     </div>

app/templates/views/templates/_search-box.html

@@ -1,10 +0,0 @@
-    {% if show_search_box %}
-      <div data-module="autofocus">
-        <div class="live-search" data-module="live-search" data-targets="#template-list .column-whole">
-          {{ textbox(
-            search_form.search,
-            width='1-1'
-          ) }}
-        </div>
-      </div>
-    {% endif %}

app/templates/views/templates/choose-reply.html

@@ -1,6 +1,7 @@
 {% from "components/pill.html" import pill %}
 {% from "components/message-count-label.html" import message_count_label %}
 {% from "components/textbox.html" import textbox %}
+{% from "components/live-search.html" import live_search %}
 
 {% extends "withnav_template.html" %}
 
@@ -28,7 +29,7 @@
 
   {% else %}
 
-    {% include "views/templates/_search-box.html" %}
+    {{ live_search(target_selector='#template-list .column-whole', show=show_search_box, form=search_form) }}
 
     <nav class="grid-row" id=template-list>
       {% for template in templates %}

app/templates/views/templates/choose.html

@@ -1,6 +1,7 @@
 {% from "components/pill.html" import pill %}
 {% from "components/message-count-label.html" import message_count_label %}
 {% from "components/textbox.html" import textbox %}
+{% from "components/live-search.html" import live_search %}
 
 {% extends "withnav_template.html" %}
 
@@ -55,7 +56,7 @@
       </div>
     {% endif %}
 
-    {% include "views/templates/_search-box.html" %}
+    {{ live_search(target_selector='#template-list .column-whole', show=show_search_box, form=search_form) }}
 
     <nav class="grid-row" id=template-list>
       {% for template in templates %}

tests/app/main/views/test_conversation.py

@@ -278,6 +278,34 @@ def test_conversation_reply_shows_templates(
         )
 
 
+def test_conversation_reply_shows_live_search_if_list_of_templates_taller_than_screen(
+    client_request,
+    fake_uuid,
+    mock_get_more_service_templates_than_can_fit_onscreen,
+):
+    page = client_request.get(
+        'main.conversation_reply',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+    )
+
+    assert page.select('.live-search')
+
+
+def test_conversation_reply_shows_live_search_if_list_of_templates_fits_onscreen(
+    client_request,
+    fake_uuid,
+    mock_get_service_templates,
+):
+    page = client_request.get(
+        'main.conversation_reply',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+    )
+
+    assert not page.select('.live-search')
+
+
 def test_conversation_reply_redirects_with_phone_number_from_notification(
     client_request,
     fake_uuid,

tests/app/main/views/test_email_branding.py

@@ -20,19 +20,26 @@ def test_email_branding_page_shows_full_branding_list(
 
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    radio_labels = page.select('div.multiple-choice > label')
-    brand_names = [element.get_text().strip() for idx, element in enumerate(radio_labels)]
+    links = page.select('.message-name a')
+    brand_names = [normalize_spaces(link.text) for link in links]
+    hrefs = [link['href'] for link in links]
 
     assert normalize_spaces(
         page.select_one('h1').text
-    ) == "Select an email branding to update or create a new email branding"
+    ) == "Email branding"
+
+    assert page.select_one('.column-three-quarters a')['href'] == url_for('main.create_email_branding')
 
-    first_label = radio_labels[0]
-    assert normalize_spaces(first_label.text) == 'org 1'
     assert brand_names == [
-        'org 1', 'org 2', 'org 3', 'org 4', 'org 5', 'Create a new email branding']
-
-    assert normalize_spaces((radio_labels[-1]).text) == 'Create a new email branding'
+        'org 1', 'org 2', 'org 3', 'org 4', 'org 5'
+    ]
+    assert hrefs == [
+        url_for('.update_email_branding', branding_id=1),
+        url_for('.update_email_branding', branding_id=2),
+        url_for('.update_email_branding', branding_id=3),
+        url_for('.update_email_branding', branding_id=4),
+        url_for('.update_email_branding', branding_id=5),
+    ]
 
 
 def test_edit_email_branding_shows_the_correct_branding_info(

tests/app/main/views/test_service_settings.py

@@ -1782,6 +1782,34 @@ def test_should_show_branding_styles(
     app.service_api_client.get_service.assert_called_once_with(service_one['id'])
 
 
+def test_should_show_live_search_if_list_of_brand_styles_fits_onscreen(
+    logged_in_platform_admin_client,
+    service_one,
+    mock_get_email_branding_that_can_fit_onscreen,
+):
+    response = logged_in_platform_admin_client.get(url_for(
+        'main.service_set_email_branding', service_id=service_one['id']
+    ))
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    assert not page.select('.live-search')
+
+
+def test_should_show_live_search_if_list_of_brand_styles_taller_than_page(
+    logged_in_platform_admin_client,
+    service_one,
+    mock_get_more_email_branding_than_can_fit_onscreen,
+):
+    response = logged_in_platform_admin_client.get(url_for(
+        'main.service_set_email_branding', service_id=service_one['id']
+    ))
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    assert page.select('.live-search')
+
+
 def test_should_send_branding_and_organisations_to_preview(
     logged_in_platform_admin_client,
     service_one,

tests/app/main/views/test_templates.py

@@ -147,6 +147,32 @@ def test_should_not_show_template_nav_if_only_one_type_of_template(
     assert not page.select('.pill')
 
 
+def test_should_not_show_live_search_if_list_of_templates_fits_onscreen(
+    client_request,
+    mock_get_service_templates
+):
+
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+    )
+
+    assert not page.select('.live-search')
+
+
+def test_should_show_live_search_if_list_of_templates_taller_than_screen(
+    client_request,
+    mock_get_more_service_templates_than_can_fit_onscreen
+):
+
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+    )
+
+    assert page.select('.live-search')
+
+
 def test_should_show_page_for_one_template(
     logged_in_client,
     mock_get_service_template,

tests/conftest.py

@@ -956,40 +956,41 @@ def mock_update_service_template_400_content_too_big(mocker):
         side_effect=_update)
 
 
+def create_service_templates(service_id, number_of_templates=6):
+    template_types = ["sms", "sms", "email", "email", "letter", "letter"]
+    service_templates = []
+
+    for _ in range(1, number_of_templates + 1):
+        template_number = "two" if _ % 2 == 0 else "one"
+        template_type = template_types[(_ % 6) - 1]
+
+        service_templates.append(template_json(
+            service_id,
+            TEMPLATE_ONE_ID if _ == 1 else str(generate_uuid),
+            "{}_template_{}".format(template_type, template_number),
+            template_type,
+            "{} template {} content".format(template_type, template_number),
+            subject="{} template {} subject".format(template_type, template_number)
+                    if template_type in ["email", "letter"] else None
+        ))
+
+    return {'data': service_templates}
+
+
 @pytest.fixture(scope='function')
 def mock_get_service_templates(mocker):
-    uuid1 = TEMPLATE_ONE_ID
-    uuid2 = str(generate_uuid())
-    uuid3 = str(generate_uuid())
-    uuid4 = str(generate_uuid())
-    uuid5 = str(generate_uuid())
-    uuid6 = str(generate_uuid())
-
     def _create(service_id):
-        return {'data': [
-            template_json(
-                service_id, uuid1, "sms_template_one", "sms", "sms template one content"
-            ),
-            template_json(
-                service_id, uuid2, "sms_template_two", "sms", "sms template two content"
-            ),
-            template_json(
-                service_id, uuid3, "email_template_one", "email", "email template one content",
-                subject='email template one subject',
-            ),
-            template_json(
-                service_id, uuid4, "email_template_two", "email", "email template two content",
-                subject='email template two subject',
-            ),
-            template_json(
-                service_id, uuid5, "letter_template_one", "letter", "letter template one content",
-                subject='letter template one subject',
-            ),
-            template_json(
-                service_id, uuid6, "letter_template_two", "letter", "letter template two content",
-                subject='letter template two subject',
-            ),
-        ]}
+        return create_service_templates(service_id)
+
+    return mocker.patch(
+        'app.service_api_client.get_service_templates',
+        side_effect=_create)
+
+
+@pytest.fixture(scope='function')
+def mock_get_more_service_templates_than_can_fit_onscreen(mocker):
+    def _create(service_id):
+        return create_service_templates(service_id, number_of_templates=20)
 
     return mocker.patch(
         'app.service_api_client.get_service_templates',
@@ -2436,37 +2437,65 @@ def mock_send_already_registered_email(mocker):
     return mocker.patch('app.user_api_client.send_already_registered_email')
 
 
+def create_email_brandings(number_of_brandings, non_standard_values={}, shuffle=False):
+    brandings = [
+        {
+            'id': str(idx),
+            'name': 'org {}'.format(idx),
+            'text': 'org {}'.format(idx),
+            'colour': None,
+            'logo': 'logo{}.png'.format(idx),
+            'brand_type': 'org',
+        } for idx in range(1, number_of_brandings + 1)]
+
+    for idx, row in enumerate(non_standard_values):
+        brandings[row['idx']].update(non_standard_values)
+
+    if shuffle:
+        brandings.insert(3, brandings.pop(4))
+
+    return brandings
+
+
 @pytest.fixture(scope='function')
 def mock_get_all_email_branding(mocker):
     def _get_all_email_branding(sort_key=None):
-        if sort_key:
-            return [
-                {'id': '1', 'name': 'org 1', 'text': 'org 1', 'colour': 'red', 'logo': 'logo1.png',
-                 'brand_type': 'govuk'},
-                {'id': '2', 'name': 'org 2', 'text': 'org 2', 'colour': 'orange', 'logo': 'logo2.png',
-                 'brand_type': 'both'},
-                {'id': '3', 'name': 'org 3', 'text': None, 'colour': None, 'logo': 'logo3.png', 'brand_type': 'org'},
-                {'id': '4', 'name': 'org 4', 'text': 'org 4', 'colour': None, 'logo': 'logo4.png',
-                 'brand_type': 'org_banner'},
-                {'id': '5', 'name': 'org 5', 'text': None, 'colour': 'blue', 'logo': 'logo5.png', 'brand_type': 'org'},
-            ]
-        else:
-            return [
-                {'id': '1', 'name': 'org 1', 'text': 'org 1', 'colour': 'red', 'logo': 'logo1.png',
-                 'brand_type': 'govuk'},
-                {'id': '2', 'name': 'org 2', 'text': 'org 2', 'colour': 'orange', 'logo': 'logo2.png',
-                 'brand_type': 'both'},
-                {'id': '3', 'name': 'org 3', 'text': None, 'colour': None, 'logo': 'logo3.png', 'brand_type': 'org'},
-                {'id': '5', 'name': 'org 5', 'text': None, 'colour': 'blue', 'logo': 'logo5.png',
-                 'brand_type': 'org_banner'},
-                {'id': '4', 'name': 'org 4', 'text': 'org 4', 'colour': None, 'logo': 'logo4.png', 'brand_type': 'org'},
-            ]
+        non_standard_values = [
+            {'idx': 1, 'colour': 'red'},
+            {'idx': 2, 'colour': 'orange'},
+            {'idx': 3, 'text': None},
+            {'idx': 4, 'colour': 'blue'},
+        ]
+        shuffle = sort_key is None
+        return create_email_brandings(5, non_standard_values=non_standard_values, shuffle=shuffle)
 
     return mocker.patch(
         'app.email_branding_client.get_all_email_branding', side_effect=_get_all_email_branding
     )
 
 
+@pytest.fixture(scope='function')
+def mock_get_more_email_branding_than_can_fit_onscreen(mocker):
+    def _get_more_email_branding_than_can_fit_onscreen():
+        return create_email_brandings(8)
+
+    return mocker.patch(
+        'app.email_branding_client.get_all_email_branding',
+        side_effect=_get_more_email_branding_than_can_fit_onscreen
+    )
+
+
+@pytest.fixture(scope='function')
+def mock_get_email_branding_that_can_fit_onscreen(mocker):
+    def _get_email_branding_that_can_fit_onscreen():
+        return create_email_brandings(4)
+
+    return mocker.patch(
+        'app.email_branding_client.get_all_email_branding',
+        side_effect=_get_email_branding_that_can_fit_onscreen
+    )
+
+
 @pytest.fixture(scope='function')
 def mock_get_letter_email_branding(mocker):
     def _get_letter_email_branding():