From 95d138b6382fbdfce2893da0db438cdeb1d31f11 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 10 Jul 2018 11:26:07 +0100 Subject: [PATCH] Add a preview of basic view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of the big things we found in user research was that people were uncertain what the effect of giving someone basic view was. So in the spirit of ‘show don’t tell’, this commit adds a way for users to preview basic view. They can go into the preview and click around as much as they like, just as if they really had the basic view assigned to them. Once they have seen enough they can return to the settings page where they can decide whether or not to switch basic view on for real. --- .../stylesheets/components/navigation.scss | 23 ++++++ app/main/views/service_settings.py | 17 ++++- app/navigation.py | 4 + app/notify_client/models.py | 8 +- app/templates/error/403.html | 2 +- .../service-settings/set-basic-view.html | 3 + app/templates/withnav_template.html | 4 + tests/app/main/views/test_service_settings.py | 75 +++++++++++++++++++ 8 files changed, 133 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/components/navigation.scss b/app/assets/stylesheets/components/navigation.scss index d6c3531d7..0e4dedfe1 100644 --- a/app/assets/stylesheets/components/navigation.scss +++ b/app/assets/stylesheets/components/navigation.scss @@ -14,6 +14,29 @@ margin: 0 0 0 0; } + &-basic-view { + + &-preview { + @include bold-16; + display: inline-block; + background: $yellow; + padding: 4px 10px 3px 10px; + position: relative; + top: -1px; + margin-left: 8px; + margin-top: -1px; + margin-bottom: -7px; + } + + &-back-link { + @include core-16; + display: inline-block; + font-weight: normal; + margin-left: 10px; + } + + } + &-switch { @include core-16; diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index f44763a06..3c4c0ffa7 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -630,8 +630,15 @@ def service_set_auth_type(service_id): @main.route("/services//service-settings/set-basic-view", methods=['GET', 'POST']) @login_required -@user_has_permissions('manage_service') +@user_has_permissions('manage_service', 'send_messages') def service_set_basic_view(service_id): + + if current_user.previewing_basic_view: + session.pop('basic', None) + + if not current_user.has_permissions('manage_service'): + abort(403) + form = ServiceBasicViewForm( enabled='caseworking' in current_service['permissions'] ) @@ -650,6 +657,14 @@ def service_set_basic_view(service_id): ) +@main.route("/services//preview-basic-view") +@login_required +@user_has_permissions('manage_service') +def preview_basic_view(service_id): + session['basic'] = True + return redirect(url_for('.service_dashboard', service_id=service_id)) + + @main.route("/services//service-settings/letter-contacts", methods=['GET']) @login_required @user_has_permissions('manage_service', 'manage_api_keys') diff --git a/app/navigation.py b/app/navigation.py index 122e3f6f3..a993c6270 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -172,6 +172,7 @@ class HeaderNavigation(Navigation): 'old_using_notify', 'organisation_dashboard', 'organisation_settings', + 'preview_basic_view', 'privacy', 'public_agreement', 'public_download_agreement', @@ -426,6 +427,7 @@ class MainNavigation(Navigation): 'platform_admin', 'platform_admin_new', 'platform_admin_list_complaints', + 'preview_basic_view', 'pricing', 'privacy', 'public_agreement', @@ -599,6 +601,7 @@ class CaseworkNavigation(Navigation): 'platform_admin', 'platform_admin_new', 'platform_admin_list_complaints', + 'preview_basic_view', 'pricing', 'privacy', 'public_agreement', @@ -817,6 +820,7 @@ class OrgNavigation(Navigation): 'platform_admin', 'platform_admin_new', 'platform_admin_list_complaints', + 'preview_basic_view', 'pricing', 'privacy', 'public_agreement', diff --git a/app/notify_client/models.py b/app/notify_client/models.py index bc7a6e7c3..3ccdce48b 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -76,7 +76,6 @@ class User(UserMixin): between on the front end. So lets collapse them into "send_messages" and "manage_service". If we want to split them out later, we'll need to rework this function. """ - self._permissions = { service: translate_permissions_from_db_to_admin_roles(permissions) for service, permissions @@ -109,6 +108,10 @@ class User(UserMixin): def permissions(self, permissions): raise AttributeError("Read only property") + @property + def previewing_basic_view(self): + return bool(session.get('basic')) + def has_permissions(self, *permissions, restrict_admin_usage=False): unknown_permissions = set(permissions) - all_permissions @@ -119,6 +122,9 @@ class User(UserMixin): service_id = _get_service_id_from_view_args() org_id = _get_org_id_from_view_args() + if self.previewing_basic_view: + return self._permissions.get(service_id) and 'send_messages' in permissions + if not service_id and not org_id: # we shouldn't have any pages that require permissions, but don't specify a service or organisation. # use @user_is_platform_admin for platform admin only pages diff --git a/app/templates/error/403.html b/app/templates/error/403.html index 105933b94..b6573c393 100644 --- a/app/templates/error/403.html +++ b/app/templates/error/403.html @@ -1,5 +1,5 @@ {% extends "withoutnav_template.html" %} -{% block per_page_title %}Forbidden{% endblock %} +{% block per_page_title %}403{% endblock %} {% block maincolumn_content %}
diff --git a/app/templates/views/service-settings/set-basic-view.html b/app/templates/views/service-settings/set-basic-view.html index 1418b4c0a..535d9b70c 100644 --- a/app/templates/views/service-settings/set-basic-view.html +++ b/app/templates/views/service-settings/set-basic-view.html @@ -21,6 +21,9 @@

You’ll get to choose which team members have basic view.

+

+ See a preview of basic view. +

{{ radios(form.enabled, hide_legend=True) }} {{ page_footer( 'Save', diff --git a/app/templates/withnav_template.html b/app/templates/withnav_template.html index 4f3221103..c03b77803 100644 --- a/app/templates/withnav_template.html +++ b/app/templates/withnav_template.html @@ -9,6 +9,10 @@
{{ current_service.name }} + {% if current_user.previewing_basic_view %} + Preview of basic view + Back to settings + {% endif %}
Switch service
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 8d74dfda5..fb98cc2cf 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -2406,6 +2406,10 @@ def test_see_basic_view_page( service_id=SERVICE_ONE_ID ) assert page.h1.text.strip() == 'Basic view' + assert page.select_one('main a')['href'] == url_for( + 'main.preview_basic_view', + service_id=SERVICE_ONE_ID, + ) assert page.select_one('input[checked]')['value'] == expected_selected @@ -2442,6 +2446,77 @@ def test_update_basic_view( ) == expected_updated_permissions +def test_preview_basic_view( + client_request, + mock_get_service_templates, +): + page = client_request.get( + "main.preview_basic_view", + service_id=SERVICE_ONE_ID, + _follow_redirects=True, + ) + + with client_request.session_transaction() as session: + assert session['basic'] is True + + assert page.h1.text.strip() == 'Choose a template' + page.select('.navigation-service-basic-view-preview') + assert normalize_spaces(page.select_one('.navigation-service').text) == ( + 'service one ' + 'Preview of basic view ' + 'Back to settings ' + 'Switch service' + ) + page.select_one('a.navigation-service-basic-view-back-link')['href'] == url_for( + 'main.service_set_basic_view', + service_id=SERVICE_ONE_ID, + ) + + client_request.get( + "main.service_set_basic_view", + service_id=SERVICE_ONE_ID, + ) + + with client_request.session_transaction() as session: + assert 'basic' not in session + + +def test_cant_preview_basic_view_for_another_service( + client_request, + mock_get_service_templates, + fake_uuid, +): + client_request.get( + "main.preview_basic_view", + service_id=SERVICE_ONE_ID, + _follow_redirects=True, + _expected_status=200, + ) + client_request.get( + "main.preview_basic_view", + service_id=fake_uuid, + _expected_status=403, + ) + + +def test_actual_basic_view_cant_escape_basic_view( + client_request, + active_caseworking_user, + mock_get_service_templates, +): + client_request.login(active_caseworking_user) + client_request.get( + "main.preview_basic_view", + service_id=SERVICE_ONE_ID, + _expected_status=403, + ) + client_request.get( + "main.service_set_basic_view", + service_id=SERVICE_ONE_ID, + _expected_status=403, + ) + + def test_service_settings_when_inbound_number_is_not_set( logged_in_client, service_one,