From 28ea75728cbcbb347a7e26bdf858050cf06f2c07 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Mar 2019 11:56:17 +0000 Subject: [PATCH 1/3] Return domains in get organisation response We need this so we can disply them in the admin app. --- app/models.py | 3 +++ tests/app/organisation/test_rest.py | 23 ++++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/models.py b/app/models.py index 57de4a046..afd817619 100644 --- a/app/models.py +++ b/app/models.py @@ -377,6 +377,9 @@ class Organisation(db.Model): "agreement_signed_at": self.agreement_signed_at, "agreement_signed_by_id": self.agreement_signed_by_id, "agreement_signed_version": self.agreement_signed_version, + "domains": [ + domain.domain for domain in self.domains + ], } diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 1bd0fecf7..d8cf2b704 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -4,7 +4,7 @@ import pytest from app.models import Organisation from app.dao.organisation_dao import dao_add_service_to_organisation, dao_add_user_to_organisation -from tests.app.db import create_organisation, create_service, create_user +from tests.app.db import create_domain, create_organisation, create_service, create_user def test_get_all_organisations(admin_request, notify_db_session): @@ -44,6 +44,7 @@ def test_get_organisation_by_id(admin_request, notify_db_session): 'agreement_signed_version', 'letter_branding_id', 'email_branding_id', + 'domains', } assert response['id'] == str(org.id) assert response['name'] == 'test_org_1' @@ -55,6 +56,26 @@ def test_get_organisation_by_id(admin_request, notify_db_session): assert response['agreement_signed_version'] is None assert response['letter_branding_id'] is None assert response['email_branding_id'] is None + assert response['domains'] == [] + + +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) + + response = admin_request.get( + 'organisation.get_organisation_by_id', + _expected_status=200, + organisation_id=org.id + ) + + assert set(response['domains']) == { + 'foo.gov.uk', + 'bar.gov.uk', + } def test_post_create_organisation(admin_request, notify_db_session): From 9783ab56b79dd3471fdfddf783bac2f9a7e94033 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Mar 2019 12:21:25 +0000 Subject: [PATCH 2/3] =?UTF-8?q?Don=E2=80=99t=20wipe=20domains=20when=20upd?= =?UTF-8?q?ating=20other=20attributes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The domains for an organisation should only be updated (or wiped) if a new list is explicitly passed in by the admin app. --- app/dao/organisation_dao.py | 2 +- tests/app/organisation/test_rest.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index 26b6dca1f..c4c0b03e5 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -53,7 +53,7 @@ def dao_create_organisation(organisation): @transactional def dao_update_organisation(organisation_id, **kwargs): - domains = kwargs.pop('domains', []) + domains = kwargs.pop('domains', None) organisation = Organisation.query.filter_by(id=organisation_id).update( kwargs diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index d8cf2b704..e8569a48b 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -201,6 +201,29 @@ def test_post_update_organisation_updates_domains( ] == domain_list +def test_update_other_organisation_attributes_doesnt_clear_domains( + admin_request, + notify_db_session, +): + org = create_organisation(name='test_org_2') + create_domain('example.gov.uk', org.id) + + admin_request.post( + 'organisation.update_organisation', + _data={ + 'agreement_signed': True, + }, + organisation_id=org.id, + _expected_status=204 + ) + + assert [ + domain.domain for domain in org.domains + ] == [ + 'example.gov.uk' + ] + + def test_post_update_organisation_raises_400_on_existing_org_name( admin_request, sample_organisation): org = create_organisation() From b3976c360a70435d49009b927eb109fc851f4bed Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Mar 2019 14:14:19 +0000 Subject: [PATCH 3/3] Add test for updating default branding --- tests/app/organisation/test_rest.py | 36 ++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index e8569a48b..9fd312d71 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -4,7 +4,14 @@ import pytest from app.models import Organisation from app.dao.organisation_dao import dao_add_service_to_organisation, dao_add_user_to_organisation -from tests.app.db import create_domain, create_organisation, create_service, create_user +from tests.app.db import ( + create_domain, + create_email_branding, + create_letter_branding, + create_organisation, + create_service, + create_user, +) def test_get_all_organisations(admin_request, notify_db_session): @@ -224,6 +231,33 @@ def test_update_other_organisation_attributes_doesnt_clear_domains( ] +def test_update_organisation_default_branding( + admin_request, + notify_db_session, +): + + org = create_organisation(name='Test Organisation') + + email_branding = create_email_branding() + letter_branding = create_letter_branding() + + assert org.email_branding is None + assert org.letter_branding is None + + admin_request.post( + 'organisation.update_organisation', + _data={ + 'email_branding_id': str(email_branding.id), + 'letter_branding_id': str(letter_branding.id), + }, + organisation_id=org.id, + _expected_status=204 + ) + + assert org.email_branding == email_branding + assert org.letter_branding == letter_branding + + def test_post_update_organisation_raises_400_on_existing_org_name( admin_request, sample_organisation): org = create_organisation()