From 1ff9d671eb9dfa3f6581c66e43e30965c943913a Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Thu, 3 Mar 2016 15:26:18 +0000 Subject: [PATCH] [WIP] pass invite instead of permissions to make update of invite easier if all goes well --- app/main/views/add_service.py | 5 +++- app/main/views/invites.py | 29 ++++++++-------------- app/notify_client/models.py | 29 ++++++++++++++++++---- app/notify_client/user_api_client.py | 4 +-- tests/app/main/views/test_accept_invite.py | 11 ++++---- tests/conftest.py | 7 +++++- 6 files changed, 52 insertions(+), 33 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 06ed495ab..700d4eb74 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -10,6 +10,8 @@ from flask_login import login_required from app.main import main 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 @@ -19,10 +21,11 @@ def add_service(): invited_user = session.get('invited_user') if invited_user: + invitation = InvitedUser(**invited_user) # 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) + user_api_client.add_user_to_service(service_id, user.id, invitation) 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 8bd6a94da..cad3bf3b4 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -1,12 +1,9 @@ from flask import ( redirect, url_for, - abort, session ) -from notifications_python_client.errors import HTTPError - from app.main import main from app import ( invite_api_client, @@ -17,20 +14,14 @@ from app import ( @main.route("/invitation/") def accept_invite(token): - try: - invited_user = invite_api_client.accept_invite(token) - existing_user = user_api_client.get_user_by_email(invited_user.email_address) + invited_user = invite_api_client.accept_invite(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) - 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 + 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')) diff --git a/app/notify_client/models.py b/app/notify_client/models.py index df8d41306..c96668241 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -126,18 +126,37 @@ class InvitedUser(object): self.service = str(service) self.from_user = from_user self.email_address = email_address - self.permissions = permissions.split(',') + if isinstance(permissions, list): + self.permissions = permissions + else: + self.permissions = permissions.split(',') self.status = status self.created_at = created_at def has_permissions(self, permission): return permission in self.permissions - def serialize(self): - return {'id': self.id, + def __eq__(self, other): + return ((self.id, + self.service, + self.from_user, + self.email_address, + self.status) == (other.id, + other.service, + other.from_user, + other.email_address, + other.status)) + + def serialize(self, permissions_as_string=False): + data = {'id': self.id, 'service': self.service, 'from_user': self.from_user, 'email_address': self.email_address, - 'permissions': self.permissions, - 'status': self.status + 'status': self.status, + 'created_at': str(self.created_at) } + if permissions_as_string: + data['permissions'] = ','.join(self.permissions) + else: + data['permissions'] = self.permissions + return data diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index d825f079f..ce9d83bce 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): + def add_user_to_service(self, service_id, user_id, invited_user): endpoint = '/service/{}/users/{}'.format(service_id, user_id) - resp = self.post(endpoint, data={}) + resp = self.post(endpoint, data=invited_user.serialize(permissions_as_string=True)) 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 78cdc28ed..7ea04af93 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -7,6 +7,7 @@ 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_get_user_by_email, mock_add_user_to_service): @@ -21,7 +22,7 @@ def test_existing_user_accept_invite_calls_api_and_redirects_to_dashboard(app_, mock_accept_invite.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) + mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, sample_invited_user) assert response.status_code == 302 assert response.location == expected_redirect_location @@ -31,12 +32,12 @@ 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_get_user_by_email, mock_add_user_to_service): expected_service = service_one['id'] - expected_redirect_location = 'http://localhost/services/{}/dashboard'.format(expected_service) with app_.test_request_context(): with app_.test_client() as client: @@ -45,7 +46,7 @@ def test_existing_signed_out_user_accept_invite_redirects_to_sign_in(app_, mock_accept_invite.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) + mock_add_user_to_service.assert_called_with(expected_service, api_user_active.id, sample_invited_user) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -59,7 +60,6 @@ def test_new_user_accept_invite_calls_api_and_redirects_to_registration(app_, mock_dont_get_user_by_email, mock_add_user_to_service): - expected_service = service_one['id'] expected_redirect_location = 'http://localhost/register-from-invite' with app_.test_request_context(): @@ -122,6 +122,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_dont_get_user_by_email, mock_register_user, @@ -157,7 +158,7 @@ def test_new_invited_user_verifies_and_added_to_service(app_, # service and sent on to dash board 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) + mock_add_user_to_service.assert_called_with(data['service'], new_user_id, sample_invited_user) 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 2779f849d..399d10539 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -596,6 +596,11 @@ def sample_invite(mocker, service_one): return invite_json(id, from_user, service_id, email_address, permissions, created_at) +@pytest.fixture(scope='function') +def sample_invited_user(mocker, sample_invite): + return InvitedUser(**sample_invite) + + @pytest.fixture(scope='function') def mock_create_invite(mocker, sample_invite): @@ -632,6 +637,6 @@ def mock_accept_invite(mocker, sample_invite): @pytest.fixture(scope='function') def mock_add_user_to_service(mocker, service_one, api_user_active): - def _add_user(service_id, user_id): + def _add_user(service_id, user_id, invited_user): return api_user_active return mocker.patch('app.user_api_client.add_user_to_service', side_effect=_add_user)