diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index bf20f6f4b..f901fce0a 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -309,7 +309,7 @@ def dao_create_service( if organisation.letter_branding and not service.letter_branding: service.letter_branding = organisation.letter_branding - elif service.organisation_type == 'nhs' or email_address_is_nhs(user.email_address): + elif service.organisation_type in ['nhs_central', 'nhs_local'] or email_address_is_nhs(user.email_address): service.email_branding = dao_get_email_branding_by_name('NHS') service.letter_branding = dao_get_letter_branding_by_name('NHS') diff --git a/app/models.py b/app/models.py index 3a40d8924..9b2885f9c 100644 --- a/app/models.py +++ b/app/models.py @@ -325,12 +325,11 @@ class Domain(db.Model): ORGANISATION_TYPES = [ - "central", "local", "nhs_central", "nhs", - "nhs_local", "emergency_service", "school_or_college", "other", + "central", "local", "nhs_central", "nhs_local", "emergency_service", "school_or_college", "other", ] CROWN_ORGANISATION_TYPES = ["nhs_central"] -NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_local", "emergency_service", "school_or_college", "nhs"] +NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_local", "emergency_service", "school_or_college"] class OrganisationTypes(db.Model): @@ -366,7 +365,12 @@ class Organisation(db.Model): agreement_signed_on_behalf_of_email_address = db.Column(db.String(255), 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) + organisation_type = db.Column( + db.String(255), + db.ForeignKey('organisation_types.name'), + unique=False, + nullable=True, + ) request_to_go_live_notes = db.Column(db.Text) domains = db.relationship( @@ -457,6 +461,8 @@ class Service(db.Model, Versioned): prefix_sms = db.Column(db.Boolean, nullable=False, default=True) organisation_type = db.Column( db.String(255), + db.ForeignKey('organisation_types.name'), + unique=False, nullable=True, ) crown = db.Column(db.Boolean, index=False, nullable=True) diff --git a/app/schemas.py b/app/schemas.py index 455b23470..33f5cd634 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -266,6 +266,7 @@ class ServiceSchema(BaseSchema): class DetailedServiceSchema(BaseSchema): statistics = fields.Dict() + organisation_type = field_for(models.Service, 'organisation_type') class Meta: model = models.Service diff --git a/migrations/versions/0300_migrate_org_types.py b/migrations/versions/0300_migrate_org_types.py new file mode 100644 index 000000000..9a9d1b4b7 --- /dev/null +++ b/migrations/versions/0300_migrate_org_types.py @@ -0,0 +1,75 @@ +import os + +""" + +Revision ID: 0300_migrate_org_types +Revises: 0299_org_types_table +Create Date: 2019-07-24 16:18:27.467361 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0300_migrate_org_types' +down_revision = '0299_org_types_table' + +environment = os.environ['NOTIFY_ENVIRONMENT'] + + +def upgrade(): + if environment not in ["live", "production"]: + op.execute(""" + UPDATE + organisation + SET + organisation_type = 'nhs_local' + WHERE + organisation.organisation_type = 'nhs' + """) + + op.execute(""" + UPDATE + services + SET + organisation_type = 'nhs_local' + WHERE + services.organisation_type = 'nhs' + """) + + op.alter_column('organisation_types', 'name', existing_type=sa.VARCHAR(), type_=sa.String(length=255)) + + op.create_foreign_key( + 'organisation_organisation_type_fkey', 'organisation', 'organisation_types', ['organisation_type'], ['name'] + ) + + op.create_foreign_key( + 'services_organisation_type_fkey', 'services', 'organisation_types', ['organisation_type'], ['name'] + ) + + +def downgrade(): + op.drop_constraint('services_organisation_type_fkey', 'services', type_='foreignkey') + + op.drop_constraint('organisation_organisation_type_fkey', 'organisation', type_='foreignkey') + + op.alter_column('organisation_types', 'name', existing_type=sa.String(length=255), type_=sa.VARCHAR()) + + if environment not in ["live", "production"]: + op.execute(""" + UPDATE + organisation + SET + organisation_type = 'nhs' + WHERE + organisation_type = 'nhs_local' + """) + + op.execute(""" + UPDATE + services + SET + organisation_type = 'nhs' + WHERE + organisation_type = 'nhs_local' + """) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 72414c2ba..61c1fc7da 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -110,8 +110,8 @@ def test_create_service(notify_db_session): @pytest.mark.parametrize('email_address, organisation_type', ( - ("test@example.gov.uk", 'nhs'), - ("test@nhs.net", 'nhs'), + ("test@example.gov.uk", 'nhs_central'), + ("test@nhs.net", 'nhs_local'), ("test@nhs.net", 'local'), ("test@nhs.net", 'central'), ("test@nhs.uk", 'central'), diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index fa230e2c9..b6d0f0341 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -204,7 +204,7 @@ def test_post_create_organisation_existing_name_raises_400(admin_request, sample 'name': 'Service name', 'crown': False, 'organisation_type': 'foo', - }, 'organisation_type foo is not one of [central, local, nhs_central, nhs, nhs_local, emergency_service, school_or_college, other]'), # noqa + }, 'organisation_type foo is not one of [central, local, nhs_central, nhs_local, emergency_service, school_or_college, other]'), # noqa )) def test_post_create_organisation_with_missing_data_gives_validation_error( admin_request, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index a741c1fed..85fea87e9 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -574,7 +574,7 @@ def test_update_service(client, notify_db, sample_service): 'email_from': 'updated.service.name', 'created_by': str(sample_service.created_by.id), 'email_branding': str(brand.id), - 'organisation_type': 'foo', + 'organisation_type': 'school_or_college', } auth_header = create_authorization_header() @@ -589,7 +589,25 @@ def test_update_service(client, notify_db, sample_service): assert result['data']['name'] == 'updated service name' assert result['data']['email_from'] == 'updated.service.name' assert result['data']['email_branding'] == str(brand.id) - assert result['data']['organisation_type'] == 'foo' + assert result['data']['organisation_type'] == 'school_or_college' + + +def test_cant_update_service_org_type_to_random_value(client, sample_service): + data = { + 'name': 'updated service name', + 'email_from': 'updated.service.name', + 'created_by': str(sample_service.created_by.id), + 'organisation_type': 'foo', + } + + auth_header = create_authorization_header() + + resp = client.post( + '/service/{}'.format(sample_service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + assert resp.status_code == 500 def test_update_service_letter_branding(client, notify_db, sample_service): diff --git a/tests/conftest.py b/tests/conftest.py index f8c6f798a..627250ca4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -102,6 +102,7 @@ def notify_db_session(notify_db): "provider_details_history", "template_process_type", "notification_status_types", + "organisation_types", "service_permission_types", "auth_type", "invite_status_type",