diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index 94cdeb237..26b6dca1f 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -1,7 +1,10 @@ +from sqlalchemy.sql.expression import func + from app import db from app.dao.dao_utils import transactional from app.models import ( Organisation, + Domain, InvitedOrganisationUser, User ) @@ -23,6 +26,21 @@ def dao_get_organisation_by_id(organisation_id): return Organisation.query.filter_by(id=organisation_id).one() +def dao_get_organisation_by_email_address(email_address): + + email_address = email_address.lower() + + for domain in Domain.query.order_by(func.char_length(Domain.domain).desc()).all(): + + if ( + email_address.endswith("@{}".format(domain.domain)) or + email_address.endswith(".{}".format(domain.domain)) + ): + return Organisation.query.filter_by(id=domain.organisation_id).one() + + return None + + def dao_get_organisation_by_service_id(service_id): return Organisation.query.join(Organisation.services).filter_by(id=service_id).first() @@ -34,10 +52,26 @@ def dao_create_organisation(organisation): @transactional def dao_update_organisation(organisation_id, **kwargs): - return Organisation.query.filter_by(id=organisation_id).update( + + domains = kwargs.pop('domains', []) + + organisation = Organisation.query.filter_by(id=organisation_id).update( kwargs ) + if isinstance(domains, list): + + Domain.query.filter_by(organisation_id=organisation_id).delete() + + db.session.bulk_save_objects([ + Domain(domain=domain.lower(), organisation_id=organisation_id) + for domain in domains + ]) + + db.session.commit() + + return organisation + @transactional def dao_add_service_to_organisation(service, organisation_id): diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index eb766c06b..70d7a8a83 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -11,6 +11,7 @@ from app.dao.dao_utils import ( transactional, version_class ) +from app.dao.organisation_dao import dao_get_organisation_by_email_address from app.dao.service_sms_sender_dao import insert_service_sms_sender from app.dao.service_user_dao import dao_get_service_user from app.models import ( @@ -151,14 +152,25 @@ def dao_fetch_service_by_id_and_user(service_id, user_id): @transactional @version_class(Service) -def dao_create_service(service, user, service_id=None, service_permissions=None, letter_branding=None): +def dao_create_service( + service, + user, + service_id=None, + service_permissions=None, + letter_branding=None, +): # the default property does not appear to work when there is a difference between the sqlalchemy schema and the # db schema (ie: during a migration), so we have to set sms_sender manually here. After the GOVUK sms_sender # migration is completed, this code should be able to be removed. + if not user: + raise ValueError("Can't create a service without a user") + if service_permissions is None: service_permissions = DEFAULT_SERVICE_PERMISSIONS + organisation = dao_get_organisation_by_email_address(user.email_address) + from app.dao.permissions_dao import permission_dao service.users.append(user) permission_dao.add_default_service_permissions_for_user(user, service) @@ -173,8 +185,20 @@ def dao_create_service(service, user, service_id=None, service_permissions=None, # do we just add the default - or will we get a value from FE? insert_service_sms_sender(service, current_app.config['FROM_NUMBER']) + if letter_branding: service.letter_branding = letter_branding + + if organisation: + + service.organisation = organisation + + if organisation.email_branding_id: + service.email_branding = organisation.email_branding_id + + if organisation.letter_branding_id and not service.letter_branding: + service.letter_branding = organisation.letter_branding_id + db.session.add(service) diff --git a/app/models.py b/app/models.py index b9c7799bd..a897bd761 100644 --- a/app/models.py +++ b/app/models.py @@ -316,6 +316,12 @@ organisation_to_service = db.Table( ) +class Domain(db.Model): + __tablename__ = "domain" + domain = db.Column(db.String(255), primary_key=True) + organisation_id = db.Column('organisation_id', UUID(as_uuid=True), db.ForeignKey('organisation.id'), nullable=False) + + class Organisation(db.Model): __tablename__ = "organisation" id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4, unique=False) @@ -329,15 +335,50 @@ class Organisation(db.Model): secondary='organisation_to_service', uselist=True) + agreement_signed = db.Column(db.Boolean, nullable=True) + agreement_signed_at = db.Column(db.DateTime, nullable=True) + agreement_signed_by_id = db.Column( + UUID(as_uuid=True), + db.ForeignKey('users.id'), + nullable=True, + ) + agreement_signed_version = db.Column(db.Float, nullable=True) + crown = db.Column(db.Boolean, nullable=True) + organisation_type = db.Column(db.String(255), nullable=True) + + domains = db.relationship( + 'Domain', + ) + + email_branding = db.relationship('EmailBranding') + email_branding_id = db.Column( + UUID(as_uuid=True), + db.ForeignKey('email_branding.id'), + nullable=True, + ) + + letter_branding = db.relationship('LetterBranding') + letter_branding_id = db.Column( + UUID(as_uuid=True), + db.ForeignKey('letter_branding.id'), + nullable=True, + ) + def serialize(self): - serialized = { + return { "id": str(self.id), "name": self.name, "active": self.active, + "crown": self.crown, + "organisation_type": self.organisation_type, + "letter_branding_id": self.letter_branding_id, + "email_branding_id": self.email_branding_id, + "agreement_signed": self.agreement_signed, + "agreement_signed_at": self.agreement_signed_at, + "agreement_signed_by_id": self.agreement_signed_by_id, + "agreement_signed_version": self.agreement_signed_version, } - return serialized - class Service(db.Model, Versioned): __tablename__ = 'services' diff --git a/migrations/versions/0278_add_more_stuff_to_orgs.py b/migrations/versions/0278_add_more_stuff_to_orgs.py new file mode 100644 index 000000000..f3bd91469 --- /dev/null +++ b/migrations/versions/0278_add_more_stuff_to_orgs.py @@ -0,0 +1,57 @@ +""" + +Revision ID: 0278_add_more_stuff_to_orgs +Revises: 0277_consent_to_research_null +Create Date: 2019-02-26 10:15:22.430340 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0278_add_more_stuff_to_orgs' +down_revision = '0277_consent_to_research_null' + + +def upgrade(): + op.create_table( + 'domain', + sa.Column('domain', sa.String(length=255), nullable=False), + sa.Column('organisation_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.ForeignKeyConstraint(['organisation_id'], ['organisation.id'], ), + sa.PrimaryKeyConstraint('domain') + ) + op.create_index(op.f('ix_domain_domain'), 'domain', ['domain'], unique=True) + + op.add_column('organisation', sa.Column('email_branding_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_foreign_key('fk_organisation_email_branding_id', 'organisation', 'email_branding', ['email_branding_id'], ['id']) + + op.add_column('organisation', sa.Column('letter_branding_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.create_foreign_key('fk_organisation_letter_branding_id', 'organisation', 'letter_branding', ['letter_branding_id'], ['id']) + + op.add_column('organisation', sa.Column('agreement_signed', sa.Boolean(), nullable=True)) + op.add_column('organisation', sa.Column('agreement_signed_at', sa.DateTime(), nullable=True)) + op.add_column('organisation', sa.Column('agreement_signed_by_id', postgresql.UUID(as_uuid=True), nullable=True)) + op.add_column('organisation', sa.Column('agreement_signed_version', sa.Float(), nullable=True)) + op.add_column('organisation', sa.Column('crown', sa.Boolean(), nullable=True)) + op.add_column('organisation', sa.Column('organisation_type', sa.String(length=255), nullable=True)) + op.create_foreign_key('fk_organisation_agreement_user_id', 'organisation', 'users', ['agreement_signed_by_id'], ['id']) + + +def downgrade(): + op.drop_constraint('fk_organisation_agreement_user_id', 'organisation', type_='foreignkey') + op.drop_column('organisation', 'organisation_type') + op.drop_column('organisation', 'crown') + op.drop_column('organisation', 'agreement_signed_version') + op.drop_column('organisation', 'agreement_signed_by_id') + op.drop_column('organisation', 'agreement_signed_at') + op.drop_column('organisation', 'agreement_signed') + + op.drop_column('organisation', 'email_branding_id') + op.drop_constraint('fk_organisation_email_branding_id', 'organisation', type_='foreignkey') + + op.drop_column('organisation', 'letter_branding_id') + op.drop_constraint('fk_organisation_letter_branding_id', 'organisation', type_='foreignkey') + + op.drop_index(op.f('ix_domain_domain'), table_name='domain') + op.drop_table('domain') diff --git a/tests/app/dao/test_organisation_dao.py b/tests/app/dao/test_organisation_dao.py index 62c81041a..667fc1438 100644 --- a/tests/app/dao/test_organisation_dao.py +++ b/tests/app/dao/test_organisation_dao.py @@ -1,3 +1,4 @@ +import datetime import uuid import pytest @@ -5,6 +6,7 @@ from sqlalchemy.exc import IntegrityError, SQLAlchemyError from app.dao.organisation_dao import ( dao_get_organisations, + dao_get_organisation_by_email_address, dao_get_organisation_by_id, dao_get_organisation_by_service_id, dao_get_organisation_services, @@ -16,7 +18,14 @@ from app.dao.organisation_dao import ( ) from app.models import Organisation -from tests.app.db import 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_organisations_gets_all_organisations_alphabetically_with_active_organisations_first( @@ -47,19 +56,67 @@ def test_get_organisation_by_id_gets_correct_organisation(notify_db, notify_db_s assert organisation_from_db == organisation -def test_update_organisation(notify_db, notify_db_session): - updated_name = 'new name' +def test_update_organisation( + notify_db, + notify_db_session, +): + create_organisation() + + organisation = Organisation.query.one() + user = create_user() + email_branding = create_email_branding() + letter_branding = create_letter_branding() + + data = { + 'name': 'new name', + "crown": True, + "organisation_type": 'local', + "agreement_signed": True, + "agreement_signed_at": datetime.datetime.utcnow(), + "agreement_signed_by_id": user.id, + "agreement_signed_version": 999.99, + "letter_branding_id": letter_branding.id, + "email_branding_id": email_branding.id, + } + + for attribute, value in data.items(): + assert getattr(organisation, attribute) != value + + dao_update_organisation(organisation.id, **data) + + organisation = Organisation.query.one() + + for attribute, value in data.items(): + assert getattr(organisation, attribute) == value + + +@pytest.mark.parametrize('domain_list, expected_domains', ( + (['abc', 'def'], {'abc', 'def'}), + (['ABC', 'DEF'], {'abc', 'def'}), + ([], set()), + (None, {'123', '456'}), + pytest.param( + ['abc', 'ABC'], {'abc'}, + marks=pytest.mark.xfail(raises=IntegrityError) + ), +)) +def test_update_organisation_domains_lowercases( + notify_db, + notify_db_session, + domain_list, + expected_domains, +): create_organisation() organisation = Organisation.query.one() - assert organisation.name != updated_name + # Seed some domains + dao_update_organisation(organisation.id, domains=['123', '456']) - dao_update_organisation(organisation.id, **{'name': updated_name}) + # This should overwrite the seeded domains + dao_update_organisation(organisation.id, domains=domain_list) - organisation = Organisation.query.one() - - assert organisation.name == updated_name + assert {domain.domain for domain in organisation.domains} == expected_domains def test_add_service_to_organisation(notify_db, notify_db_session, sample_service, sample_organisation): @@ -171,3 +228,30 @@ def test_add_user_to_organisation_when_user_does_not_exist(sample_organisation): def test_add_user_to_organisation_when_organisation_does_not_exist(sample_user): with pytest.raises(expected_exception=SQLAlchemyError): dao_add_user_to_organisation(organisation_id=uuid.uuid4(), user_id=sample_user.id) + + +@pytest.mark.parametrize('domain, expected_org', ( + ('unknown.gov.uk', False), + ('example.gov.uk', True), +)) +def test_get_organisation_by_email_address( + admin_request, + sample_user, + domain, + expected_org, +): + + org = create_organisation() + create_domain('example.gov.uk', org.id) + create_domain('test.gov.uk', org.id) + + another_org = create_organisation(name='Another') + create_domain('cabinet-office.gov.uk', another_org.id) + create_domain('cabinetoffice.gov.uk', another_org.id) + + found_org = dao_get_organisation_by_email_address('test@{}'.format(domain)) + + if expected_org: + assert found_org is org + else: + assert found_org is None diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 6461fd93c..cbc6cb24c 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -4,7 +4,7 @@ from datetime import datetime import pytest from freezegun import freeze_time from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm.exc import FlushError, NoResultFound +from sqlalchemy.orm.exc import NoResultFound from app import db from app.dao.inbound_numbers_dao import ( @@ -167,9 +167,9 @@ def test_cannot_create_service_with_no_user(notify_db_session): message_limit=1000, restricted=False, created_by=user) - with pytest.raises(FlushError) as excinfo: + with pytest.raises(ValueError) as excinfo: dao_create_service(service, None) - assert "Can't flush None value found in collection Service.users" in str(excinfo.value) + assert "Can't create a service without a user" in str(excinfo.value) def test_should_add_user_to_service(notify_db_session): diff --git a/tests/app/db.py b/tests/app/db.py index 3661a157f..8c082350a 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -52,7 +52,8 @@ from app.models import ( Complaint, InvitedUser, TemplateFolder, - LetterBranding + LetterBranding, + Domain, ) @@ -506,6 +507,16 @@ def create_annual_billing( return annual_billing +def create_domain(domain, organisation_id): + + domain = Domain(domain=domain, organisation_id=organisation_id) + + db.session.add(domain) + db.session.commit() + + return domain + + def create_organisation(name='test_org_1', active=True): data = { 'name': name, diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index dee403245..1bd0fecf7 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -32,10 +32,29 @@ def test_get_organisation_by_id(admin_request, notify_db_session): organisation_id=org.id ) - assert set(response.keys()) == {'id', 'name', 'active'} + assert set(response.keys()) == { + 'id', + 'name', + 'active', + 'crown', + 'organisation_type', + 'agreement_signed', + 'agreement_signed_at', + 'agreement_signed_by_id', + 'agreement_signed_version', + 'letter_branding_id', + 'email_branding_id', + } assert response['id'] == str(org.id) assert response['name'] == 'test_org_1' assert response['active'] is True + assert response['crown'] is None + assert response['organisation_type'] is None + assert response['agreement_signed'] is None + assert response['agreement_signed_by_id'] is None + assert response['agreement_signed_version'] is None + assert response['letter_branding_id'] is None + assert response['email_branding_id'] is None def test_post_create_organisation(admin_request, notify_db_session): @@ -91,12 +110,27 @@ def test_post_create_organisation_with_missing_name_gives_validation_error(admin assert response['errors'][0]['message'] == 'name is a required property' -def test_post_update_organisation_updates_fields(admin_request, notify_db_session): +@pytest.mark.parametrize('agreement_signed', ( + None, True, False +)) +@pytest.mark.parametrize('crown', ( + None, True, False +)) +def test_post_update_organisation_updates_fields( + admin_request, + notify_db_session, + agreement_signed, + crown, +): org = create_organisation() data = { 'name': 'new organisation name', - 'active': False + 'active': False, + 'agreement_signed': agreement_signed, + 'crown': crown, } + assert org.agreement_signed is None + assert org.crown is None admin_request.post( 'organisation.update_organisation', @@ -111,6 +145,39 @@ def test_post_update_organisation_updates_fields(admin_request, notify_db_sessio assert organisation[0].id == org.id assert organisation[0].name == data['name'] assert organisation[0].active == data['active'] + assert organisation[0].agreement_signed == agreement_signed + assert organisation[0].crown == crown + assert organisation[0].domains == [] + + +@pytest.mark.parametrize('domain_list', ( + ['example.com'], + ['example.com', 'example.org', 'example.net'], + [], +)) +def test_post_update_organisation_updates_domains( + admin_request, + notify_db_session, + domain_list, +): + org = create_organisation(name='test_org_2') + data = { + 'domains': domain_list, + } + + admin_request.post( + 'organisation.update_organisation', + _data=data, + organisation_id=org.id, + _expected_status=204 + ) + + organisation = Organisation.query.all() + + assert len(organisation) == 1 + assert [ + domain.domain for domain in organisation[0].domains + ] == domain_list def test_post_update_organisation_raises_400_on_existing_org_name( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 22208dd52..744f509e9 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -44,7 +44,9 @@ from tests.app.db import ( create_inbound_number, create_service_sms_sender, create_service_with_defined_sms_sender, - create_letter_branding + create_letter_branding, + create_organisation, + create_domain, ) from tests.app.db import create_user @@ -253,6 +255,55 @@ def test_create_service(admin_request, sample_user): assert service_sms_senders[0].sms_sender == current_app.config['FROM_NUMBER'] +@pytest.mark.parametrize('domain, expected_org', ( + (None, False), + ('', False), + ('unknown.gov.uk', False), + ('unknown-example.gov.uk', False), + ('example.gov.uk', True), + ('test.gov.uk', True), + ('test.example.gov.uk', True), +)) +def test_create_service_with_domain_sets_organisation( + admin_request, + sample_user, + domain, + expected_org, +): + + red_herring_org = create_organisation(name='Sub example') + create_domain('specific.example.gov.uk', red_herring_org.id) + create_domain('aaaaaaaa.example.gov.uk', red_herring_org.id) + + org = create_organisation() + create_domain('example.gov.uk', org.id) + create_domain('test.gov.uk', org.id) + + another_org = create_organisation(name='Another') + create_domain('cabinet-office.gov.uk', another_org.id) + create_domain('cabinetoffice.gov.uk', another_org.id) + + sample_user.email_address = 'test@{}'.format(domain) + + data = { + 'name': 'created service', + 'user_id': str(sample_user.id), + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'created.service', + 'created_by': str(sample_user.id), + 'service_domain': domain, + } + + json_resp = admin_request.post('service.create_service', _data=data, _expected_status=201) + + if expected_org: + assert json_resp['data']['organisation'] == str(org.id) + else: + assert json_resp['data']['organisation'] is None + + def test_create_service_with_domain_sets_letter_branding(admin_request, sample_user): letter_branding = create_letter_branding( name='test domain', filename='test-domain', domain='test.domain'