From a974e6e157e8bcd17f1209b66789a4910363168d Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 3 Mar 2016 17:53:16 +0000 Subject: [PATCH 01/11] [WIP] Add call to api to update invitation to accepted. When flow for invited user is complete, that is when user has been added to service, update invitation to accepted --- app/main/views/add_service.py | 9 ++++- app/main/views/invites.py | 34 ++++++++++------ app/notify_client/invite_api_client.py | 7 +++- app/notify_client/user_api_client.py | 4 +- tests/app/main/views/test_accept_invite.py | 46 +++++++++++++--------- tests/conftest.py | 15 +++++-- 6 files changed, 76 insertions(+), 39 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 700d4eb74..05febf569 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -12,7 +12,10 @@ from app.main.dao import services_dao, users_dao from app.main.forms import AddServiceForm from app.notify_client.models import InvitedUser -from app import user_api_client +from app import ( + invite_api_client, + user_api_client +) @main.route("/add-service", methods=['GET', 'POST']) @@ -25,7 +28,9 @@ def add_service(): # if invited user add to service and redirect to dashboard user = users_dao.get_user_by_id(session['user_id']) service_id = invited_user['service'] - user_api_client.add_user_to_service(service_id, user.id, invitation) + user_api_client.add_user_to_service(service_id, user.id, invitation.permissions) + invite_api_client.accept_invite(service_id, invitation.id) + session.pop('invited_user', None) return redirect(url_for('main.service_dashboard', service_id=service_id)) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index cad3bf3b4..4fce06803 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -1,9 +1,12 @@ from flask import ( redirect, url_for, - session + session, + abort ) +from notifications_python_client.errors import HTTPError + from app.main import main from app import ( invite_api_client, @@ -14,14 +17,23 @@ from app import ( @main.route("/invitation/") def accept_invite(token): - invited_user = invite_api_client.accept_invite(token) - existing_user = user_api_client.get_user_by_email(invited_user.email_address) + try: - if existing_user: - user_api_client.add_user_to_service(invited_user.service, - existing_user.id, - invited_user) - return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) - else: - session['invited_user'] = invited_user.serialize() - return redirect(url_for('main.register_from_invite')) + invited_user = invite_api_client.check_token(token) + existing_user = user_api_client.get_user_by_email(invited_user.email_address) + + if existing_user: + user_api_client.add_user_to_service(invited_user.service, + existing_user.id, + invited_user.permissions) + invite_api_client.accept_invite(invited_user.service, invited_user.id) + return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) + else: + session['invited_user'] = invited_user.serialize() + return redirect(url_for('main.register_from_invite')) + + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 6f2712848..03ce6e85c 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -31,7 +31,7 @@ class InviteApiClient(BaseAPIClient): invited_users = self._get_invited_users(invites) return invited_users - def accept_invite(self, token): + def check_token(self, token): resp = self.get(url='/invite/{}'.format(token)) return InvitedUser(**resp['data']) @@ -40,6 +40,11 @@ class InviteApiClient(BaseAPIClient): self.post(url='/service/{0}/invite/{1}'.format(service_id, invited_user_id), data=data) + def accept_invite(self, service_id, invited_user_id): + data = {'status': 'accepted'} + self.post(url='/service/{0}/invite/{1}'.format(service_id, invited_user_id), + data=data) + def _get_invited_users(self, invites): invited_users = [] for invite in invites: diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index ce9d83bce..74c1f9af9 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -94,9 +94,9 @@ class UserApiClient(BaseAPIClient): resp = self.get(endpoint) return [User(data) for data in resp['data']] - def add_user_to_service(self, service_id, user_id, invited_user): + def add_user_to_service(self, service_id, user_id, permissions): endpoint = '/service/{}/users/{}'.format(service_id, user_id) - resp = self.post(endpoint, data=invited_user.serialize(permissions_as_string=True)) + resp = self.post(endpoint, data={'permissions': permissions}) return User(resp['data'], max_failed_login_count=self.max_failed_login_count) def set_user_permissions(self, user_id, service_id, permissions): diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 7ea04af93..2c0d001be 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -7,22 +7,24 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, service_one, api_user_active, sample_invite, - sample_invited_user, - mock_accept_invite, + mock_check_invite_token, mock_get_user_by_email, - mock_add_user_to_service): + mock_add_user_to_service, + mock_accept_invite): expected_service = service_one['id'] expected_redirect_location = 'http://localhost/services/{}/dashboard'.format(expected_service) + expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys'] with app_.test_request_context(): with app_.test_client() as client: response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) - mock_accept_invite.assert_called_with('thisisnotarealtoken') + mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_get_user_by_email.assert_called_with('invited_user@test.gov.uk') - mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, sample_invited_user) + mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, expected_permissions) + mock_accept_invite.assert_called_with(expected_service, sample_invite['id']) assert response.status_code == 302 assert response.location == expected_redirect_location @@ -32,21 +34,22 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_, service_one, api_user_active, sample_invite, - sample_invited_user, - mock_accept_invite, + mock_check_invite_token, mock_get_user_by_email, - mock_add_user_to_service): + mock_add_user_to_service, + mock_accept_invite): expected_service = service_one['id'] - + expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys'] with app_.test_request_context(): with app_.test_client() as client: response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) - mock_accept_invite.assert_called_with('thisisnotarealtoken') + mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_get_user_by_email.assert_called_with('invited_user@test.gov.uk') - mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, sample_invited_user) + mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, expected_permissions) + mock_accept_invite.assert_called_with(expected_service, sample_invite['id']) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -56,9 +59,10 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_, def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, service_one, sample_invite, - mock_accept_invite, + mock_check_invite_token, mock_dont_get_user_by_email, - mock_add_user_to_service): + mock_add_user_to_service, + mock_accept_invite): expected_redirect_location = 'http://localhost/register-from-invite' @@ -67,7 +71,7 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) - mock_accept_invite.assert_called_with('thisisnotarealtoken') + mock_check_invite_token.assert_called_with('thisisnotarealtoken') mock_dont_get_user_by_email.assert_called_with('invited_user@test.gov.uk') assert response.status_code == 302 @@ -77,11 +81,12 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(app_, service_one, sample_invite, - mock_accept_invite, + mock_check_invite_token, mock_dont_get_user_by_email, mock_register_user, mock_send_verify_code, - mock_add_user_to_service): + mock_add_user_to_service, + mock_accept_invite): expected_service = service_one['id'] expected_email = sample_invite['email_address'] @@ -122,8 +127,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a def test_new_invited_user_verifies_and_added_to_service(app_, service_one, sample_invite, - sample_invited_user, - mock_accept_invite, + mock_check_invite_token, mock_dont_get_user_by_email, mock_register_user, mock_send_verify_code, @@ -131,6 +135,7 @@ def test_new_invited_user_verifies_and_added_to_service(app_, mock_get_user, mock_update_user, mock_add_user_to_service, + mock_accept_invite, mock_get_service, mock_get_service_templates, mock_get_jobs): @@ -156,9 +161,12 @@ def test_new_invited_user_verifies_and_added_to_service(app_, # when they post codes back to admin user should be added to # service and sent on to dash board + expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys'] with client.session_transaction() as session: new_user_id = session['user_id'] - mock_add_user_to_service.assert_called_with(data['service'], new_user_id, sample_invited_user) + mock_add_user_to_service.assert_called_with(data['service'], new_user_id, expected_permissions) + + mock_accept_invite.assert_called_with(data['service'], sample_invite['id']) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') element = page.find('h2', class_='navigation-service-name').find('a') diff --git a/tests/conftest.py b/tests/conftest.py index 399d10539..2f627e5a8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -629,14 +629,21 @@ def mock_get_invites_for_service(mocker, service_one, sample_invite): @pytest.fixture(scope='function') -def mock_accept_invite(mocker, sample_invite): - def _accept_token(token): +def mock_check_invite_token(mocker, sample_invite): + def _check_token(token): return InvitedUser(**sample_invite) - return mocker.patch('app.invite_api_client.accept_invite', side_effect=_accept_token) + return mocker.patch('app.invite_api_client.check_token', side_effect=_check_token) + + +@pytest.fixture(scope='function') +def mock_accept_invite(mocker, sample_invite): + def _accept(service_id, invite_id): + return InvitedUser(**sample_invite) + return mocker.patch('app.invite_api_client.accept_invite', side_effect=_accept) @pytest.fixture(scope='function') def mock_add_user_to_service(mocker, service_one, api_user_active): - def _add_user(service_id, user_id, invited_user): + def _add_user(service_id, user_id, permissions): return api_user_active return mocker.patch('app.user_api_client.add_user_to_service', side_effect=_add_user) From 8074c6ea7fd9793c0d43f0ebf3ddb0e57aaa9772 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 4 Mar 2016 14:42:52 +0000 Subject: [PATCH 02/11] Add cancelled-invite html. If a invited user accepts a cancelled invitation they are directed to a page telling them the invitation is cancelled. Without this they were able to register and were added to the service. --- app/main/views/invites.py | 6 +++++- app/templates/views/cancelled-invitation.html | 14 +++++++++++++ tests/__init__.py | 4 ++-- tests/app/main/views/test_accept_invite.py | 20 ++++++++++++++++++- tests/conftest.py | 4 ++-- 5 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 app/templates/views/cancelled-invitation.html diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 4fce06803..d6d496fe0 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -2,7 +2,8 @@ from flask import ( redirect, url_for, session, - abort + abort, + render_template ) from notifications_python_client.errors import HTTPError @@ -20,6 +21,9 @@ def accept_invite(token): try: invited_user = invite_api_client.check_token(token) + if invited_user.status == 'cancelled': + return render_template('views/cancelled-invitation.html') + existing_user = user_api_client.get_user_by_email(invited_user.email_address) if existing_user: diff --git a/app/templates/views/cancelled-invitation.html b/app/templates/views/cancelled-invitation.html new file mode 100644 index 000000000..d1b78cd3c --- /dev/null +++ b/app/templates/views/cancelled-invitation.html @@ -0,0 +1,14 @@ +{% extends "withoutnav_template.html" %} +{% block page_title %}Invitation has been cancelled{% endblock %} +{% block maincolumn_content %} +
+
+

+ The invitation you were sent has been cancelled. +

+

+ The person that sent you the invitation decided that it was sent in error and has cancelled the invitation. +

+
+
+{% endblock %} diff --git a/tests/__init__.py b/tests/__init__.py index f5028b65b..a712ce968 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -48,12 +48,12 @@ def api_key_json(id_, name, expiry_date=None): } -def invite_json(id, from_user, service_id, email_address, permissions, created_at): +def invite_json(id, from_user, service_id, email_address, permissions, created_at, status): return {'id': id, 'from_user': from_user, 'service': service_id, 'email_address': email_address, - 'status': 'pending', + 'status': status, 'permissions': permissions, 'created_at': created_at } diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 2c0d001be..8518309fc 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -2,6 +2,10 @@ from flask import url_for from bs4 import BeautifulSoup +import app +from tests.conftest import sample_invite as create_sample_invite +from tests.conftest import mock_check_invite_token as mock_check_token_invite + def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, service_one, @@ -58,7 +62,6 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_, def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, service_one, - sample_invite, mock_check_invite_token, mock_dont_get_user_by_email, mock_add_user_to_service, @@ -78,6 +81,21 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, assert response.location == expected_redirect_location +def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation(app_, + service_one, + mocker + ): + with app_.test_request_context(): + with app_.test_client() as client: + cancelled_invitation = create_sample_invite(mocker, service_one, status='cancelled') + mock_check_token_invite(mocker, cancelled_invitation) + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken')) + + app.invite_api_client.check_token.assert_called_with('thisisnotarealtoken') + assert response.status_code == 200 + assert 'Invitation has been cancelled' in response.get_data(as_text=True) + + def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(app_, service_one, sample_invite, diff --git a/tests/conftest.py b/tests/conftest.py index 2f627e5a8..cfd37f045 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -585,7 +585,7 @@ def mock_s3_upload(mocker): @pytest.fixture(scope='function') -def sample_invite(mocker, service_one): +def sample_invite(mocker, service_one, status='pending'): import datetime id = str(uuid.uuid4()) from_user = service_one['users'][0] @@ -593,7 +593,7 @@ def sample_invite(mocker, service_one): service_id = service_one['id'] permissions = 'send_messages,manage_service,manage_api_keys' created_at = str(datetime.datetime.now()) - return invite_json(id, from_user, service_id, email_address, permissions, created_at) + return invite_json(id, from_user, service_id, email_address, permissions, created_at, status) @pytest.fixture(scope='function') From 41b08b7ca1f9328d565090dfd427ba400633b8e2 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 4 Mar 2016 15:17:04 +0000 Subject: [PATCH 03/11] Added from_user name and service name for the cancelled invitation message. --- app/main/views/invites.py | 7 ++++++- app/templates/views/cancelled-invitation.html | 7 +++++-- tests/app/main/views/test_accept_invite.py | 7 +++++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index d6d496fe0..0df0306fb 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -9,6 +9,7 @@ from flask import ( from notifications_python_client.errors import HTTPError from app.main import main +from app.main.dao.services_dao import get_service_by_id_or_404 from app import ( invite_api_client, user_api_client @@ -22,7 +23,11 @@ def accept_invite(token): invited_user = invite_api_client.check_token(token) if invited_user.status == 'cancelled': - return render_template('views/cancelled-invitation.html') + from_user = user_api_client.get_user(invited_user.from_user) + service = get_service_by_id_or_404(invited_user.service) + return render_template('views/cancelled-invitation.html', + from_user=from_user.name, + service_name=service['name']) existing_user = user_api_client.get_user_by_email(invited_user.email_address) diff --git a/app/templates/views/cancelled-invitation.html b/app/templates/views/cancelled-invitation.html index d1b78cd3c..1e418505f 100644 --- a/app/templates/views/cancelled-invitation.html +++ b/app/templates/views/cancelled-invitation.html @@ -4,10 +4,13 @@

- The invitation you were sent has been cancelled. + The invitation you were sent has been cancelled

- The person that sent you the invitation decided that it was sent in error and has cancelled the invitation. + {{ from_user }} decided to cancel this invitation. +

+

+ If you need access to {{ service_name }}, you’ll have to ask them to invite you again.

diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 8518309fc..3a83f7578 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -83,7 +83,9 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation(app_, service_one, - mocker + mocker, + mock_get_user, + mock_get_service ): with app_.test_request_context(): with app_.test_client() as client: @@ -93,7 +95,8 @@ def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation app.invite_api_client.check_token.assert_called_with('thisisnotarealtoken') assert response.status_code == 200 - assert 'Invitation has been cancelled' in response.get_data(as_text=True) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'The invitation you were sent has been cancelled' def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(app_, From 7e27700ca8240ec3ef5e9c44e03f734ffe7d2853 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 4 Mar 2016 15:59:56 +0000 Subject: [PATCH 04/11] Show how complete a job is on dashboard --- app/templates/views/service_dashboard.html | 4 ++-- tests/__init__.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index bc70de076..b860e7f8b 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -56,7 +56,7 @@ jobs, caption="Recent text messages", empty_message='You haven’t sent any text messages yet', - field_headings=['Job', 'Created', right_aligned_field_heading('Status')] + field_headings=['Job', 'Created', right_aligned_field_heading('completion')] ) %} {% call field() %} {{ item.original_file_name }} @@ -65,7 +65,7 @@ {{ item.created_at|format_datetime }} {% endcall %} {% call field(align='right') %} - {{ item.status }} + {{ (item.notifications_sent / item.notification_count * 100)|round|int }}% {% endcall %} {% endcall %} {% if more_jobs_to_show %} diff --git a/tests/__init__.py b/tests/__init__.py index f5028b65b..9d1530349 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -110,6 +110,7 @@ def job_json(): 'file_name': '{}.csv'.format(job_id), 'created_at': created_at, 'notification_count': 1, + 'notifications_sent': 1, 'status': '' } return data From ee86f400b0eff7ff0a0033cf3d34f680d2c29239 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Mon, 7 Mar 2016 10:19:44 +0000 Subject: [PATCH 05/11] Filter out accepted invites from manage users page Added basic test of invite client. --- app/notify_client/invite_api_client.py | 5 ++- .../main/notify_client/test_invite_client.py | 38 +++++++++++++++++++ tests/app/main/views/test_manage_users.py | 33 ++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tests/app/main/notify_client/test_invite_client.py diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 03ce6e85c..0afd46d9b 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -48,6 +48,7 @@ class InviteApiClient(BaseAPIClient): def _get_invited_users(self, invites): invited_users = [] for invite in invites: - invited_user = InvitedUser(**invite) - invited_users.append(invited_user) + if invite['status'] != 'accepted': + invited_user = InvitedUser(**invite) + invited_users.append(invited_user) return invited_users diff --git a/tests/app/main/notify_client/test_invite_client.py b/tests/app/main/notify_client/test_invite_client.py new file mode 100644 index 000000000..8085f7095 --- /dev/null +++ b/tests/app/main/notify_client/test_invite_client.py @@ -0,0 +1,38 @@ +from app.notify_client.invite_api_client import InviteApiClient + + +def test_client_returns_invite(mocker, sample_invite): + + sample_invite['status'] = 'pending' + service_id = sample_invite['service'] + + expected_data = {'data': [sample_invite]} + + expected_url = '/service/{}/invite'.format(service_id) + + client = InviteApiClient() + mock_get = mocker.patch('app.notify_client.invite_api_client.InviteApiClient.get', return_value=expected_data) + + invites = client.get_invites_for_service(service_id) + + mock_get.assert_called_once_with(expected_url) + assert len(invites) == 1 + assert invites[0].status == 'pending' + + +def test_client_filters_out_accepted_invites(mocker, sample_invite): + + sample_invite['status'] = 'accepted' + service_id = sample_invite['service'] + + expected_data = {'data': [sample_invite]} + + expected_url = '/service/{}/invite'.format(service_id) + + client = InviteApiClient() + mock_get = mocker.patch('app.notify_client.invite_api_client.InviteApiClient.get', return_value=expected_data) + + invites = client.get_invites_for_service(service_id) + + mock_get.assert_called_once_with(expected_url) + assert len(invites) == 0 diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 123a225e3..b40179643 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -2,6 +2,8 @@ from flask import url_for from bs4 import BeautifulSoup +from app.notify_client.models import InvitedUser + def test_should_show_overview_page( app_, @@ -180,3 +182,34 @@ def test_cancel_invited_user_cancels_user_invitations(app_, assert response.status_code == 302 assert response.location == url_for('main.manage_users', service_id=service_id, _external=True) + + +def test_manage_users_shows_invited_user(app_, + mocker, + api_user_active, + mock_get_service, + mock_login, + mock_has_permissions, + mock_get_users_by_service, + sample_invite): + + import uuid + invited_user_id = uuid.uuid4() + sample_invite['id'] = invited_user_id + data = [InvitedUser(**sample_invite)] + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + + mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) + + response = client.get(url_for('main.manage_users', service_id=55555)) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Manage team' + invites_table = page.find_all('table')[1] + cols = invites_table.find_all('td') + assert cols[0].text.strip() == 'invited_user@test.gov.uk' + assert cols[4].text.strip() == 'Cancel invitation' From 569f61578e1318cfe5e3b86db78e22a20297a800 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Mon, 7 Mar 2016 11:55:18 +0000 Subject: [PATCH 06/11] Invited user email is shown on regiser from intive page but is not editable. --- app/main/forms.py | 2 +- app/templates/components/textbox.html | 21 +++++++++--- app/templates/views/register-from-invite.html | 2 +- tests/app/main/views/test_accept_invite.py | 33 +++++++++++++++++++ 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 4d0cb8f80..166323db7 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -104,7 +104,7 @@ class RegisterUserFromInviteForm(Form): mobile_number = mobile_number() password = password() service = HiddenField('service') - email_address = HiddenField('email_address') + email_address = email_address() class InviteUserForm(Form): diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html index b50fb9ee4..2fb5ea89e 100644 --- a/app/templates/components/textbox.html +++ b/app/templates/components/textbox.html @@ -6,7 +6,8 @@ help_link=None, help_link_text=None, width='2-3', - suffix=None + suffix=None, + disabled=False ) %}
- {{ field(**{ - 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), - 'data-module': 'highlight-tags' if highlight_tags else '' - }) }} + {% if disabled %} + {{ field(**{ + 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), + 'data-module': 'highlight-tags' if highlight_tags else '', + 'disabled': 'disabled' + }) }} + {% else %} + {{ field(**{ + 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), + 'data-module': 'highlight-tags' if highlight_tags else '' + }) }} + {% endif %} + + {% if suffix %} {{ suffix }} {% endif %} diff --git a/app/templates/views/register-from-invite.html b/app/templates/views/register-from-invite.html index 7e7e81aaa..b08e8d24f 100644 --- a/app/templates/views/register-from-invite.html +++ b/app/templates/views/register-from-invite.html @@ -12,12 +12,12 @@ Create an account – GOV.UK Notify

Create an account

+ {{ textbox(form.email_address, width='3-4', disabled=True ) }} {{ textbox(form.name, width='3-4') }} {{ textbox(form.mobile_number, width='3-4') }} {{ textbox(form.password, hint="Your password must have at least 10 characters", width='3-4') }} {{ page_footer("Continue") }} {{form.service}} - {{form.email_address}}
diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 3a83f7578..0cc735aeb 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -81,6 +81,39 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, assert response.location == expected_redirect_location +def test_new_user_accept_invite_calls_api_and_views_registration_page(app_, + service_one, + mock_check_invite_token, + mock_dont_get_user_by_email, + mock_add_user_to_service, + mock_accept_invite): + + with app_.test_request_context(): + with app_.test_client() as client: + + response = client.get(url_for('main.accept_invite', token='thisisnotarealtoken'), follow_redirects=True) + + mock_check_invite_token.assert_called_with('thisisnotarealtoken') + mock_dont_get_user_by_email.assert_called_with('invited_user@test.gov.uk') + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Create an account' + + form = page.find('form') + email = form.find('input', id='email_address') + name = form.find('input', id='name') + password = form.find('input', id='password') + service = form.find('input', type='hidden', id='service') + + assert email + assert email.attrs['disabled'] + assert name + assert password + assert service + assert service.attrs['value'] == service_one['id'] + + def test_cancelled_invited_user_accepts_invited_redirect_to_cancelled_invitation(app_, service_one, mocker, From 208a586948020bda52c4dad8ab7ba85615416dec Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Mon, 7 Mar 2016 13:59:54 +0000 Subject: [PATCH 07/11] Filter out accepted invites in template not client. --- app/main/views/manage_users.py | 6 +++- app/notify_client/invite_api_client.py | 5 ++- .../main/notify_client/test_invite_client.py | 18 ----------- tests/app/main/views/test_manage_users.py | 31 +++++++++++++++++++ 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index b8dbfc4c2..a1f44ad71 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -28,11 +28,15 @@ from app.utils import user_has_permissions def manage_users(service_id): users = user_api_client.get_users_for_service(service_id=service_id) invited_users = invite_api_client.get_invites_for_service(service_id=service_id) + filtered_invites = [] + for invite in invited_users: + if invite.status != 'accepted': + filtered_invites.append(invite) return render_template('views/manage-users.html', service_id=service_id, users=users, current_user=current_user, - invited_users=invited_users) + invited_users=filtered_invites) @main.route("/services//users/invite", methods=['GET', 'POST']) diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index 0afd46d9b..03ce6e85c 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -48,7 +48,6 @@ class InviteApiClient(BaseAPIClient): def _get_invited_users(self, invites): invited_users = [] for invite in invites: - if invite['status'] != 'accepted': - invited_user = InvitedUser(**invite) - invited_users.append(invited_user) + invited_user = InvitedUser(**invite) + invited_users.append(invited_user) return invited_users diff --git a/tests/app/main/notify_client/test_invite_client.py b/tests/app/main/notify_client/test_invite_client.py index 8085f7095..94e80863d 100644 --- a/tests/app/main/notify_client/test_invite_client.py +++ b/tests/app/main/notify_client/test_invite_client.py @@ -18,21 +18,3 @@ def test_client_returns_invite(mocker, sample_invite): mock_get.assert_called_once_with(expected_url) assert len(invites) == 1 assert invites[0].status == 'pending' - - -def test_client_filters_out_accepted_invites(mocker, sample_invite): - - sample_invite['status'] = 'accepted' - service_id = sample_invite['service'] - - expected_data = {'data': [sample_invite]} - - expected_url = '/service/{}/invite'.format(service_id) - - client = InviteApiClient() - mock_get = mocker.patch('app.notify_client.invite_api_client.InviteApiClient.get', return_value=expected_data) - - invites = client.get_invites_for_service(service_id) - - mock_get.assert_called_once_with(expected_url) - assert len(invites) == 0 diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index b40179643..e45392a06 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -213,3 +213,34 @@ def test_manage_users_shows_invited_user(app_, cols = invites_table.find_all('td') assert cols[0].text.strip() == 'invited_user@test.gov.uk' assert cols[4].text.strip() == 'Cancel invitation' + + +def test_manage_users_does_not_show_accepted_invite(app_, + mocker, + api_user_active, + mock_get_service, + mock_login, + mock_has_permissions, + mock_get_users_by_service, + sample_invite): + + import uuid + invited_user_id = uuid.uuid4() + sample_invite['id'] = invited_user_id + sample_invite['status'] = 'accepted' + data = [InvitedUser(**sample_invite)] + + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + + mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) + + response = client.get(url_for('main.manage_users', service_id=55555)) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'Manage team' + tables = page.find_all('table') + assert len(tables) == 1 + assert not page.find(text='invited_user@test.gov.uk') From 5b37145f613f1468bd54d768dfad8ef0b4631f57 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Mon, 7 Mar 2016 14:09:03 +0000 Subject: [PATCH 08/11] Wrap un editiable email address for invite in p tag to give it some space from rest of form. --- app/templates/components/textbox.html | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html index 2fb5ea89e..78241a630 100644 --- a/app/templates/components/textbox.html +++ b/app/templates/components/textbox.html @@ -24,11 +24,12 @@ {% endif %} {% if disabled %} - {{ field(**{ - 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), - 'data-module': 'highlight-tags' if highlight_tags else '', - 'disabled': 'disabled' - }) }} +

{{ field(**{ + 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), + 'data-module': 'highlight-tags' if highlight_tags else '', + 'disabled': 'disabled' + }) }} +

{% else %} {{ field(**{ 'class': 'form-control form-control-{} textbox-highlight-textbox'.format(width) if highlight_tags else 'form-control form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else ''), From 5429107f93c7fbf4510ef0b97d70f421350fe71c Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Mon, 7 Mar 2016 14:39:20 +0000 Subject: [PATCH 09/11] Removed remember me checkbox - remember me functionality always applied. --- app/main/forms.py | 1 - app/main/views/two_factor.py | 12 +++++++----- app/templates/views/two-factor.html | 1 - 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 4d0cb8f80..ad9036e1c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -127,7 +127,6 @@ class TwoFactorForm(Form): super(TwoFactorForm, self).__init__(*args, **kwargs) sms_code = sms_code() - remember_me = BooleanField("Remember me") def validate_sms_code(self, field): is_valid, reason = self.validate_code_func(field.data) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 325225ae2..6ed2a6089 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -1,6 +1,9 @@ - from flask import ( - render_template, redirect, jsonify, session, url_for) + render_template, + redirect, + session, + url_for +) from flask_login import login_user @@ -26,14 +29,13 @@ def two_factor(): try: user = users_dao.get_user_by_id(user_id) services = services_dao.get_services(user_id).get('data', []) - # Check if coming from new password page if 'password' in session['user_details']: user.set_password(session['user_details']['password']) users_dao.update_user(user) - login_user(user, remember=form.remember_me.data if form.remember_me.data else False) + login_user(user, remember=True) finally: del session['user_details'] - if (len(services) == 1): + if len(services) == 1: return redirect(url_for('main.service_dashboard', service_id=services[0]['id'])) else: return redirect(url_for('main.choose_service')) diff --git a/app/templates/views/two-factor.html b/app/templates/views/two-factor.html index 19126706e..f834b93e4 100644 --- a/app/templates/views/two-factor.html +++ b/app/templates/views/two-factor.html @@ -23,7 +23,6 @@ help_link=url_for('.verification_code_not_received'), help_link_text='I haven’t received a text message' ) }} - {{ checkbox(form.remember_me) }} {{ page_footer( "Continue" ) }} From 9bc5d08d52e86a0eadb77ad7cab42016426fde3a Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Tue, 8 Mar 2016 08:18:41 +0000 Subject: [PATCH 10/11] Flash message to confirm invitation accepted and user has been added to service. --- app/main/views/add_service.py | 2 -- app/main/views/dashboard.py | 16 +++++++++++++--- app/main/views/invites.py | 2 +- tests/app/main/views/test_accept_invite.py | 3 +++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 05febf569..937c28310 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -30,8 +30,6 @@ def add_service(): service_id = invited_user['service'] user_api_client.add_user_to_service(service_id, user.id, invitation.permissions) invite_api_client.accept_invite(service_id, invitation.id) - - session.pop('invited_user', None) return redirect(url_for('main.service_dashboard', service_id=service_id)) form = AddServiceForm(services_dao.find_all_service_names) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 9f7dc1044..e0c2a84ab 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -1,4 +1,10 @@ -from flask import (abort, render_template, session) +from flask import ( + abort, + render_template, + session, + flash +) + from flask_login import login_required from app.main import main from app.main.dao.services_dao import get_service_by_id @@ -6,8 +12,6 @@ from app.main.dao import templates_dao from notifications_python_client.errors import HTTPError from app import job_api_client -from app.utils import user_has_permissions - @main.route("/services//dashboard") @login_required @@ -24,6 +28,12 @@ def service_dashboard(service_id): service = get_service_by_id(service_id) session['service_name'] = service['data']['name'] session['service_id'] = service['data']['id'] + + if session.get('invited_user'): + session.pop('invited_user', None) + service_name = service['data']['name'] + message = 'You have sucessfully accepted your invitation and been added to {}'.format(service_name) + flash(message, 'default_with_tick') except HTTPError as e: if e.status_code == 404: abort(404) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 0df0306fb..662d66cec 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -30,6 +30,7 @@ def accept_invite(token): service_name=service['name']) existing_user = user_api_client.get_user_by_email(invited_user.email_address) + session['invited_user'] = invited_user.serialize() if existing_user: user_api_client.add_user_to_service(invited_user.service, @@ -38,7 +39,6 @@ def accept_invite(token): invite_api_client.accept_invite(invited_user.service, invited_user.id) return redirect(url_for('main.service_dashboard', service_id=invited_user.service)) else: - session['invited_user'] = invited_user.serialize() return redirect(url_for('main.register_from_invite')) except HTTPError as e: diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index 3a83f7578..4144d4d41 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -194,3 +194,6 @@ def test_new_invited_user_verifies_and_added_to_service(app_, assert element.text == 'Test Service' service_link = element.attrs['href'] assert service_link == '/services/{}/dashboard'.format(service_one['id']) + + flash_banner = page.find('div', class_='banner-default-with-tick').string.strip() + assert flash_banner == 'You have sucessfully accepted your invitation and been added to Test Service' From eb994ff18931b00fcc659864b53536204f0a1369 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Tue, 8 Mar 2016 10:40:07 +0000 Subject: [PATCH 11/11] Change put to post for service update. --- app/notify_client/api_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/notify_client/api_client.py b/app/notify_client/api_client.py index 6d2a2016a..719f015b5 100644 --- a/app/notify_client/api_client.py +++ b/app/notify_client/api_client.py @@ -68,7 +68,7 @@ class NotificationsAdminAPIClient(NotificationsAPIClient): "users": users } endpoint = "/service/{0}".format(service_id) - return self.put(endpoint, data) + return self.post(endpoint, data) def create_service_template(self, name, type_, content, service_id, subject=None): """