From b6fdb269e4bcb2c589f2e6bae1fd0118df96acf2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 Jun 2019 10:34:15 +0100 Subject: [PATCH] Return all required org and services info for user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin app now needs to know a few extra things about orgs and services in order to list them. At the moment it does this by making multiple API calls. This commit adds extra fields to the existing response. Once the admin app is using this fields we’ll be able to remove: - `reponse['services_without_organisations']` - `reponse['organisations']['services']` --- app/user/rest.py | 15 ++- tests/app/user/test_rest.py | 199 ++++++++++++++++++++++++------------ 2 files changed, 145 insertions(+), 69 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index 456008c30..872735de2 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -515,7 +515,11 @@ def get_orgs_and_services(user): } for service in org.services if service.active and service in user.services - ] + ], + 'count_of_live_services': len([ + service for service in org.services + if service.active and not service.restricted + ]), } for org in user.organisations if org.active ], @@ -534,5 +538,14 @@ def get_orgs_and_services(user): service.organisation not in user.organisations ) ) + ], + 'services': [ + { + 'id': service.id, + 'name': service.name, + 'restricted': service.restricted, + 'organisation': service.organisation.id if service.organisation else None, + } + for service in user.services if service.active ] } diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 35e3809ce..8a51d8968 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -835,38 +835,63 @@ def test_get_orgs_and_services_nests_services(admin_request, sample_user): 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), - 'restricted': False, - }, - { - 'name': service2.name, - 'id': str(service2.id), - 'restricted': False, - } - ] - }, - { - 'name': org2.name, - 'id': str(org2.id), - 'services': [] - } - ], - 'services_without_organisations': [ - { - 'name': service3.name, - 'id': str(service3.id), - 'restricted': False, - } - ] + assert set(resp.keys()) == { + 'organisations', + 'services_without_organisations', + 'services', } + assert resp['organisations'] == [ + { + 'name': org1.name, + 'id': str(org1.id), + 'services': [ + { + 'name': service1.name, + 'id': str(service1.id), + 'restricted': False, + }, + { + 'name': service2.name, + 'id': str(service2.id), + 'restricted': False, + } + ], + 'count_of_live_services': 2, + }, + { + 'name': org2.name, + 'id': str(org2.id), + 'services': [], + 'count_of_live_services': 0, + }, + ] + assert resp['services_without_organisations'] == [ + { + 'name': service3.name, + 'id': str(service3.id), + 'restricted': False, + } + ] + assert resp['services'] == [ + { + 'name': service1.name, + 'id': str(service1.id), + 'restricted': False, + 'organisation': str(org1.id), + }, + { + 'name': service2.name, + 'id': str(service2.id), + 'restricted': False, + 'organisation': str(org1.id), + }, + { + 'name': service3.name, + 'id': str(service3.id), + 'restricted': False, + 'organisation': None, + }, + ] def test_get_orgs_and_services_only_returns_active(admin_request, sample_user): @@ -890,28 +915,52 @@ def test_get_orgs_and_services_only_returns_active(admin_request, sample_user): 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), - 'restricted': False, - } - ] - } - ], - 'services_without_organisations': [ - { - 'name': service4.name, - 'id': str(service4.id), - 'restricted': False, - } - ] + assert set(resp.keys()) == { + 'organisations', + 'services_without_organisations', + 'services', } + assert resp['organisations'] == [ + { + 'name': org1.name, + 'id': str(org1.id), + 'services': [ + { + 'name': service1.name, + 'id': str(service1.id), + 'restricted': False, + } + ], + 'count_of_live_services': 1, + } + ] + assert resp['services_without_organisations'] == [ + { + 'name': service4.name, + 'id': str(service4.id), + 'restricted': False, + } + ] + assert resp['services'] == [ + { + 'name': service1.name, + 'id': str(service1.id), + 'restricted': False, + 'organisation': str(org1.id) + }, + { + 'name': service3.name, + 'id': str(service3.id), + 'restricted': False, + 'organisation': str(org2.id) + }, + { + 'name': service4.name, + 'id': str(service4.id), + 'restricted': False, + 'organisation': None, + }, + ] def test_get_orgs_and_services_only_shows_users_orgs_and_services(admin_request, sample_user): @@ -932,23 +981,37 @@ def test_get_orgs_and_services_only_shows_users_orgs_and_services(admin_request, 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), - 'restricted': False, - } - ] + assert set(resp.keys()) == { + 'organisations', + 'services_without_organisations', + 'services', } + assert resp['organisations'] == [ + { + 'name': org2.name, + 'id': str(org2.id), + 'services': [], + 'count_of_live_services': 0, + } + ] + # service1 belongs to org1, but the user doesn't know about org1 + assert resp['services_without_organisations'] == [ + { + 'name': service1.name, + 'id': str(service1.id), + 'restricted': False, + } + ] + # 'services' always returns the org_id no matter whether the user + # belongs to that org or not + assert resp['services'] == [ + { + 'name': service1.name, + 'id': str(service1.id), + 'restricted': False, + 'organisation': str(org1.id), + } + ] def test_find_users_by_email_finds_user_by_partial_email(notify_db, client):