diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 293c550bf..50113bbf6 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -1,6 +1,9 @@ from random import (SystemRandom) from datetime import (datetime, timedelta) + from sqlalchemy import func +from sqlalchemy.orm import joinedload + from app import db from app.models import (User, VerifyCode) @@ -113,3 +116,16 @@ def update_user_password(user, password): user.password_changed_at = datetime.utcnow() db.session.add(user) db.session.commit() + + +def get_user_and_accounts(user_id): + return User.query.filter( + User.id == user_id + ).options( + # eagerly load the user's services and organisations, and also the service's org and vice versa + # (so we can see if the user knows about it) + joinedload('services'), + joinedload('organisations'), + joinedload('organisations.services'), + joinedload('services.organisation'), + ).one() diff --git a/app/models.py b/app/models.py index a9e63109a..a1d06fa30 100644 --- a/app/models.py +++ b/app/models.py @@ -116,11 +116,11 @@ class User(db.Model): services = db.relationship( 'Service', secondary='user_to_service', - backref=db.backref('user_to_service', lazy='dynamic')) + backref='user_to_service') organisations = db.relationship( 'Organisation', secondary='user_to_organisation', - backref=db.backref('users', lazy='dynamic')) + backref='users') @property def password(self): diff --git a/app/user/rest.py b/app/user/rest.py index 207d19bfe..1b5fbf987 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -19,7 +19,8 @@ from app.dao.users_dao import ( create_secret_code, save_user_attribute, update_user_password, - count_user_verify_codes + count_user_verify_codes, + get_user_and_accounts ) from app.dao.permissions_dao import permission_dao from app.dao.services_dao import dao_fetch_service_by_id @@ -397,8 +398,7 @@ def update_password(user_id): @user_blueprint.route('//organisations-and-services', methods=['GET']) def get_organisations_and_services_for_user(user_id): - user = get_user_by_id(user_id=user_id) - + user = get_user_and_accounts(user_id) data = { 'organisations': [ { @@ -410,10 +410,10 @@ def get_organisations_and_services_for_user(user_id): 'name': service.name } for service in org.services - if service.active and user in service.users + if service.active and service in user.services ] } - for org in user.organisations + for org in user.organisations if org.active ], 'services_without_organisations': [ { @@ -426,7 +426,7 @@ def get_organisations_and_services_for_user(user_id): # but not one that the user can see. ( not service.organisation or - user not in service.organisation.users + service.organisation not in user.organisations ) ) ] diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 5b79cd61b..fc4d9632d 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -15,6 +15,7 @@ from app.models import ( ) from app.dao.permissions_dao import default_service_permissions from tests import create_authorization_header +from tests.app.db import create_service, create_organisation, create_user def test_get_user_list(admin_request, sample_service): @@ -605,3 +606,128 @@ def test_cannot_update_user_password_using_attributes_method(admin_request, samp _expected_status=400 ) assert resp['message']['_schema'] == ['Unknown field name password'] + + +def test_get_orgs_and_services_nests_services(admin_request, sample_user): + org1 = create_organisation(name='org1') + org2 = create_organisation(name='org2') + service1 = create_service(service_name='service1') + service2 = create_service(service_name='service2') + service3 = create_service(service_name='service3') + + org1.services = [service1, service2] + org2.services = [] + + sample_user.organisations = [org1, org2] + sample_user.services = [service1, service2, service3] + + resp = admin_request.get('user.get_organisations_and_services_for_user', user_id=sample_user.id) + + assert resp == { + 'organisations': [ + { + 'name': org1.name, + 'id': str(org1.id), + 'services': [ + { + 'name': service1.name, + 'id': str(service1.id) + }, + { + 'name': service2.name, + 'id': str(service2.id) + } + ] + }, + { + 'name': org2.name, + 'id': str(org2.id), + 'services': [] + } + ], + 'services_without_organisations': [ + { + 'name': service3.name, + 'id': str(service3.id) + } + ] + } + + +def test_get_orgs_and_services_only_returns_active(admin_request, sample_user): + org1 = create_organisation(name='org1', active=True) + org2 = create_organisation(name='org2', active=False) + + # in an active org + service1 = create_service(service_name='service1', active=True) + service2 = create_service(service_name='service2', active=False) + # active but in an inactive org + service3 = create_service(service_name='service3', active=True) + # not in an org + service4 = create_service(service_name='service4', active=True) + service5 = create_service(service_name='service5', active=False) + + org1.services = [service1, service2] + org2.services = [service3] + + sample_user.organisations = [org1, org2] + sample_user.services = [service1, service2, service3, service4, service5] + + resp = admin_request.get('user.get_organisations_and_services_for_user', user_id=sample_user.id) + + assert resp == { + 'organisations': [ + { + 'name': org1.name, + 'id': str(org1.id), + 'services': [ + { + 'name': service1.name, + 'id': str(service1.id) + } + ] + } + ], + 'services_without_organisations': [ + { + 'name': service4.name, + 'id': str(service4.id) + } + ] + } + + +def test_get_orgs_and_services_only_shows_users_orgs_and_services(admin_request, sample_user): + other_user = create_user(email='other@user.com') + + org1 = create_organisation(name='org1') + org2 = create_organisation(name='org2') + service1 = create_service(service_name='service1') + service2 = create_service(service_name='service2') + + org1.services = [service1] + + sample_user.organisations = [org2] + sample_user.services = [service1] + + other_user.organisations = [org1, org2] + other_user.services = [service1, service2] + + resp = admin_request.get('user.get_organisations_and_services_for_user', user_id=sample_user.id) + + assert resp == { + 'organisations': [ + { + 'name': org2.name, + 'id': str(org2.id), + 'services': [] + } + ], + # service1 belongs to org1, but the user doesn't know about org1 + 'services_without_organisations': [ + { + 'name': service1.name, + 'id': str(service1.id) + } + ] + }