From c724f84c2354a374719fde97d64d6390eee8bdf5 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 14 Jun 2019 12:32:47 +0100 Subject: [PATCH] change wording of platform admin toggle to positive rather than negative CHS Approved Wording :+1: also rename suppress_platform_admin -> disable_platform_admin_view in the backend, as suppress is a kinda weird word. --- app/main/views/user_profile.py | 27 +++++--- app/models/user.py | 2 +- app/templates/views/user-profile.html | 8 +-- ....html => disable-platform-admin-view.html} | 4 +- tests/app/main/views/test_user_profile.py | 62 +++++++++---------- tests/app/models/test_user.py | 4 +- 6 files changed, 57 insertions(+), 50 deletions(-) rename app/templates/views/user-profile/{suppress-platform-admin.html => disable-platform-admin-view.html} (90%) diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py index d69398102..f136c61b7 100644 --- a/app/main/views/user_profile.py +++ b/app/main/views/user_profile.py @@ -1,6 +1,13 @@ import json -from flask import current_app, redirect, render_template, session, url_for, abort +from flask import ( + abort, + current_app, + redirect, + render_template, + session, + url_for, +) from flask_login import current_user, login_required from notifications_utils.url_safe_token import check_token @@ -195,24 +202,24 @@ def user_profile_password(): ) -@main.route("/user-profile/suppress-platform-admin", methods=['GET', 'POST']) +@main.route("/user-profile/disable-platform-admin-view", methods=['GET', 'POST']) @login_required -def user_profile_suppress_platform_admin(): - if not current_user.platform_admin and not session.get('suppress_platform_admin'): +def user_profile_disable_platform_admin_view(): + if not current_user.platform_admin and not session.get('disable_platform_admin_view'): abort(403) form = ServiceOnOffSettingForm( - name="This setting will be cleared if you sign out and sign in again", - enabled=session.get('suppress_platform_admin', False), - truthy='Yes (view as regular user)', - falsey='No (view as platform admin)', + name="Signing in again clears this setting", + enabled=not session.get('disable_platform_admin_view'), + truthy='Yes', + falsey='No', ) if form.validate_on_submit(): - session['suppress_platform_admin'] = form.enabled.data + session['disable_platform_admin_view'] = not form.enabled.data return redirect(url_for('.user_profile')) return render_template( - 'views/user-profile/suppress-platform-admin.html', + 'views/user-profile/disable-platform-admin-view.html', form=form ) diff --git a/app/models/user.py b/app/models/user.py index 10921dee4..2fa83b318 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -176,7 +176,7 @@ class User(JSONModel, UserMixin): @property def platform_admin(self): - return self._platform_admin and not session.get('suppress_platform_admin', False) + return self._platform_admin and not session.get('disable_platform_admin_view', False) def has_permissions(self, *permissions, restrict_admin_usage=False): unknown_permissions = set(permissions) - all_permissions diff --git a/app/templates/views/user-profile.html b/app/templates/views/user-profile.html index b259a2d93..a39853c32 100644 --- a/app/templates/views/user-profile.html +++ b/app/templates/views/user-profile.html @@ -45,11 +45,11 @@ {{ edit_field('Change', url_for('.user_profile_password')) }} {% endcall %} - {% if current_user.platform_admin or session.get('suppress_platform_admin') %} + {% if current_user.platform_admin or session.get('disable_platform_admin_view') %} {% call row() %} - {{ text_field('Suppress Platform Admin') }} - {{ text_field('On' if session.get('suppress_platform_admin') else 'Off') }} - {{ edit_field('Change', url_for('.user_profile_suppress_platform_admin')) }} + {{ text_field('Use platform admin view') }} + {{ text_field('Yes' if not session.get('disable_platform_admin_view') else 'No') }} + {{ edit_field('Change', url_for('.user_profile_disable_platform_admin_view')) }} {% endcall %} {% endif %} diff --git a/app/templates/views/user-profile/suppress-platform-admin.html b/app/templates/views/user-profile/disable-platform-admin-view.html similarity index 90% rename from app/templates/views/user-profile/suppress-platform-admin.html rename to app/templates/views/user-profile/disable-platform-admin-view.html index a579f001c..334de0b13 100644 --- a/app/templates/views/user-profile/suppress-platform-admin.html +++ b/app/templates/views/user-profile/disable-platform-admin-view.html @@ -5,7 +5,7 @@ {% from "components/radios.html" import radios %} {% block per_page_title %} - Suppress platform admin view + Use platform admin view {% endblock %} {% block maincolumn_content %} @@ -13,7 +13,7 @@
{{ page_header( - 'Suppress platform admin', + 'Use platform admin view', back_link=url_for('.user_profile') ) }} diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index f77756e7c..afd0f9a79 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -14,18 +14,18 @@ def test_should_show_overview_page( ): page = client_request.get(('main.user_profile')) assert page.select_one('h1').text.strip() == 'Your profile' - assert 'Suppress Platform Admin' not in page + assert 'Use platform admin view' not in page -def test_overview_page_shows_suppress_for_platform_admin( +def test_overview_page_shows_disable_for_platform_admin( client_request, platform_admin_user ): client_request.login(platform_admin_user) page = client_request.get(('main.user_profile')) assert page.select_one('h1').text.strip() == 'Your profile' - suppress_platform_admin_row = page.select('tr')[-1] - assert ' '.join(suppress_platform_admin_row.text.split()) == 'Suppress Platform Admin Off Change' + disable_platform_admin_row = page.select('tr')[-1] + assert ' '.join(disable_platform_admin_row.text.split()) == 'Use platform admin view Yes Change' def test_should_show_name_page( @@ -278,47 +278,47 @@ def test_non_gov_user_cannot_access_change_email_page( client_request.get('main.user_profile_email', _expected_status=403) -def test_normal_user_doesnt_see_suppress_platform_admin(client_request): - client_request.get('main.user_profile_suppress_platform_admin', _expected_status=403) +def test_normal_user_doesnt_see_disable_platform_admin(client_request): + client_request.get('main.user_profile_disable_platform_admin_view', _expected_status=403) -def test_platform_admin_can_see_suppress_platform_admin_page(client_request, platform_admin_user): +def test_platform_admin_can_see_disable_platform_admin_page(client_request, platform_admin_user): client_request.login(platform_admin_user) - page = client_request.get('main.user_profile_suppress_platform_admin') + page = client_request.get('main.user_profile_disable_platform_admin_view') - assert page.select_one('h1').text.strip() == 'Suppress platform admin' - assert page.select_one('input[checked]')['value'] == 'False' + assert page.select_one('h1').text.strip() == 'Use platform admin view' + assert page.select_one('input[checked]')['value'] == 'True' -def test_can_suppress_platform_admin(client_request, platform_admin_user): +def test_can_disable_platform_admin(client_request, platform_admin_user): client_request.login(platform_admin_user) with client_request.session_transaction() as session: - assert 'suppress_platform_admin' not in session + assert 'disable_platform_admin_view' not in session client_request.post( - 'main.user_profile_suppress_platform_admin', - _data={'enabled': True}, - _expected_status=302, - _expected_redirect=url_for('main.user_profile', _external=True), - ) - - with client_request.session_transaction() as session: - assert session['suppress_platform_admin'] is True - - -def test_can_turn_off_suppress_platform_admin(client_request, platform_admin_user): - client_request.login(platform_admin_user) - - with client_request.session_transaction() as session: - session['suppress_platform_admin'] = True - - client_request.post( - 'main.user_profile_suppress_platform_admin', + 'main.user_profile_disable_platform_admin_view', _data={'enabled': False}, _expected_status=302, _expected_redirect=url_for('main.user_profile', _external=True), ) with client_request.session_transaction() as session: - assert session['suppress_platform_admin'] is False + assert session['disable_platform_admin_view'] is True + + +def test_can_reenable_platform_admin(client_request, platform_admin_user): + client_request.login(platform_admin_user) + + with client_request.session_transaction() as session: + session['disable_platform_admin_view'] = True + + client_request.post( + 'main.user_profile_disable_platform_admin_view', + _data={'enabled': True}, + _expected_status=302, + _expected_redirect=url_for('main.user_profile', _external=True), + ) + + with client_request.session_transaction() as session: + assert session['disable_platform_admin_view'] is False diff --git a/tests/app/models/test_user.py b/tests/app/models/test_user.py index bae3189bd..edc69b51e 100644 --- a/tests/app/models/test_user.py +++ b/tests/app/models/test_user.py @@ -51,10 +51,10 @@ def test_activate_user_already_active(app_, api_user_active, mock_activate_user) (False, False, False), (False, None, False), ]) -def test_platform_admin_checks_flag_set_in_session(client, mocker, is_platform_admin, value_in_session, expected_result): +def test_platform_admin_flag_set_in_session(client, mocker, is_platform_admin, value_in_session, expected_result): session_dict = {} if value_in_session is not None: - session_dict['suppress_platform_admin'] = value_in_session + session_dict['disable_platform_admin_view'] = value_in_session mocker.patch.dict('app.models.user.session', values=session_dict, clear=True)