Merge pull request #2540 from alphagov/remove-old-fields-from-organisations-and-services

Remove old fields from organisations and services

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'

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)

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,

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
 
 

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',

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'] == [