Allow setting of caseworking on a user

This commit changes the form that the user sees when inviting or editing
another user, if the service has the ‘caseworking’ permission set.

This will allow creating a new type of user, one who only has the
`send_messages` permission, without the `view_activity` permission.

We are doing this because we think there are a number of services with a
lot of users who don’t need to see the dashboard, or the other team
members, and that we can make a simpler interface for these users.
This commit is contained in:
Chris Hill-Scott
2018-06-12 14:29:47 +01:00
parent d952b5ca3c
commit f4d2958d58
13 changed files with 436 additions and 47 deletions

View File

@@ -0,0 +1,25 @@
(function(Modules) {
"use strict";
Modules.ConditionalRadios = function() {
this.start = function(component) {
const $radios = $('[type=radio]', $(component)),
showHidePanels = function() {
$radios.each(function() {
$('#panel-' + $(this).attr('value'))
.toggleClass(
'js-hidden',
!$(this).is(":checked")
);
});
};
$radios.on('click', showHidePanels);
showHidePanels();
};
};
})(window.GOVUK.Modules);

View File

@@ -0,0 +1,26 @@
$border-thickness: 9px;
.multiple-choice {
z-index: 10;
.block-label {
&:before {
box-shadow: 0 4px 0 0 $white;
}
}
}
.conditional-radios {
&-panel {
border-left: $border-thickness solid $border-colour;
margin: 0 0 0 ($border-thickness + 6px);
padding: $gutter-one-third 0 0 ($gutter - 3px);
position: relative;
top: -$gutter-one-third;
z-index: 1;
}
}

View File

@@ -58,6 +58,7 @@ $path: '/static/images/';
@import 'components/live-search'; @import 'components/live-search';
@import 'components/stick-at-top-when-scrolling'; @import 'components/stick-at-top-when-scrolling';
@import 'components/fullscreen-table'; @import 'components/fullscreen-table';
@import 'components/conditional-radios';
@import 'components/vendor/breadcrumbs'; @import 'components/vendor/breadcrumbs';
@import 'components/vendor/responsive-embed'; @import 'components/vendor/responsive-embed';

View File

@@ -287,6 +287,28 @@ class AdminPermissionsForm(AbstractPermissionsForm):
self.view_activity.data = True self.view_activity.data = True
class CaseworkingPermissionsForm(AbstractPermissionsForm):
def process(self, *args, **kwargs):
super().process(*args, **kwargs)
if self.user_type.data == 'admin':
self.view_activity.data = True
elif self.user_type.data == 'caseworker':
self.view_activity.data = False
self.manage_templates.data = False
self.manage_service.data = False
self.manage_api_keys.data = False
self.send_messages.data = True
user_type = RadioField(
'User type',
choices=[
('caseworker', 'Caseworker'),
('admin', 'Admin'),
],
)
class AbstractInviteUserForm(StripWhitespaceForm): class AbstractInviteUserForm(StripWhitespaceForm):
email_address = email_address(gov_user=False) email_address = email_address(gov_user=False)
@@ -303,6 +325,10 @@ class AdminInviteUserForm(AbstractInviteUserForm, AdminPermissionsForm):
pass pass
class CaseworkingInviteUserForm(AbstractInviteUserForm, CaseworkingPermissionsForm):
pass
class InviteOrgUserForm(StripWhitespaceForm): class InviteOrgUserForm(StripWhitespaceForm):
email_address = email_address(gov_user=False) email_address = email_address(gov_user=False)

View File

@@ -1,3 +1,5 @@
from functools import partial
from flask import abort, flash, redirect, render_template, request, url_for from flask import abort, flash, redirect, render_template, request, url_for
from flask_login import current_user, login_required from flask_login import current_user, login_required
from notifications_python_client.errors import HTTPError from notifications_python_client.errors import HTTPError
@@ -12,6 +14,8 @@ from app.main import main
from app.main.forms import ( from app.main.forms import (
AdminInviteUserForm, AdminInviteUserForm,
AdminPermissionsForm, AdminPermissionsForm,
CaseworkingInviteUserForm,
CaseworkingPermissionsForm,
SearchUsersForm, SearchUsersForm,
) )
from app.notify_client.models import roles from app.notify_client.models import roles
@@ -44,9 +48,12 @@ def manage_users(service_id):
@user_has_permissions('manage_service') @user_has_permissions('manage_service')
def invite_user(service_id): def invite_user(service_id):
form = AdminInviteUserForm( if 'caseworking' in current_service['permissions']:
invalid_email_address=current_user.email_address form = CaseworkingInviteUserForm
) else:
form = AdminInviteUserForm
form = form(invalid_email_address=current_user.email_address)
service_has_email_auth = 'email_auth' in current_service['permissions'] service_has_email_auth = 'email_auth' in current_service['permissions']
if not service_has_email_auth: if not service_has_email_auth:
@@ -82,10 +89,19 @@ def edit_user_permissions(service_id, user_id):
user = user_api_client.get_user(user_id) user = user_api_client.get_user(user_id)
user_has_no_mobile_number = user.mobile_number is None user_has_no_mobile_number = user.mobile_number is None
form = AdminPermissionsForm( if 'caseworking' in current_service['permissions']:
form = partial(
CaseworkingPermissionsForm,
user_type='admin' if user.has_permission_for_service(service_id, 'view_activity') else 'caseworker',
)
else:
form = AdminPermissionsForm
form = form(
**{role: user.has_permission_for_service(service_id, role) for role in roles.keys()}, **{role: user.has_permission_for_service(service_id, role) for role in roles.keys()},
login_authentication=user.auth_type login_authentication=user.auth_type
) )
if form.validate_on_submit(): if form.validate_on_submit():
user_api_client.set_user_permissions( user_api_client.set_user_permissions(
user_id, service_id, user_id, service_id,

View File

@@ -185,7 +185,9 @@ class InvitedUser(object):
return set(self.permissions) > set(permissions) return set(self.permissions) > set(permissions)
def has_permission_for_service(self, service_id, permission): def has_permission_for_service(self, service_id, permission):
return self.status != 'cancelled' and self.service == service_id and permission in self.permissions if self.status == 'cancelled' and permission != 'view_activity':
return False
return self.service == service_id and permission in self.permissions
def __eq__(self, other): def __eq__(self, other):
return ((self.id, return ((self.id,

View File

@@ -135,3 +135,9 @@
</fieldset> </fieldset>
</div> </div>
{% endmacro %} {% endmacro %}
{% macro conditional_radio_panel(id) %}
<div class="js-hidden panel panel-border-wide conditional-radios-panel" id="panel-{{ id }}">
{{ caller() }}
</div>
{% endmacro %}

View File

@@ -63,22 +63,35 @@
</h3> </h3>
<ul class="tick-cross-list"> <ul class="tick-cross-list">
<div class="tick-cross-list-permissions"> <div class="tick-cross-list-permissions">
{{ tick_cross( {% if user.has_permission_for_service(current_service.id, 'view_activity') %}
user.has_permission_for_service(current_service.id, 'send_messages'), {% if 'caseworking' in current_service.permissions %}
'Send messages' {{ tick_cross(
) }} user.status != 'cancelled',
{{ tick_cross( 'Admin'
user.has_permission_for_service(current_service.id, 'manage_templates'), ) }}
'Add and edit templates' {% endif %}
) }} {{ tick_cross(
{{ tick_cross( user.has_permission_for_service(current_service.id, 'send_messages'),
user.has_permission_for_service(current_service.id, 'manage_service'), 'Send messages'
'Manage service' ) }}
) }} {{ tick_cross(
{{ tick_cross( user.has_permission_for_service(current_service.id, 'manage_templates'),
user.has_permission_for_service(current_service.id, 'manage_api_keys'), 'Add and edit templates'
'Access API keys' ) }}
) }} {{ tick_cross(
user.has_permission_for_service(current_service.id, 'manage_service'),
'Manage service'
) }}
{{ tick_cross(
user.has_permission_for_service(current_service.id, 'manage_api_keys'),
'Access API keys'
) }}
{% else %}
{{ tick_cross(
True,
'Caseworker'
) }}
{% endif %}
{% if 'email_auth' in current_service['permissions'] %} {% if 'email_auth' in current_service['permissions'] %}
<div class="tick-cross-list-hint"> <div class="tick-cross-list-hint">
{% if user.auth_type == 'sms_auth' %} {% if user.auth_type == 'sms_auth' %}

View File

@@ -1,26 +1,58 @@
{% from "components/checkbox.html" import checkbox %} {% from "components/checkbox.html" import checkbox %}
{% from "components/radios.html" import radios %} {% from "components/radios.html" import radio, radios, radios_wrapper, conditional_radio_panel %}
<fieldset class="form-group"> {% if 'caseworking' in current_service.permissions %}
<legend class="form-label"> <div data-module="conditional-radios">
Permissions {% call radios_wrapper(form.user_type, hide_legend=True) %}
</legend> {% for option in form.user_type %}
{{ checkbox(form.send_messages) }} {{ radio(option) }}
{{ checkbox(form.manage_templates) }} {% if option.data == 'admin' %}
{{ checkbox(form.manage_service) }} {% call conditional_radio_panel(option.data) %}
{{ checkbox(form.manage_api_keys) }} <div class="bottom-gutter-1-2">
</fieldset> <p class="form-label">
All admin users can see
<div class="bottom-gutter"> </p>
<p class="form-label"> <ul class="list list-bullet">
All team members can see <li>templates</li>
</p> <li>history of sent messages</li>
<ul class="list list-bullet"> <li>who the other team members are</li>
<li>templates</li> </ul>
<li>history of sent messages</li> </div>
<li>who the other team members are</li> <fieldset class="form-group">
</ul> <legend class="form-label">
</div> Permissions
</legend>
{{ checkbox(form.send_messages) }}
{{ checkbox(form.manage_templates) }}
{{ checkbox(form.manage_service) }}
{{ checkbox(form.manage_api_keys) }}
</fieldset>
{% endcall %}
{% endif %}
{% endfor %}
{% endcall %}
</div>
{% else %}
<fieldset class="form-group">
<legend class="form-label">
Permissions
</legend>
{{ checkbox(form.send_messages) }}
{{ checkbox(form.manage_templates) }}
{{ checkbox(form.manage_service) }}
{{ checkbox(form.manage_api_keys) }}
</fieldset>
<div class="bottom-gutter">
<p class="form-label">
All team members can see
</p>
<ul class="list list-bullet">
<li>templates</li>
<li>history of sent messages</li>
<li>who the other team members are</li>
</ul>
</div>
{% endif %}
{% if service_has_email_auth %} {% if service_has_email_auth %}
{% if user_has_no_mobile_number %} {% if user_has_no_mobile_number %}

View File

@@ -73,6 +73,7 @@ gulp.task('javascripts', () => gulp
paths.src + 'javascripts/errorTracking.js', paths.src + 'javascripts/errorTracking.js',
paths.src + 'javascripts/preventDuplicateFormSubmissions.js', paths.src + 'javascripts/preventDuplicateFormSubmissions.js',
paths.src + 'javascripts/fullscreenTable.js', paths.src + 'javascripts/fullscreenTable.js',
paths.src + 'javascripts/conditionalRadios.js',
paths.src + 'javascripts/main.js' paths.src + 'javascripts/main.js'
]) ])
.pipe(plugins.prettyerror()) .pipe(plugins.prettyerror())

View File

@@ -24,7 +24,7 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(
mocker, mocker,
): ):
expected_service = service_one['id'] expected_service = service_one['id']
expected_permissions = {'send_messages', 'manage_service', 'manage_api_keys'} expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'}
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'))
@@ -123,7 +123,7 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(
mocker, mocker,
): ):
expected_service = service_one['id'] expected_service = service_one['id']
expected_permissions = {'send_messages', 'manage_service', 'manage_api_keys'} expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'}
response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True)
@@ -401,7 +401,7 @@ def test_new_invited_user_verifies_and_added_to_service(
# when they post codes back to admin user should be added to # when they post codes back to admin user should be added to
# service and sent on to dash board # service and sent on to dash board
expected_permissions = {'send_messages', 'manage_service', 'manage_api_keys'} expected_permissions = {'view_activity', 'send_messages', 'manage_service', 'manage_api_keys'}
with client.session_transaction() as session: with client.session_transaction() as session:
new_user_id = session['user_id'] new_user_id = session['user_id']

View File

@@ -9,6 +9,7 @@ from app.notify_client.models import InvitedUser
from app.utils import is_gov_user from app.utils import is_gov_user
from tests.conftest import ( from tests.conftest import (
SERVICE_ONE_ID, SERVICE_ONE_ID,
active_caseworking_user,
active_user_manage_template_permission, active_user_manage_template_permission,
active_user_no_mobile, active_user_no_mobile,
active_user_view_permissions, active_user_view_permissions,
@@ -53,12 +54,24 @@ from tests.conftest import service_one as create_sample_service
'Cant Send messages Cant Add and edit templates Cant Manage service Cant Access API keys' 'Cant Send messages Cant Add and edit templates Cant Manage service Cant Access API keys'
) )
), ),
(
active_user_manage_template_permission,
(
'Test User With Permissions (you) '
'Cant Send messages Can Add and edit templates Cant Manage service Cant Access API keys'
),
(
'ZZZZZZZZ zzzzzzz@example.gov.uk '
'Cant Send messages Cant Add and edit templates Cant Manage service Cant Access API keys'
)
),
]) ])
def test_should_show_overview_page( def test_should_show_overview_page(
client_request, client_request,
mocker, mocker,
mock_get_invites_for_service, mock_get_invites_for_service,
fake_uuid, fake_uuid,
service_one,
user, user,
expected_self_text, expected_self_text,
expected_coworker_text, expected_coworker_text,
@@ -85,6 +98,42 @@ def test_should_show_overview_page(
app.user_api_client.get_users_for_service.assert_called_once_with(service_id=SERVICE_ONE_ID) app.user_api_client.get_users_for_service.assert_called_once_with(service_id=SERVICE_ONE_ID)
def test_should_show_caseworker_on_overview_page(
client_request,
mocker,
mock_get_invites_for_service,
fake_uuid,
service_one,
):
service_one['permissions'].append('caseworking')
current_user = active_user_view_permissions(active_user_view_permissions)
other_user = active_caseworking_user(fake_uuid)
other_user.email_address = 'zzzzzzz@example.gov.uk'
mocker.patch('app.user_api_client.get_user', return_value=current_user)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[
current_user,
other_user,
])
page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
assert normalize_spaces(page.select_one('h1').text) == 'Team members'
assert normalize_spaces(page.select('.user-list-item')[0].text) == (
'Test User With Permissions (you) '
'Can Admin '
'Cant Send messages '
'Cant Add and edit templates '
'Cant Manage service '
'Cant Access API keys'
)
# [1:5] are invited users
assert normalize_spaces(page.select('.user-list-item')[6].text) == (
'Test User zzzzzzz@example.gov.uk '
'Can Caseworker'
)
@pytest.mark.parametrize('endpoint, extra_args, service_has_email_auth, auth_options_hidden', [ @pytest.mark.parametrize('endpoint, extra_args, service_has_email_auth, auth_options_hidden', [
( (
'main.edit_user_permissions', 'main.edit_user_permissions',
@@ -125,6 +174,59 @@ def test_service_with_no_email_auth_hides_auth_type_options(
assert (page.find('input', attrs={"name": "login_authentication"}) is None) == auth_options_hidden assert (page.find('input', attrs={"name": "login_authentication"}) is None) == auth_options_hidden
@pytest.mark.parametrize('endpoint, extra_args, service_has_caseworking, radio_buttons_on_page', [
(
'main.edit_user_permissions',
{'user_id': 0},
True,
True,
),
(
'main.edit_user_permissions',
{'user_id': 0},
False,
False,
),
(
'main.invite_user',
{},
True,
True,
),
(
'main.invite_user',
{},
False,
False,
)
])
def test_service_without_caseworking_doesnt_show_admin_vs_caseworker(
client_request,
endpoint,
extra_args,
service_has_caseworking,
radio_buttons_on_page,
service_one
):
if service_has_caseworking:
service_one['permissions'].append('caseworking')
page = client_request.get(endpoint, service_id=service_one['id'], **extra_args)
radio_buttons = page.select('input[name=user_type]')
admin_permissions_panel = page.select_one('#panel-admin')
if radio_buttons_on_page:
assert radio_buttons[0]['type'] == 'radio'
assert radio_buttons[0]['value'] == 'caseworker'
assert radio_buttons[1]['type'] == 'radio'
assert radio_buttons[1]['value'] == 'admin'
assert admin_permissions_panel.select('input')[0]['name'] == 'send_messages'
assert admin_permissions_panel.select('input')[1]['name'] == 'manage_templates'
assert admin_permissions_panel.select('input')[2]['name'] == 'manage_service'
assert admin_permissions_panel.select('input')[3]['name'] == 'manage_api_keys'
else:
assert not radio_buttons
assert not admin_permissions_panel
@pytest.mark.parametrize('service_has_email_auth, displays_auth_type', [ @pytest.mark.parametrize('service_has_email_auth, displays_auth_type', [
(True, True), (True, True),
(False, False) (False, False)
@@ -343,6 +445,85 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service(
) )
def test_edit_user_to_be_admin(
client_request,
active_user_with_permissions,
mocker,
mock_get_invites_for_service,
mock_set_user_permissions,
service_one,
):
service_one['permissions'].append('caseworking')
client_request.post(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data={
'email_address': active_user_with_permissions.email_address,
'user_type': 'admin',
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
},
_expected_redirect=url_for(
'main.manage_users', service_id=SERVICE_ONE_ID, _external=True
),
)
mock_set_user_permissions.assert_called_with(
str(active_user_with_permissions.id),
SERVICE_ONE_ID,
permissions={
'send_messages',
'manage_service',
'manage_templates',
'manage_api_keys',
'view_activity'
}
)
@pytest.mark.parametrize('extra_args', (
# The user shouldnt be able to forge a request which makes a
# caseworker without the send permission…
({'send_messages': 'n'}),
# …or with any additional permissions
({'manage_templates': 'y'}),
({'manage_service': 'y'}),
({'manage_api_keys': 'y'}),
))
def test_edit_user_to_be_caseworker(
client_request,
active_user_with_permissions,
mocker,
mock_get_invites_for_service,
mock_set_user_permissions,
service_one,
extra_args,
):
service_one['permissions'].append('caseworking')
client_request.post(
'main.edit_user_permissions',
service_id=SERVICE_ONE_ID,
user_id=active_user_with_permissions.id,
_data=dict(
email_address=active_user_with_permissions.email_address,
user_type='caseworker',
**extra_args
),
_expected_redirect=url_for(
'main.manage_users', service_id=SERVICE_ONE_ID, _external=True
),
)
mock_set_user_permissions.assert_called_with(
str(active_user_with_permissions.id),
SERVICE_ONE_ID,
permissions={
'send_messages',
}
)
def test_should_show_page_for_inviting_user( def test_should_show_page_for_inviting_user(
logged_in_client, logged_in_client,
active_user_with_permissions, active_user_with_permissions,
@@ -452,6 +633,41 @@ def test_invite_user_with_email_auth_service(
auth_type) auth_type)
@pytest.mark.parametrize('extra_args', (
{},
{
'send_messages': 'y',
'manage_templates': 'y',
'manage_service': 'y',
'manage_api_keys': 'y',
},
))
def test_invite_user_must_choose_caseworker_or_admin(
client_request,
mock_set_user_permissions,
service_one,
fake_uuid,
extra_args,
):
service_one['permissions'].append('caseworking')
page = client_request.post(
'main.invite_user',
service_id=service_one['id'],
user_id=fake_uuid,
_data={
'email_address': 'test@example.com',
**extra_args
},
_expected_status=200,
)
assert page.select_one('.error-message').text.strip() == (
'Not a valid choice'
)
assert mock_set_user_permissions.called is False
for form_input in page.select('form input'):
assert 'checked' not in form_input
def test_cancel_invited_user_cancels_user_invitations( def test_cancel_invited_user_cancels_user_invitations(
logged_in_client, logged_in_client,
active_user_with_permissions, active_user_with_permissions,
@@ -496,7 +712,6 @@ def test_manage_users_shows_invited_user(
): ):
sample_invite['status'] = invite_status sample_invite['status'] = invite_status
data = [InvitedUser(**sample_invite)] data = [InvitedUser(**sample_invite)]
mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])

View File

@@ -1161,6 +1161,32 @@ def active_user_with_permissions(fake_uuid):
return user return user
@pytest.fixture(scope='function')
def active_caseworking_user(fake_uuid):
from app.notify_client.user_api_client import User
user_data = {
'id': fake_uuid,
'name': 'Test User',
'password': 'somepassword',
'password_changed_at': str(datetime.utcnow()),
'email_address': 'caseworker@example.gov.uk',
'mobile_number': '07700 900762',
'state': 'active',
'failed_login_count': 0,
'permissions': {SERVICE_ONE_ID: [
'send_texts',
'send_emails',
'send_letters',
]},
'platform_admin': False,
'auth_type': 'sms_auth',
'organisations': [],
}
user = User(user_data)
return user
@pytest.fixture(scope='function') @pytest.fixture(scope='function')
def active_user_no_mobile(fake_uuid): def active_user_no_mobile(fake_uuid):
from app.notify_client.user_api_client import User from app.notify_client.user_api_client import User
@@ -2046,7 +2072,7 @@ def sample_invite(mocker, service_one, status='pending'):
from_user = service_one['users'][0] from_user = service_one['users'][0]
email_address = 'invited_user@test.gov.uk' email_address = 'invited_user@test.gov.uk'
service_id = service_one['id'] service_id = service_one['id']
permissions = 'send_messages,manage_service,manage_api_keys' permissions = 'view_activity,send_messages,manage_service,manage_api_keys'
created_at = str(datetime.utcnow()) created_at = str(datetime.utcnow())
auth_type = 'sms_auth' auth_type = 'sms_auth'