Merge pull request #2164 from alphagov/rename-basic-nav

Improve navigation in basic view

app/main/views/templates.py

@@ -140,14 +140,8 @@ def choose_template(service_id, template_type='all'):
         )
     ]
 
-    if current_user.has_permissions('view_activity'):
-        page_title = 'Templates'
-    else:
-        page_title = 'Choose a template'
-
     return render_template(
         'views/templates/choose.html',
-        page_title=page_title,
         templates=templates_on_page,
         show_search_box=(len(templates_on_page) > 7),
         show_template_nav=has_multiple_template_types and (len(templates) > 2),

app/templates/main_nav.html

@@ -20,7 +20,7 @@
 {% else %}
 <nav class="navigation">
   <ul>
-    <li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}" {{ casework_navigation.is_selected('send-one-off') }}>Send a message</a></li>
+    <li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}" {{ casework_navigation.is_selected('send-one-off') }}>Templates</a></li>
   </ul>
   <ul>
     <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='sending,delivered,failed') }}" {{ casework_navigation.is_selected('sent-messages') }}>Sent messages</a></li>

app/templates/views/notifications.html

@@ -4,16 +4,20 @@
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/textbox.html" import textbox %}
 
+{% set page_title = (
+  message_count_label(99, message_type, suffix='') | capitalize
+  if current_user.has_permissions('view_activity')
+  else 'Sent messages'
+) %}
+
 {% block service_page_title %}
-  {{ message_count_label(99, message_type, suffix='') | capitalize }}
+  {{ page_title }}
 {% endblock %}
 
 {% block maincolumn_content %}
 
   <h1 class="heading-large">
-    {% if not current_user.has_permissions('view_activity') %}<span class="visually-hidden">{% endif %}
+    {{ page_title }}
-      {{ message_count_label(99, message_type, suffix='') | capitalize }}
-    {% if not current_user.has_permissions('view_activity') %}</span>{% endif %}
   </h1>
   {% if not message_type == "letter" %}
 

app/templates/views/templates/choose.html

@@ -4,6 +4,8 @@
 
 {% extends "withnav_template.html" %}
 
+{% set page_title = 'Templates' %}
+
 {% block service_page_title %}
   {{ page_title }}
 {% endblock %}

tests/app/main/views/test_accept_invite.py

@@ -122,7 +122,7 @@ def test_invite_goes_in_session(
 
 @pytest.mark.parametrize('user, landing_page_title', [
     (active_user_with_permissions, 'Dashboard'),
-    (active_caseworking_user, 'Choose a template'),
+    (active_caseworking_user, 'Templates'),
 ])
 def test_accepting_invite_removes_invite_from_session(
     client_request,

tests/app/main/views/test_activity.py

@@ -19,14 +19,11 @@ from tests.conftest import (
 )
 
 
-@pytest.mark.parametrize('user', (
-    active_user_view_permissions,
-    active_caseworking_user,
-))
 @pytest.mark.parametrize(
-    "message_type,page_title", [
+    "user,extra_args,expected_update_endpoint,page_title", [
-        ('email', 'Emails'),
+        (active_user_view_permissions, {'message_type': 'email'}, '/email.json', 'Emails'),
-        ('sms', 'Text messages')
+        (active_user_view_permissions, {'message_type': 'sms'}, '/sms.json', 'Text messages'),
+        (active_caseworking_user, {}, '.json', 'Sent messages'),
     ]
 )
 @pytest.mark.parametrize(
@@ -68,11 +65,14 @@ from tests.conftest import (
     ]
 )
 def test_can_show_notifications(
+    client_request,
     logged_in_client,
     service_one,
     mock_get_notifications,
     mock_get_service_statistics,
-    message_type,
+    user,
+    extra_args,
+    expected_update_endpoint,
     page_title,
     status_argument,
     expected_api_call,
@@ -81,33 +81,29 @@ def test_can_show_notifications(
     to_argument,
     expected_to_argument,
     mocker,
-    user,
     fake_uuid,
 ):
-    mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid))
+    client_request.login(user(fake_uuid))
     if expected_to_argument:
-        response = logged_in_client.post(
+        page = client_request.post(
-            url_for(
-                'main.view_notifications',
-                service_id=service_one['id'],
-                message_type=message_type,
-                status=status_argument,
-                page=page_argument,
-            ),
-            data={
-                'to': to_argument
-            }
-        )
-    else:
-        response = logged_in_client.get(url_for(
             'main.view_notifications',
             service_id=service_one['id'],
-            message_type=message_type,
             status=status_argument,
             page=page_argument,
-        ))
+            _data={
-    assert response.status_code == 200
+                'to': to_argument
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+            },
+            _expected_status=200,
+            **extra_args
+        )
+    else:
+        page = client_request.get(
+            'main.view_notifications',
+            service_id=service_one['id'],
+            status=status_argument,
+            page=page_argument,
+            **extra_args
+        )
     text_of_first_row = page.select('tbody tr')[0].text
     assert '07123456789' in text_of_first_row
     assert (
@@ -120,7 +116,10 @@ def test_can_show_notifications(
     path_to_json = page.find("div", {'data-key': 'notifications'})['data-resource']
 
     url = urlparse(path_to_json)
-    assert url.path == '/services/{}/notifications/{}.json'.format(service_one['id'], message_type)
+    assert url.path == '/services/{}/notifications{}'.format(
+        service_one['id'],
+        expected_update_endpoint,
+    )
     query_dict = parse_qs(url.query)
     if status_argument:
         assert query_dict['status'] == [status_argument]
@@ -133,15 +132,15 @@ def test_can_show_notifications(
         page=expected_page_argument,
         service_id=service_one['id'],
         status=expected_api_call,
-        template_type=[message_type],
+        template_type=list(extra_args.values()),
         to=expected_to_argument,
     )
 
     json_response = logged_in_client.get(url_for(
         'main.get_notifications_as_json',
         service_id=service_one['id'],
-        message_type=message_type,
+        status=status_argument,
-        status=status_argument
+        **extra_args
     ))
     json_content = json.loads(json_response.get_data(as_text=True))
     assert json_content.keys() == {'counts', 'notifications'}

tests/app/main/views/test_service_settings.py

@@ -2459,7 +2459,7 @@ def test_preview_basic_view(
     with client_request.session_transaction() as session:
         assert session['basic'] is True
 
-    assert page.h1.text.strip() == 'Choose a template'
+    assert page.h1.text.strip() == 'Templates'
     page.select('.navigation-service-basic-view-preview')
     assert normalize_spaces(page.select_one('.navigation-service').text) == (
         'service one '

tests/app/main/views/test_templates.py

@@ -70,7 +70,7 @@ from tests.conftest import single_letter_contact_block
         ),
         (
             active_caseworking_user,
-            'Choose a template',
+            'Templates',
             {},
             ['Text message', 'Email'],
             [
@@ -82,7 +82,7 @@ from tests.conftest import single_letter_contact_block
         ),
         (
             active_caseworking_user,
-            'Choose a template',
+            'Templates',
             {'template_type': 'email'},
             ['All', 'Text message'],
             ['email_template_one', 'email_template_two'],

tests/app/test_navigation.py

@@ -159,5 +159,5 @@ def test_caseworkers_get_caseworking_navigation(
     )
     page = client_request.get('main.choose_template', service_id=SERVICE_ONE_ID)
     assert normalize_spaces(page.select_one('#content nav').text) == (
-        'Send a message Sent messages'
+        'Templates Sent messages'
     )