diff --git a/app/assets/images/cross.png b/app/assets/images/cross.png new file mode 100644 index 000000000..fab3b895c Binary files /dev/null and b/app/assets/images/cross.png differ diff --git a/app/assets/images/tick-white.png b/app/assets/images/tick-white.png new file mode 100644 index 000000000..c08e57c4f Binary files /dev/null and b/app/assets/images/tick-white.png differ diff --git a/app/assets/images/tick.png b/app/assets/images/tick.png new file mode 100644 index 000000000..77591a595 Binary files /dev/null and b/app/assets/images/tick.png differ diff --git a/app/assets/images/tick.psd b/app/assets/images/tick.psd new file mode 100644 index 000000000..7f90c3098 Binary files /dev/null and b/app/assets/images/tick.psd differ diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index e4a629217..57b5db45a 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -66,3 +66,16 @@ a { font-family: monospace; overflow-x: scroll; } + +.inline { + + .block-label { + + @include media(tablet) { + float: none; + display: inline-block; + } + + } + +} diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 3a8898c83..7c2fd2b99 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -16,19 +16,13 @@ .banner-with-tick, .banner-default-with-tick { - @extend %banner; padding: $gutter-half ($gutter + $gutter-half); - - &:before { - @include core-24; - content: '✔'; - position: absolute; - top: $gutter-half; - left: $gutter-half; - margin-top: -2px; - } - + background-image: file-url('tick-white.png'); + background-size: 19px; + background-repeat: no-repeat; + background-position: $gutter-half $gutter-half; + font-weight: bold; } .banner-dangerous { diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 1b40c5fa8..477ddb208 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -7,6 +7,12 @@ margin: 40px 0 5px 0; } +.table-field-headings { + th { + padding: 0 0 5px 0; + } +} + %table-field, .table-field { @@ -36,6 +42,23 @@ } + &-yes, + &-no { + display: block; + text-indent: -999em; + background-size: 19px 19px; + background-repeat: no-repeat; + background-position: 50% 50%; + } + + &-yes { + background-image: file-url('tick.png'); + } + + &-no { + background-image: file-url('cross.png'); + } + &-missing { color: $error-colour; font-weight: bold; @@ -77,4 +100,5 @@ margin-top: -20px; border-bottom: 1px solid $border-colour; padding-bottom: 10px; + text-align: center; } diff --git a/app/assets/stylesheets/components/yes-no.scss b/app/assets/stylesheets/components/yes-no.scss new file mode 100644 index 000000000..9a71113a5 --- /dev/null +++ b/app/assets/stylesheets/components/yes-no.scss @@ -0,0 +1,36 @@ +.yes-no-wrapper { + border-bottom: 1px solid $border-colour; + margin: 0 0 $gutter 0; +} + +.yes-no { + + border-top: 1px solid $border-colour; + padding: 10px 0; + + &-label { + padding-top: 19px; + float: left; + } + + &-fields { + + text-align: right; + + .block-label { + + @include media(tablet) { + + margin-bottom: 0; + + &:last-child { + margin-right: 0; + } + + } + + } + + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index eb420be44..e9c36ad7a 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -47,6 +47,7 @@ $path: '/static/images/'; @import 'components/browse-list'; @import 'components/email-message'; @import 'components/api-key'; +@import 'components/yes-no'; @import 'views/job'; @import 'views/edit-template'; diff --git a/app/main/__init__.py b/app/main/__init__.py index 7ce30ff8a..fbfd2562a 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -5,5 +5,5 @@ main = Blueprint('main', __name__) from app.main.views import ( index, sign_in, sign_out, register, two_factor, verify, sms, add_service, code_not_received, jobs, dashboard, templates, service_settings, forgot_password, - new_password, styleguide, user_profile, choose_service, api_keys + new_password, styleguide, user_profile, choose_service, api_keys, manage_users ) diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 604b2e27d..77f6610dc 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -1,7 +1,7 @@ -from flask import url_for +from flask import url_for, abort from app import notifications_api_client -from notifications_python_client.errors import HTTPError from app.utils import BrowsableItem +from notifications_python_client.errors import HTTPError def insert_new_service(service_name, user_id): @@ -29,7 +29,9 @@ def get_service_by_id(id_): def get_service_by_id_or_404(id_): try: - return get_service_by_id(id_) + return notifications_api_client.get_service(id_)['data'] + except KeyError: + abort(404) except HTTPError as e: if e.status_code == 404: abort(404) diff --git a/app/main/forms.py b/app/main/forms.py index ddab3fe01..20b007514 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -21,10 +21,10 @@ from app.utils import ( ) -def email_address(): +def email_address(label='Email address'): gov_uk_email \ = "(^[^@^\\s]+@[^@^\\.^\\s]+(\\.[^@^\\.^\\s]*)*.gov.uk)" - return EmailField('Email address', validators=[ + return EmailField(label, validators=[ Length(min=5, max=255), DataRequired(message='Email cannot be empty'), Email(message='Enter a valid email address'), @@ -96,6 +96,10 @@ class RegisterUserForm(Form): password = password() +class InviteUserForm(Form): + email_address = email_address('Their email address') + + class TwoFactorForm(Form): def __init__(self, validate_code_func, *args, **kwargs): ''' diff --git a/app/main/views/index.py b/app/main/views/index.py index 151ee4e80..1200cf65b 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -34,63 +34,3 @@ def send_email(service_id): @login_required def check_email(service_id): return render_template('views/check-email.html') - - -@main.route("/services//manage-users") -@login_required -def manage_users(service_id): - users = [ - { - 'name': 'Henry Hadlow', - 'permission_send_messages': True, - 'permission_manage_service': False, - 'permission_manage_api_keys': False - }, - - { - 'name': 'Pete Herlihy', - 'permission_send_messages': False, - 'permission_manage_service': False, - 'permission_manage_api_keys': False, - }, - { - 'name': 'Chris Hill-Scott', - 'permission_send_messages': True, - 'permission_manage_service': True, - 'permission_manage_api_keys': True - }, - { - 'name': 'Martyn Inglis', - 'permission_send_messages': True, - 'permission_manage_service': True, - 'permission_manage_api_keys': True - } - ] - invited_users = [ - { - 'email_localpart': 'caley.smolska', - 'permission_send_messages': True, - 'permission_manage_service': False, - 'permission_manage_api_keys': False - }, - - { - 'email_localpart': 'ash.stephens', - 'permission_send_messages': False, - 'permission_manage_service': False, - 'permission_manage_api_keys': False - }, - { - 'email_localpart': 'nicholas.staples', - 'permission_send_messages': True, - 'permission_manage_service': True, - 'permission_manage_api_keys': True - }, - { - 'email_localpart': 'adam.shimali', - 'permission_send_messages': True, - 'permission_manage_service': True, - 'permission_manage_api_keys': True - } - ] - return render_template('views/manage-users.html', service_id=service_id, users=users, invited_users=invited_users) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py new file mode 100644 index 000000000..54be424d2 --- /dev/null +++ b/app/main/views/manage_users.py @@ -0,0 +1,141 @@ +from flask import ( + request, + render_template, + redirect, + abort, + url_for, + flash +) + +from flask_login import login_required, current_user + +from app.main import main +from app.main.dao import users_dao +from app.main.forms import InviteUserForm +from app.main.dao.services_dao import get_service_by_id_or_404 +from app import user_api_client + + +fake_users = [ + { + 'name': 'Henry Hadlow', + 'email_localpart': 'henry.hadlow', + 'permission_send_messages': True, + 'permission_manage_service': False, + 'permission_manage_api_keys': False, + 'active': True + }, + + { + 'name': 'Pete Herlihy', + 'email_localpart': 'pete.herlihy', + 'permission_send_messages': False, + 'permission_manage_service': False, + 'permission_manage_api_keys': False, + 'active': True + }, + { + 'name': 'Chris Hill-Scott', + 'email_localpart': 'chris.hill-scott', + 'permission_send_messages': True, + 'permission_manage_service': True, + 'permission_manage_api_keys': True, + 'active': True + }, + { + 'name': 'Martyn Inglis', + 'email_localpart': 'martyn.inglis', + 'permission_send_messages': True, + 'permission_manage_service': True, + 'permission_manage_api_keys': True, + 'active': True + }, + { + 'email_localpart': 'caley.smolska', + 'permission_send_messages': True, + 'permission_manage_service': False, + 'permission_manage_api_keys': False, + 'active': False + }, + + { + 'email_localpart': 'ash.stephens', + 'permission_send_messages': False, + 'permission_manage_service': False, + 'permission_manage_api_keys': False, + 'active': False + } +] + + +@main.route("/services//users") +@login_required +def manage_users(service_id): + return render_template( + 'views/manage-users.html', + service_id=service_id, + users=[ + dict(id=user_id, **user) for (user_id, user) in enumerate(fake_users) if user['active'] + ], + invited_users=[ + dict(id=user_id, **user) for (user_id, user) in enumerate(fake_users) if not user['active'] + ] + ) + + +@main.route("/services//users/invite", methods=['GET', 'POST']) +@login_required +def invite_user(service_id): + + form = InviteUserForm() + + if form.validate_on_submit(): + flash('Invite sent to {}'.format(form.email_address.data), 'default_with_tick') + return redirect(url_for('.manage_users', service_id=service_id)) + + return render_template( + 'views/invite-user.html', + user={}, + service=get_service_by_id_or_404(service_id), + service_id=service_id, + form=form + ) + + +@main.route("/services//users/", methods=['GET', 'POST']) +@login_required +def edit_user(service_id, user_id): + + if request.method == 'POST': + return redirect(url_for('.manage_users', service_id=service_id)) + + return render_template( + 'views/invite-user.html', + user=fake_users[int(user_id)], + user_id=user_id, + service=get_service_by_id_or_404(service_id), + service_id=service_id + ) + + +@main.route("/services//users//delete", methods=['GET', 'POST']) +@login_required +def delete_user(service_id, user_id): + + if request.method == 'POST': + return redirect(url_for('.manage_users', service_id=service_id)) + + user = fake_users[int(user_id)] + + flash( + 'Are you sure you want to delete {}’s account?'.format(user.get('name') or user['email_localpart']), + 'delete' + ) + + return render_template( + 'views/invite-user.html', + user=user, + user_id=user_id, + service=get_service_by_id_or_404(service_id), + service_id=service_id + ) diff --git a/app/templates/components/table.html b/app/templates/components/table.html index ed9b35902..9ab70a29f 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -55,6 +55,18 @@ {%- endmacro %} +{% macro text_field(text) -%} + {% call field() %} + {{ text }} + {% endcall %} +{%- endmacro %} + +{% macro boolean_field(yes) -%} + {% call field(status='yes' if yes else 'no') %} + {{ "Yes" if yes else "No" }} + {% endcall %} +{%- endmacro %} + {% macro right_aligned_field_heading(text) %} {{ text }} {%- endmacro %} diff --git a/app/templates/components/yes-no.html b/app/templates/components/yes-no.html new file mode 100644 index 000000000..4432d6a9f --- /dev/null +++ b/app/templates/components/yes-no.html @@ -0,0 +1,17 @@ +{% macro yes_no(name, label, current_value=None) %} +
+ + {{ label }} + +
+ + +
+
+{% endmacro %} diff --git a/app/templates/flash_messages.html b/app/templates/flash_messages.html index ec2b1a5d0..bf109421e 100644 --- a/app/templates/flash_messages.html +++ b/app/templates/flash_messages.html @@ -5,7 +5,7 @@ {{ banner( message, 'default' if ((category == 'default') or (category == 'default_with_tick')) else 'dangerous', - delete_button="Yes, delete this template" if 'delete' == category else None, + delete_button="Yes, delete" if 'delete' == category else None, with_tick=True if category == 'default_with_tick' else False )}} {% endfor %} diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index c4e48051a..7e5563890 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -8,7 +8,7 @@
    diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index 7ae0c1b90..b7434514c 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -57,7 +57,7 @@ {% endif %} {% endcall %} -

    +

    Create a new API key

    diff --git a/app/templates/views/invite-user.html b/app/templates/views/invite-user.html new file mode 100644 index 000000000..2ca19bc2f --- /dev/null +++ b/app/templates/views/invite-user.html @@ -0,0 +1,48 @@ +{% extends "withnav_template.html" %} +{% from "components/yes-no.html" import yes_no %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +Manage users – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

    + {{ user.name or user.email_localpart or "Add a new team member" }} +

    + +
    +
    + + {% if user %} +

    + {{ user.email_localpart }}@digital.cabinet-office.gov.uk +

    + {% else %} + {{ textbox(form.email_address, hint='Email address must end in .gov.uk', width='1-1') }} + {% endif %} + +
    + + Permissions + + {{ yes_no('send_messages', 'Send messages', user.permission_send_messages) }} + {{ yes_no('manage_service', 'Manage service', user.permission_manage_service) }} + {{ yes_no('manage_api_keys', 'Manage API keys', user.permission_manage_api_keys) }} +
    + + {% if user %} + {{ page_footer( + 'Save', + delete_link=url_for('.delete_user', service_id=service_id, user_id=user_id), + delete_link_text='delete this account' + ) }} + {% else %} + {{ page_footer('Send invitation email') }} + {% endif %} + +
    +
    +{% endblock %} diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index ac881a790..4af05c7aa 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -1,65 +1,56 @@ {% extends "withnav_template.html" %} -{% from "components/table.html" import list_table, row, field %} +{% from "components/table.html" import list_table, row, field, boolean_field, hidden_field_heading %} {% from "components/page-footer.html" import page_footer %} +{% set table_options = { + 'field_headings': [ + 'Name', 'Send messages', 'Manage service', 'Manage API keys', hidden_field_heading('Link to change') + ], + 'field_headings_visible': True, + 'caption_visible': True +} %} + {% block page_title %} Manage users – GOV.UK Notify {% endblock %} {% block maincolumn_content %} -

    Manage users

    +

    + Manage team +

    -

    - Invite users -

    + {% call(item) list_table( + users, caption='Active', **table_options + ) %} + {% call field() %} + {{ item.name }} + {% endcall %} + {{ boolean_field(item.permission_send_messages) }} + {{ boolean_field(item.permission_manage_service) }} + {{ boolean_field(item.permission_manage_api_keys) }} + {% call field(align='right') %} + Change + {% endcall %} + {% endcall %} +

    + Add a new team member +

    -{% call(item) list_table( - users, - caption='Active users', - field_headings=['Name', 'Send messages', 'Manage Service', 'Manage API keys', 'Link to change'], - field_headings_visible=True, - caption_visible=True -) %} - {% call field() %} - {{ item.name }} - {% endcall %} - {% call field() %} - {{ "✔" if item.permission_send_messages else "❌" }} - {% endcall %} - {% call field() %} - {{ "✔" if item.permission_manage_service else "❌" }} - {% endcall %} - {% call field() %} - {{ "✔" if item.permission_manage_api_keys else "❌" }} - {% endcall %} - {% call field(align='right') %} - Change - {% endcall %} -{% endcall %} - -{% call(item) list_table( - invited_users, - caption='Invited users', - field_headings=['Name', 'Send messages', 'Manage Service', 'Manage API keys', 'Link to change'], - field_headings_visible=True, - caption_visible=True -) %} - {% call field() %} - {{ item.email_localpart }} - {% endcall %} - {% call field() %} - {{ "✔" if item.permission_send_messages else "❌" }} - {% endcall %} - {% call field() %} - {{ "✔" if item.permission_manage_service else "❌" }} - {% endcall %} - {% call field() %} - {{ "✔" if item.permission_manage_api_keys else "❌" }} - {% endcall %} - {% call field(align='right') %} - Change - {% endcall %} -{% endcall %} + {% if invited_users %} + {% call(item) list_table( + invited_users, caption='Invited', **table_options + ) %} + {% call field() %} + {{ item.email_localpart }} + {% endcall %} + {{ boolean_field(item.permission_send_messages) }} + {{ boolean_field(item.permission_manage_service) }} + {{ boolean_field(item.permission_manage_api_keys) }} + {% call field(align='right') %} + Change + {% endcall %} + {% endcall %} + {% endif %} {% endblock %} diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 7fbc03610..3938b00a8 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -82,6 +82,7 @@ gulp.task('watchForChanges', function() { gulp.watch(paths.src + 'javascripts/**/*', ['javascripts']); gulp.watch(paths.src + 'stylesheets/**/*', ['sass']); gulp.watch(paths.src + 'images/**/*', ['images']); + gulp.watch('gulpfile.babel.js', ['default']); }); gulp.task('lint:sass', () => gulp diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py new file mode 100644 index 000000000..000464ecb --- /dev/null +++ b/tests/app/main/views/test_manage_users.py @@ -0,0 +1,87 @@ +import json +from flask import url_for + + +def test_should_show_overview_page( + app_, + api_user_active, + mock_login, + mock_get_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.manage_users', service_id=55555)) + + assert 'Manage team' in response.get_data(as_text=True) + assert 'Henry Hadlow' in response.get_data(as_text=True) + assert 'caley.smolska' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_show_page_for_one_user( + app_, + api_user_active, + mock_login, + mock_get_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.edit_user', service_id=55555, user_id=0)) + + assert 'Henry Hadlow' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_redirect_after_saving_user( + app_, + api_user_active, + mock_login, + mock_get_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.post(url_for( + 'main.edit_user', service_id=55555, user_id=0 + )) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.manage_users', service_id=55555, _external=True + ) + + +def test_should_show_page_for_inviting_user( + app_, + api_user_active, + mock_login, + mock_get_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.get(url_for('main.invite_user', service_id=55555)) + + assert 'Add a new team member' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_invite_user( + app_, + api_user_active, + mock_login, + mock_get_service +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + response = client.post( + url_for('main.invite_user', service_id=55555), + data={'email_address': 'test@example.gov.uk'}, + follow_redirects=True + ) + + assert response.status_code == 200 + assert 'Invite sent to test@example.gov.uk' in response.get_data(as_text=True)