diff --git a/app/models.py b/app/models.py index 320aa33de..a50a19868 100644 --- a/app/models.py +++ b/app/models.py @@ -376,6 +376,12 @@ class Organisation(db.Model): if service.active and not service.restricted ] + @property + def domain_list(self): + return [ + domain.domain for domain in self.domains + ] + def serialize(self): return { "id": str(self.id), @@ -391,13 +397,20 @@ class Organisation(db.Model): "agreement_signed_on_behalf_of_name": self.agreement_signed_on_behalf_of_name, "agreement_signed_on_behalf_of_email_address": self.agreement_signed_on_behalf_of_email_address, "agreement_signed_version": self.agreement_signed_version, - "domains": [ - domain.domain for domain in self.domains - ], + "domains": self.domain_list, "request_to_go_live_notes": self.request_to_go_live_notes, "count_of_live_services": len(self.live_services), } + def serialize_for_list(self): + return { + 'name': self.name, + 'id': str(self.id), + 'active': self.active, + 'count_of_live_services': len(self.live_services), + 'domains': self.domain_list, + } + class Service(db.Model, Versioned): __tablename__ = 'services' diff --git a/app/organisation/rest.py b/app/organisation/rest.py index fbfe09d22..6296e63dd 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -45,7 +45,7 @@ def handle_integrity_error(exc): @organisation_blueprint.route('', methods=['GET']) def get_organisations(): organisations = [ - org.serialize() for org in dao_get_organisations() + org.serialize_for_list() for org in dao_get_organisations() ] return jsonify(organisations) diff --git a/app/user/rest.py b/app/user/rest.py index d4e140fb9..bb3a9ed9d 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -507,35 +507,10 @@ def get_orgs_and_services(user): { 'name': org.name, 'id': org.id, - 'services': [ - { - 'id': service.id, - 'name': service.name, - 'restricted': service.restricted, - } - for service in org.services - if service.active and service in user.services - ], 'count_of_live_services': len(org.live_services), } for org in user.organisations if org.active ], - 'services_without_organisations': [ - { - 'id': service.id, - 'name': service.name, - 'restricted': service.restricted, - } for service in user.services - if ( - service.active and - # include services that either aren't in an organisation, or are in an organisation, - # but not one that the user can see. - ( - not service.organisation or - service.organisation not in user.organisations - ) - ) - ], 'services': [ { 'id': service.id, diff --git a/tests/app/db.py b/tests/app/db.py index b4213bf01..38fa3ed92 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -622,15 +622,18 @@ def create_domain(domain, organisation_id): return domain -def create_organisation(name='test_org_1', active=True, organisation_type=None): +def create_organisation(name='test_org_1', active=True, organisation_type=None, domains=None): data = { 'name': name, 'active': active, - 'organisation_type': organisation_type + 'organisation_type': organisation_type, } organisation = Organisation(**data) dao_create_organisation(organisation) + for domain in domains or []: + create_domain(domain, organisation.id) + return organisation diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index d99670d7a..9d9a42b4c 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -16,7 +16,7 @@ from tests.app.db import ( def test_get_all_organisations(admin_request, notify_db_session): create_organisation(name='inactive org', active=False) - create_organisation(name='active org') + create_organisation(name='active org', domains=['example.com']) response = admin_request.get( 'organisation.get_organisations', @@ -27,9 +27,11 @@ def test_get_all_organisations(admin_request, notify_db_session): assert response[0]['name'] == 'active org' assert response[0]['active'] is True assert response[0]['count_of_live_services'] == 0 + assert response[0]['domains'] == ['example.com'] assert response[1]['name'] == 'inactive org' assert response[1]['active'] is False assert response[1]['count_of_live_services'] == 0 + assert response[1]['domains'] == [] def test_get_organisation_by_id(admin_request, notify_db_session): @@ -78,10 +80,10 @@ def test_get_organisation_by_id(admin_request, notify_db_session): def test_get_organisation_by_id_returns_domains(admin_request, notify_db_session): - org = create_organisation() - - create_domain('foo.gov.uk', org.id) - create_domain('bar.gov.uk', org.id) + org = create_organisation(domains=[ + 'foo.gov.uk', + 'bar.gov.uk', + ]) response = admin_request.get( 'organisation.get_organisation_by_id', diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 8a51d8968..801c9b6e3 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -837,41 +837,20 @@ def test_get_orgs_and_services_nests_services(admin_request, sample_user): 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, @@ -917,30 +896,15 @@ def test_get_orgs_and_services_only_returns_active(admin_request, sample_user): 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, @@ -983,25 +947,15 @@ def test_get_orgs_and_services_only_shows_users_orgs_and_services(admin_request, 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'] == [