Merge pull request #2942 from alphagov/fix-heading-size-new-templates-page

Fix heading size on empty templates page

app/templates/components/folder-path.html

@@ -3,38 +3,30 @@
   service_id,
   template_type,
   current_user,
-  fallback_page_title=None,
-  show_fallback_page_title=False,
   link_current_item=False
 ) %}
-  {% if show_fallback_page_title %}
-    <h1 class="heading-medium">
-      {{ fallback_page_title }}
-    </h1>
-  {% else %}
-    <h1 class="heading-medium folder-heading">
-      {% for folder in folders %}
-        {% if loop.last and not link_current_item %}
-          {% if folder.template_type or not folder.id %}
-            <span class="folder-heading-template">{{ folder.name }}</span>
+  <h1 class="heading-medium folder-heading">
+    {% for folder in folders %}
+      {% if loop.last and not link_current_item %}
+        {% if folder.template_type or not folder.id %}
+          <span class="folder-heading-template">{{ folder.name }}</span>
+        {% else %}
+          <span class="folder-heading-folder">{{ folder.name }}</span>
+        {% endif %}
+      {% else %}
+        {% if folder.id %}
+          {% if current_user.has_template_folder_permission(folder) %}
+            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type, template_folder_id=folder.id) }}" class="folder-heading-folder {% if loop.index < (loop.length - 1) %}folder-heading-folder-truncated{% endif %}" title="{{ folder.name }}">{{ folder.name }}</a>
           {% else %}
             <span class="folder-heading-folder">{{ folder.name }}</span>
           {% endif %}
         {% else %}
-          {% if folder.id %}
-            {% if current_user.has_template_folder_permission(folder) %}
-              <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type, template_folder_id=folder.id) }}" class="folder-heading-folder {% if loop.index < (loop.length - 1) %}folder-heading-folder-truncated{% endif %}" title="{{ folder.name }}">{{ folder.name }}</a>
-            {% else %}
-              <span class="folder-heading-folder">{{ folder.name }}</span>
-            {% endif %}
-          {% else %}
-            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type) }}" title="Templates" class="{% if loop.length > 2 %}folder-heading-folder-root-truncated{% endif %}">Templates</a>
-          {% endif %}
-          {% if not loop.last %}{{ folder_path_separator() }}{% endif %}
+          <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type) }}" title="Templates" class="{% if loop.length > 2 %}folder-heading-folder-root-truncated{% endif %}">Templates</a>
         {% endif %}
-      {% endfor %}
-    </h1>
-  {% endif %}
+        {% if not loop.last %}{{ folder_path_separator() }}{% endif %}
+      {% endif %}
+    {% endfor %}
+  </h1>
 {% endmacro %}
 
 
@@ -75,21 +67,13 @@
 {% endmacro %}
 
 
-{% macro page_title_folder_path(
-  folders,
-  fallback_page_title=None,
-  show_fallback_page_title=False
-) %}
-  {% if show_fallback_page_title %}
-    {{ fallback_page_title }}
-  {% else %}
-    {% for folder in folders|reverse %}
-      {{ folder.name }}
-      {% if not loop.last %}
-        –
-      {% endif %}
-    {% endfor %}
-  {% endif %}
+{% macro page_title_folder_path(folders) %}
+  {% for folder in folders|reverse %}
+    {{ folder.name }}
+    {% if not loop.last %}
+      –
+    {% endif %}
+  {% endfor %}
 {% endmacro %}
 
 

app/templates/views/templates/choose.html

@@ -11,18 +11,14 @@
 {% set page_title = 'Templates' %}
 
 {% block service_page_title %}
-  {{ page_title_folder_path(
-    template_folder_path,
-    fallback_page_title=page_title,
-    show_fallback_page_title=not current_service.all_template_folders
-  ) }}
+  {{ page_title_folder_path(template_folder_path) }}
 {% endblock %}
 
 {% block maincolumn_content %}
 
   {% if (not current_service.all_templates) and (not current_service.all_template_folders) %}
 
-    <h1 class="heading-large">
+    <h1 class="heading-medium">
       {{ page_title }}
     </h1>
     {% if current_user.has_permissions('manage_templates') %}
@@ -53,9 +49,7 @@
           folders=template_folder_path,
           service_id=current_service.id,
           template_type=template_type,
-          current_user=current_user,
-          fallback_page_title=page_title,
-          show_fallback_page_title=not current_service.all_template_folders
+          current_user=current_user
         ) }}
       </div>
       {% if current_user.has_permissions('manage_templates') and current_template_folder_id and user_has_template_folder_permission %}

app/templates/views/templates/template.html

@@ -1,12 +1,12 @@
 {% extends "withnav_template.html" %}
 {% from "components/banner.html" import banner_wrapper %}
-{% from "components/folder-path.html" import folder_path %}
+{% from "components/folder-path.html" import folder_path, page_title_folder_path %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/textbox.html" import textbox %}
 {% from "components/api-key.html" import api_key %}
 
 {% block service_page_title %}
-  {{ template.name }}
+  {{ page_title_folder_path(current_service.get_template_path(template._template)) }}
 {% endblock %}
 
 {% block maincolumn_content %}
@@ -35,9 +35,7 @@
           folders=current_service.get_template_path(template._template),
           service_id=current_service.id,
           template_type='all',
-          current_user=current_user,
-          fallback_page_title=template.name,
-          show_fallback_page_title=not current_service.all_template_folders
+          current_user=current_user
         ) }}
       </div>
     </div>

tests/app/main/views/accounts/test_choose_accounts.py

@@ -176,6 +176,7 @@ def test_should_not_show_back_to_service_if_user_doesnt_belong_to_service(
         service_id=mock_get_service.return_value['id'],
         template_id=fake_uuid,
         _expected_status=expected_status,
+        _test_page_title=False,
     )
 
     assert normalize_spaces(

tests/app/main/views/test_accept_invite.py

@@ -163,7 +163,7 @@ def test_accepting_invite_removes_invite_from_session(
         token='thisisnotarealtoken',
         _follow_redirects=True,
     )
-    assert normalize_spaces(page.h1.string) == landing_page_title
+    assert normalize_spaces(page.select_one('h1').text) == landing_page_title
 
     with client_request.session_transaction() as session:
         assert 'invited_user' not in session

tests/app/main/views/test_templates.py

@@ -3,7 +3,6 @@ from functools import partial
 from unittest.mock import ANY, Mock
 
 import pytest
-from bs4 import BeautifulSoup
 from flask import url_for
 from freezegun import freeze_time
 from notifications_python_client.errors import HTTPError
@@ -426,8 +425,14 @@ def test_user_with_only_send_and_view_sees_letter_page(
         'main.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
+    )
+    assert normalize_spaces(page.select_one('h1').text) == (
+        'Templates / Two week reminder'
+    )
+    assert normalize_spaces(page.select_one('title').text) == (
+        'Two week reminder – Templates – service one – GOV.UK Notify'
     )
-    assert page.select_one('h1').text.strip() == 'Two week reminder'
 
 
 @pytest.mark.parametrize('letter_branding, expected_link, expected_link_text', (
@@ -462,6 +467,7 @@ def test_letter_with_default_branding_has_add_logo_button(
         'main.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=TEMPLATE_ONE_ID,
+        _test_page_title=False,
     )
 
     first_edit_link = page.select_one('.template-container a')
@@ -493,6 +499,7 @@ def test_view_letter_template_displays_postage(
         'main.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
     )
 
     assert normalize_spaces(page.select_one('.letter-postage').text) == expected_result
@@ -508,6 +515,7 @@ def test_view_non_letter_template_does_not_display_postage(
         '.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
     )
     assert "Postage" not in page.text
 
@@ -595,29 +603,33 @@ def test_edit_letter_templates_postage_updates_postage(
     ),
 ])
 def test_should_be_able_to_view_a_template_with_links(
-    client,
+    client_request,
     mock_get_service_template,
     mock_get_template_folders,
     active_user_with_permissions,
     single_letter_contact_block,
-    mocker,
-    service_one,
     fake_uuid,
     permissions,
     links_to_be_shown,
     permissions_warning_to_be_shown,
 ):
-    active_user_with_permissions._permissions[service_one['id']] = permissions + ['view_activity']
-    client.login(active_user_with_permissions, mocker, service_one)
+    active_user_with_permissions._permissions[SERVICE_ONE_ID] = permissions + ['view_activity']
+    client_request.login(active_user_with_permissions)
 
-    response = client.get(url_for(
+    page = client_request.get(
         '.view_template',
-        service_id=service_one['id'],
-        template_id=fake_uuid
-    ))
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+        _test_page_title=False,
+    )
+
+    assert normalize_spaces(page.select_one('h1').text) == (
+        'Templates / Two week reminder'
+    )
+    assert normalize_spaces(page.select_one('title').text) == (
+        'Two week reminder – Templates – service one – GOV.UK Notify'
+    )
 
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
     links_in_page = page.select('.pill-separate-item')
 
     assert len(links_in_page) == len(links_to_be_shown)
@@ -625,7 +637,7 @@ def test_should_be_able_to_view_a_template_with_links(
     for index, link_to_be_shown in enumerate(links_to_be_shown):
         assert links_in_page[index]['href'] == url_for(
             link_to_be_shown,
-            service_id=service_one['id'],
+            service_id=SERVICE_ONE_ID,
             template_id=fake_uuid,
         )
 
@@ -644,6 +656,7 @@ def test_should_show_template_id_on_template_page(
         '.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
     )
     assert page.select('.api-key-key')[0].text == fake_uuid
 
@@ -668,6 +681,7 @@ def test_should_show_sms_template_with_downgraded_unicode_characters(
         '.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
     )
 
     assert rendered_msg in page.text
@@ -688,7 +702,8 @@ def test_should_let_letter_contact_block_be_changed_for_the_template(
     page = client_request.get(
         'main.view_template',
         service_id=SERVICE_ONE_ID,
-        template_id=fake_uuid
+        template_id=fake_uuid,
+        _test_page_title=False,
     )
 
     assert page.find('a', {'class': 'edit-template-link-letter-contact'})['href'] == url_for(
@@ -1814,6 +1829,7 @@ def test_should_show_page_for_a_deleted_template(
         '.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=template_id,
+        _test_page_title=False,
     )
 
     content = str(page)
@@ -2138,6 +2154,7 @@ def test_should_show_hint_once_template_redacted(
         'main.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
     )
 
     assert page.select('.hint')[0].text == 'Personalisation is hidden after sending'
@@ -2158,6 +2175,7 @@ def test_should_not_show_redaction_stuff_for_letters(
         'main.view_template',
         service_id=SERVICE_ONE_ID,
         template_id=fake_uuid,
+        _test_page_title=False,
     )
 
     assert page.select('.hint') == []