From d344bc7006fb6e826390bc51c645467373ffac5c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 27 Jun 2019 15:34:23 +0100 Subject: [PATCH 1/2] Fix a bug with inviting existing users to an organisation. The method to add the user to the organisation was missing the user id. This PR fixes that. --- app/main/views/invites.py | 2 +- app/models/user.py | 4 ++-- .../main/views/organisations/test_organisation_invites.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 8c3eb427e..f6a070ea0 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -105,7 +105,7 @@ def accept_org_invite(token): if existing_user: invited_org_user.accept_invite() if existing_user not in organisation_users: - invited_org_user.add_to_organisation() + invited_org_user.add_to_organisation(existing_user.id) return redirect(url_for('main.organisation_dashboard', org_id=invited_org_user.organisation)) else: return redirect(url_for('main.register_from_org_invite')) diff --git a/app/models/user.py b/app/models/user.py index e0e46e620..062d4e983 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -584,8 +584,8 @@ class InvitedOrgUser(JSONModel): def accept_invite(self): org_invite_api_client.accept_invite(self.organisation, self.id) - def add_to_organisation(self): - user_api_client.add_user_to_organisation(self.organisation, self.id) + def add_to_organisation(self, user_id): + user_api_client.add_user_to_organisation(self.organisation, user_id) class AnonymousUser(AnonymousUserMixin): diff --git a/tests/app/main/views/organisations/test_organisation_invites.py b/tests/app/main/views/organisations/test_organisation_invites.py index 7d623dbfa..dd88e1aea 100644 --- a/tests/app/main/views/organisations/test_organisation_invites.py +++ b/tests/app/main/views/organisations/test_organisation_invites.py @@ -1,6 +1,5 @@ from datetime import datetime, timedelta from unittest.mock import ANY -from uuid import UUID import pytest from bs4 import BeautifulSoup @@ -84,7 +83,7 @@ def test_invite_org_user_errors_when_same_email_as_inviter( assert normalize_spaces(page.select_one('.error-message').text) == 'You can’t send an invitation to yourself' -def test_accepted_invite_when_user_already_logged_in( +def test_accepted_invite_when_other_user_already_logged_in( client_request, mock_check_org_invite_token ): @@ -165,6 +164,7 @@ def test_existing_user_invite_already_is_member_of_organisation( def test_existing_user_invite_not_a_member_of_organisation( client, + api_user_active, mock_check_org_invite_token, mock_get_user_by_email, mock_get_users_for_organisation, @@ -186,7 +186,7 @@ def test_existing_user_invite_not_a_member_of_organisation( mock_get_users_for_organisation.assert_called_once_with(ORGANISATION_ID) mock_add_user_to_organisation.assert_called_once_with( ORGANISATION_ID, - str(UUID(bytes=b'sample_org_invit', version=4)) + api_user_active['id'], ) From 6026ce3f8d98ceb4f9ae5f2b02a117c3b2d39693 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 27 Jun 2019 15:48:29 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Refactor=20model=20to=20put=20`add=5Fto?= =?UTF-8?q?=E2=80=A6`=20methods=20on=20user?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An invited user can’t be added to an organisation or service, only a real user can. So the methods to do this should be on the user model, and take the details of the invite as arguments. --- app/main/views/invites.py | 8 ++++++-- app/models/user.py | 25 ++++++++++++++----------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/app/main/views/invites.py b/app/main/views/invites.py index f6a070ea0..c1e03d402 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -65,7 +65,11 @@ def accept_invite(token): invited_user.auth_type == 'email_auth' ): existing_user.update(auth_type=invited_user.auth_type) - invited_user.add_to_service(existing_user_id=existing_user.id) + existing_user.add_to_service( + service_id=invited_user.service, + permissions=invited_user.permissions, + folder_permissions=invited_user.folder_permissions, + ) return redirect(url_for('main.service_dashboard', service_id=service.id)) else: return redirect(url_for('main.register_from_invite')) @@ -105,7 +109,7 @@ def accept_org_invite(token): if existing_user: invited_org_user.accept_invite() if existing_user not in organisation_users: - invited_org_user.add_to_organisation(existing_user.id) + existing_user.add_to_organisation(organisation_id=invited_org_user.organisation) return redirect(url_for('main.organisation_dashboard', org_id=invited_org_user.organisation)) else: return redirect(url_for('main.register_from_org_invite')) diff --git a/app/models/user.py b/app/models/user.py index 062d4e983..8810719bc 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -389,6 +389,20 @@ class User(JSONModel, UserMixin): self.current_session_id = user_api_client.get_user(self.id).get('current_session_id') session['current_session_id'] = self.current_session_id + def add_to_service(self, service_id, permissions, folder_permissions): + user_api_client.add_user_to_service( + service_id, + self.id, + permissions, + folder_permissions, + ) + + def add_to_organisation(self, organisation_id): + user_api_client.add_user_to_organisation( + organisation_id, + self.id, + ) + class InvitedUser(JSONModel): @@ -430,14 +444,6 @@ class InvitedUser(JSONModel): def accept_invite(self): invite_api_client.accept_invite(self.service, self.id) - def add_to_service(self, existing_user_id): - user_api_client.add_user_to_service( - self.service, - existing_user_id, - self.permissions, - self.folder_permissions, - ) - @property def permissions(self): return self._permissions @@ -584,9 +590,6 @@ class InvitedOrgUser(JSONModel): def accept_invite(self): org_invite_api_client.accept_invite(self.organisation, self.id) - def add_to_organisation(self, user_id): - user_api_client.add_user_to_organisation(self.organisation, user_id) - class AnonymousUser(AnonymousUserMixin): # set the anonymous user so that if a new browser hits us we don't error http://stackoverflow.com/a/19275188