Merge pull request #1330 from alphagov/template-nav

Make list of templates filterable by type

app/assets/stylesheets/components/pill.scss

@@ -58,6 +58,11 @@
     }
 
   }
+
+  &-centered-item {
+    text-align: center;
+  }
+
 }
 
 .pill-separate {

app/main/views/templates.py

@@ -43,26 +43,7 @@ page_headings = {
 }
 
 
-@main.route("/services/<service_id>/templates", methods=['GET'])
-@login_required
-@user_has_permissions(
-    'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
-    admin_override=True,
-    any_=True,
-)
-def choose_template(service_id):
-    return render_template(
-        'views/templates/choose.html',
-        templates=service_api_client.get_service_templates(service_id)['data'],
-        search_form=SearchTemplatesForm(),
-    )
-
-
-@main.route("/services/<service_id>/templates/<template_id>")
+@main.route("/services/<service_id>/templates/<uuid:template_id>")
 @login_required
 @user_has_permissions(
     'view_activity',
@@ -73,7 +54,7 @@ def choose_template(service_id):
     admin_override=True, any_=True
 )
 def view_template(service_id, template_id):
-    template = service_api_client.get_service_template(service_id, template_id)['data']
+    template = service_api_client.get_service_template(service_id, str(template_id))['data']
     return render_template(
         'views/templates/template.html',
         template=get_template(
@@ -92,6 +73,51 @@ def view_template(service_id, template_id):
     )
 
 
+@main.route("/services/<service_id>/templates")
+@main.route("/services/<service_id>/templates/<template_type>")
+@login_required
+@user_has_permissions(
+    'view_activity',
+    'send_texts',
+    'send_emails',
+    'manage_templates',
+    'manage_api_keys',
+    admin_override=True,
+    any_=True,
+)
+def choose_template(service_id, template_type='all'):
+    templates = service_api_client.get_service_templates(service_id)['data']
+
+    has_multiple_template_types = len({
+        template['template_type'] for template in templates
+    }) > 1
+
+    template_nav_items = [
+        (label, key, url_for('.choose_template', service_id=current_service['id'], template_type=key), '')
+        for label, key in filter(None, [
+            ('All', 'all'),
+            ('Text message', 'sms'),
+            ('Email', 'email'),
+            ('Letter', 'letter') if current_service['can_send_letters'] else None,
+        ])
+    ]
+
+    templates_on_page = [
+        template for template in templates
+        if template_type in ['all', template['template_type']]
+    ]
+
+    return render_template(
+        'views/templates/choose.html',
+        templates=templates_on_page,
+        show_search_box=(len(templates_on_page) > 7),
+        show_template_nav=has_multiple_template_types and (len(templates) > 2),
+        template_nav_items=template_nav_items,
+        template_type=template_type,
+        search_form=SearchTemplatesForm(),
+    )
+
+
 @main.route("/services/<service_id>/templates/<template_id>.<filetype>")
 @login_required
 @user_has_permissions('view_activity', admin_override=True)

app/templates/components/pill.html

@@ -3,19 +3,22 @@
 {% macro pill(
   items=[],
   current_value=None,
-  big_number_args={'smaller': True}
+  big_number_args={'smaller': True},
+  show_count=True
 ) %}
   <ul role='tablist' class='pill'>
     {% for label, option, link, count in items %}
       {% if current_value == option %}
         <li aria-selected='true' role='tab'>
-          <div class='pill-selected-item' tabindex='0'>
+          <div class='pill-selected-item{% if not show_count %} pill-centered-item{% endif %}' tabindex='0'>
       {% else %}
         <li aria-selected='false' role='tab'>
           <a href="{{ link }}">
       {% endif %}
+        {% if show_count %}
             {{ big_number(count, **big_number_args) }}
-            <div class="pill-label">{{ label }}</div>
+        {% endif %}
+            <div class="pill-label{% if not show_count %} pill-centered-item{% endif %}">{{ label }}</div>
       {% if current_value == option %}
           </div>
       {% else %}

app/templates/views/templates/choose.html

@@ -1,3 +1,4 @@
+{% from "components/pill.html" import pill %}
 {% from "components/message-count-label.html" import message_count_label %}
 {% from "components/textbox.html" import textbox %}
 
@@ -36,7 +37,7 @@
 
   {% else %}
 
-    <div class="grid-row bottom-gutter-1-2">
+    <div class="grid-row bottom-gutter-2-3">
       <div class="column-two-thirds">
         <h1 class="heading-large">Templates</h1>
       </div>
@@ -47,7 +48,13 @@
       {% endif %}
     </div>
 
-    {% if templates|length > 7 %}
+    {% if show_template_nav %}
+      <div class="bottom-gutter-2-3">
+        {{ pill(template_nav_items, current_value=template_type, show_count=False) }}
+      </div>
+    {% endif %}
+
+    {% if show_search_box %}
       <div data-module="autofocus">
         <div class="live-search" data-module="live-search" data-targets="#template-list .column-whole">
           {{ textbox(

tests/app/main/views/test_templates.py

@@ -7,13 +7,76 @@ from flask import url_for
 from freezegun import freeze_time
 from notifications_python_client.errors import HTTPError
 from tests.conftest import service_one as create_sample_service
-from tests.conftest import mock_get_service_email_template, mock_get_service_letter_template
+from tests.conftest import mock_get_service_email_template, mock_get_service_letter_template, SERVICE_ONE_ID
 from tests import validate_route_permission, template_json, single_notification_json
 from tests.app.test_utils import normalize_spaces
 
 from app.main.views.templates import get_last_use_message, get_human_readable_delta
 
 
+@pytest.mark.parametrize('extra_args, expected_nav_links, expected_templates', [
+    (
+        {},
+        ['Text message', 'Email'],
+        ['sms_template_one', 'sms_template_two', 'email_template_one', 'email_template_two']
+    ),
+    (
+        {'template_type': 'sms'},
+        ['All', 'Email'],
+        ['sms_template_one', 'sms_template_two'],
+    ),
+    (
+        {'template_type': 'email'},
+        ['All', 'Text message'],
+        ['email_template_one', 'email_template_two'],
+    ),
+])
+def test_should_show_page_for_choosing_a_template(
+    client_request,
+    mock_get_service_templates,
+    extra_args,
+    expected_nav_links,
+    expected_templates,
+):
+
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+        **extra_args
+    )
+
+    assert normalize_spaces(page.select('h1')[0].text) == 'Templates'
+
+    links_in_page = page.select('.pill a')
+
+    assert len(links_in_page) == len(expected_nav_links)
+
+    for index, expected_link in enumerate(expected_nav_links):
+        assert links_in_page[index].text.strip() == expected_link
+
+    template_links = page.select('.message-name a')
+
+    assert len(template_links) == len(expected_templates)
+
+    for index, expected_template in enumerate(expected_templates):
+        assert template_links[index].text.strip() == expected_template
+
+    mock_get_service_templates.assert_called_with(SERVICE_ONE_ID)
+
+
+def test_should_not_show_template_nav_if_only_one_type_of_template(
+    client_request,
+    mock_get_service_templates_with_only_one_template,
+):
+
+    page = client_request.get(
+        'main.choose_template',
+        service_id=SERVICE_ONE_ID,
+    )
+
+    assert not page.select('.pill')
+
+
 def test_should_show_page_for_one_template(
     logged_in_client,
     mock_get_service_template,

tests/conftest.py

@@ -492,6 +492,21 @@ def mock_get_service_templates_when_no_templates_exist(mocker):
         side_effect=_create)
 
 
+@pytest.fixture(scope='function')
+def mock_get_service_templates_with_only_one_template(mocker):
+
+    def _get(service_id):
+        return {'data': [
+            template_json(
+                service_id, generate_uuid(), "sms_template_one", "sms", "sms template one content"
+            )
+        ]}
+
+    return mocker.patch(
+        'app.service_api_client.get_service_templates',
+        side_effect=_get)
+
+
 @pytest.fixture(scope='function')
 def mock_delete_service_template(mocker):
     def _delete(service_id, template_id):