From f386b991cb84f00afe2d1868173f53985c759e06 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 13 Jun 2017 15:27:29 +0100 Subject: [PATCH] =?UTF-8?q?Only=20show=20template=20navigation=20when=20it?= =?UTF-8?q?=E2=80=99s=20useful?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are lots of services that only send emails, or only send text messages. For these services, being able to filter the list of templates but type is pointless – it won’t cut the list down at all. This commit adds some logic to only show the navigation if the service has some variety of template types. --- app/main/views/templates.py | 5 +++++ app/templates/views/templates/choose.html | 8 +++++--- tests/app/main/views/test_templates.py | 13 +++++++++++++ tests/conftest.py | 15 +++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 95023c9d4..ab917225e 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -88,6 +88,10 @@ def view_template(service_id, template_id): 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, [ @@ -104,6 +108,7 @@ def choose_template(service_id, template_type='all'): template for template in templates if template_type in ['all', template['template_type']] ], + show_template_nav=has_multiple_template_types, template_nav_items=template_nav_items, template_type=template_type, search_form=SearchTemplatesForm(), diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index 7112cd39f..e055c8b71 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -48,9 +48,11 @@ {% endif %} -
- {{ pill(template_nav_items, current_value=template_type, show_count=False) }} -
+ {% if show_template_nav %} +
+ {{ pill(template_nav_items, current_value=template_type, show_count=False) }} +
+ {% endif %} {% if templates|length > 7 %}
diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 35384d0bb..780ecca78 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -64,6 +64,19 @@ def test_should_show_page_for_choosing_a_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, diff --git a/tests/conftest.py b/tests/conftest.py index 5e4aefbe7..819caa29b 100644 --- a/tests/conftest.py +++ b/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):