darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-05-06 00:48:46 -04:00
Code Issues Packages Projects Releases Wiki Activity

Change display of cancelled users, fix edit link

Browse Source 
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.
This commit is contained in:
Chris Hill-Scott
2018-01-26 17:14:00 +00:00
parent 6fcb2dfde7
commit 2221a8ca48
3 changed files with 52 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/notify_client/models.py
View File

@@ -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):

4
app/templates/views/manage-users.html
View File

@@ -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 @@
<li class="tick-cross-list-edit-link">
{% if user.status == 'pending' %}
<a href="{{ url_for('.cancel_invited_user', service_id=current_service.id, invited_user_id=user.id)}}">Cancel invitation</a>
{% elif user.status == 'active' and current_user.id != user.id %}
{% elif user.state == 'active' and current_user.id != user.id %}
<a href="{{ url_for('.edit_user_permissions', service_id=current_service.id, user_id=user.id)}}">Edit permissions</a>
{% endif %}
</li>

60
tests/app/main/views/test_manage_users.py
View File

@@ -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(