From 42cb4b0d9922baf9da03627ecfedf0fffecd3c98 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 1 Apr 2016 10:42:23 +0100 Subject: [PATCH] Remove the verbs from the navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because ‘Send text messages’ isn’t very helpful if you’re looking to edit a template. It also helps front-load the navigation, ie ‘Team’ is the first word, rather than the more generic ‘Manage’. --- app/main/views/send.py | 21 ++++----------------- app/templates/main_nav.html | 17 ++++++----------- app/templates/views/manage-users.html | 6 +----- app/templates/views/service-settings.html | 2 +- tests/app/main/views/test_manage_users.py | 10 +++++----- 5 files changed, 17 insertions(+), 39 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 6b1fe2618..40622b360 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -29,18 +29,6 @@ from app import (job_api_client, service_api_client) from app.utils import user_has_permissions, get_errors_for_csv -send_messages_page_headings = { - 'email': 'Send emails', - 'sms': 'Send text messages' -} - - -manage_templates_page_headings = { - 'email': 'Email templates', - 'sms': 'Text message templates' -} - - def get_send_button_text(template_type, number_of_messages): if 1 == number_of_messages: return { @@ -55,11 +43,10 @@ def get_send_button_text(template_type, number_of_messages): def get_page_headings(template_type): - # User has manage_service role - if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']): - return send_messages_page_headings[template_type] - else: - return manage_templates_page_headings[template_type] + return { + 'email': 'Email templates', + 'sms': 'Text message templates' + }[template_type] @main.route("/services//send/", methods=['GET']) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 5cff65c33..7fa2d0330 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -4,15 +4,10 @@ {% if current_user.has_permissions(['view_activity'], admin_override=True) %} {% endif %} - {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} - - {% elif current_user.has_permissions(['view_activity', 'manage_templates','manage_api_keys'], admin_override=True, any_=True) %} + {% if current_user.has_permissions(['view_activity', 'manage_templates', 'manage_api_keys'], admin_override=True, any_=True) %}
  • Text message templates
  • Email templates
    • @@ -20,17 +15,17 @@ {% endif %} {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %} {% elif current_user.has_permissions(['view_activity']) %} {% endif %} {% if current_user.has_permissions(['manage_api_keys']) %} {% endif %} {% if current_user.has_permissions(admin_override=True) %} diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 5f0cc021f..1f88b9631 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -19,11 +19,7 @@ Manage users – GOV.UK Notify

    - {% if current_user.has_permissions(['manage_users']) %} - Manage team - {% else %} - View team members - {% endif %} + Team members

    {% if current_user.has_permissions(['manage_users']) %} diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index eb72fb62e..a444cec9f 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -7,7 +7,7 @@ {% block maincolumn_content %} -

    Manage settings

    +

    Settings

    {{ browse_list([ { diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 92c4f0686..7c264d9e8 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -18,7 +18,7 @@ def test_should_show_overview_page( mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) response = client.get(url_for('main.manage_users', service_id=service['id'])) - assert 'Manage team' in response.get_data(as_text=True) + assert 'Team members' in response.get_data(as_text=True) assert response.status_code == 200 app.user_api_client.get_users_for_service.assert_called_once_with(service_id=service['id']) @@ -158,7 +158,7 @@ def test_invite_user( assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Manage team' + assert page.h1.string.strip() == 'Team members' flash_banner = page.find('div', class_='banner-default-with-tick').string.strip() assert flash_banner == 'Invite sent to test@example.gov.uk' excpected_permissions = 'manage_api_keys,manage_service,send_messages,view_activity' @@ -203,7 +203,7 @@ def test_manage_users_shows_invited_user(app_, assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Manage team' + assert page.h1.string.strip() == 'Team members' invites_table = page.find_all('table')[1] cols = invites_table.find_all('td') assert cols[0].text.strip() == 'invited_user@test.gov.uk' @@ -231,7 +231,7 @@ def test_manage_users_does_not_show_accepted_invite(app_, assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Manage team' + assert page.h1.string.strip() == 'Team members' tables = page.find_all('table') assert len(tables) == 1 assert not page.find(text='invited_user@test.gov.uk') @@ -275,7 +275,7 @@ def test_no_permission_manage_users_page(app_, resp_text = response.get_data(as_text=True) assert url_for('.invite_user', service_id=service_one['id']) not in resp_text assert "Edit permission" not in resp_text - assert "Manage team" not in resp_text + assert "Team members" not in resp_text def test_get_remove_user_from_service(app_,