From 2221a8ca485810bfca32b114f2b60f36d8c4c754 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 26 Jan 2018 17:14:00 +0000 Subject: [PATCH] Change display of cancelled users, fix edit link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s confusing showing green ticks for cancelled invites. This commit changes the appearance so that only pending or active users (ie those that could actually do some damage) get green ticks. Also fixes missing edit links caused by instances of `User` having `.state` but instances of `InvitedUser` having `.status`. Right now these are two separate lists. Which makes it harder to add improvements that will make large numbers of users easier to manage. --- app/notify_client/models.py | 2 + app/templates/views/manage-users.html | 4 +- tests/app/main/views/test_manage_users.py | 60 ++++++++++++++++++----- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 3e6a7ae60..394402154 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -184,6 +184,8 @@ class InvitedUser(object): self.auth_type = auth_type def has_permissions(self, *permissions): + if self.status == 'cancelled': + return False return set(self.permissions) > set(permissions) def __eq__(self, other): diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 5a86710be..414157432 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -54,7 +54,7 @@ {{ user.email_address }} (invited) {%- elif user.status == 'cancelled' -%} {{ user.email_address }} (cancelled invite) - {%- elif user.email_address == current_user.email_address -%} + {%- elif user.id == current_user.id -%} (you) {% else %} {{ user.email_address }} @@ -93,7 +93,7 @@
  • {% if user.status == 'pending' %} Cancel invitation - {% elif user.status == 'active' and current_user.id != user.id %} + {% elif user.state == 'active' and current_user.id != user.id %} Edit permissions {% endif %}
    • diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 96a58938a..8572e78aa 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -1,3 +1,4 @@ +import copy import pytest from flask import url_for from bs4 import BeautifulSoup @@ -15,13 +16,18 @@ from tests.conftest import ( ) -@pytest.mark.parametrize('user, expected_text', [ +@pytest.mark.parametrize('user, expected_self_text, expected_coworker_text', [ ( active_user_with_permissions, ( 'Test User (you) ' 'Can Send messages Can Add and edit templates Can Manage service Can Access API keys' ), + ( + 'ZZZZZZZZ zzzzzzz@example.gov.uk ' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can’t Access API keys ' + 'Edit permissions' + ) ), ( active_user_view_permissions, @@ -29,6 +35,10 @@ from tests.conftest import ( 'Test User With Permissions (you) ' 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can’t Access API keys' ), + ( + 'ZZZZZZZZ zzzzzzz@example.gov.uk ' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can’t Access API keys' + ) ), ( active_user_manage_template_permission, @@ -36,6 +46,10 @@ from tests.conftest import ( 'Test User With Permissions (you) ' 'Can’t Send messages Can Add and edit templates Can’t Manage service Can’t Access API keys' ), + ( + 'ZZZZZZZZ zzzzzzz@example.gov.uk ' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can’t Access API keys' + ) ), ]) def test_should_show_overview_page( @@ -44,15 +58,28 @@ def test_should_show_overview_page( mock_get_invites_for_service, fake_uuid, user, - expected_text, + expected_self_text, + expected_coworker_text, + active_user_view_permissions, ): - mocker.patch('app.user_api_client.get_users_for_service', return_value=[user(fake_uuid)]) + current_user = user(fake_uuid) + other_user = copy.deepcopy(active_user_view_permissions) + other_user.email_address = 'zzzzzzz@example.gov.uk' + other_user.name = 'ZZZZZZZZ' + other_user.id = 'zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz' + + 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_one('.user-list-item').text) == ( - expected_text - ) + assert normalize_spaces(page.select('.user-list-item')[0].text) == expected_self_text + # [1:5] are invited users + assert normalize_spaces(page.select('.user-list-item')[6].text) == expected_coworker_text app.user_api_client.get_users_for_service.assert_called_once_with(service_id=SERVICE_ONE_ID) @@ -447,25 +474,34 @@ def test_cancel_invited_user_cancels_user_invitations( assert response.location == url_for('main.manage_users', service_id=service['id'], _external=True) +@pytest.mark.parametrize('invite_status, expected_text', [ + ('pending', ( + 'invited_user@test.gov.uk (invited) ' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can Access API keys ' + 'Cancel invitation' + )), + ('cancelled', ( + 'invited_user@test.gov.uk (cancelled invite) ' + 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can’t Access API keys' + )), +]) def test_manage_users_shows_invited_user( client_request, mocker, active_user_with_permissions, sample_invite, + invite_status, + expected_text, ): + sample_invite['status'] = invite_status data = [InvitedUser(**sample_invite)] 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]) page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID) - assert page.h1.string.strip() == 'Team members' - assert normalize_spaces(page.select('.user-list-item')[0].text) == ( - 'invited_user@test.gov.uk (invited) ' - 'Can’t Send messages Can’t Add and edit templates Can’t Manage service Can Access API keys ' - 'Cancel invitation' - ) + assert normalize_spaces(page.select('.user-list-item')[0].text) == expected_text def test_manage_users_does_not_show_accepted_invite(