From 9dfc550f462ad4e161af51f06c0b1f473c89c621 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 10 Jul 2019 18:17:33 +0100 Subject: [PATCH 1/7] Add org types table --- app/dao/organisation_dao.py | 5 +++ app/dao/services_dao.py | 6 ++- app/models.py | 19 ++++++-- app/service/rest.py | 7 +-- migrations/versions/0299_org_types_table_.py | 47 ++++++++++++++++++++ tests/app/conftest.py | 3 +- tests/app/dao/test_services_dao.py | 2 +- tests/app/db.py | 6 ++- tests/app/service/test_rest.py | 13 ++++-- 9 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 migrations/versions/0299_org_types_table_.py diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index 4d4060469..a7a5211a3 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -101,9 +101,14 @@ def dao_add_service_to_organisation(service, organisation_id): organisation.services.append(service) service.organisation_type = organisation.organisation_type + service.crown = organisation.crown + db.session.add(service) + + + def dao_get_invited_organisation_user(user_id): return InvitedOrganisationUser.query.filter_by(id=user_id).one() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 2523a4d2a..f67b038d7 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -41,6 +41,7 @@ from app.models import ( EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, KEY_TYPE_TEST, + NON_CROWN_ORGANISATION_TYPES, SMS_TYPE, LETTER_TYPE, ) @@ -282,7 +283,10 @@ def dao_create_service( service.id = service_id or uuid.uuid4() # must be set now so version history model can use same id service.active = True service.research_mode = False - service.crown = service.organisation_type == 'central' + if organisation: + service.crown = organisation.crown + elif service.organisation_type in (NON_CROWN_ORGANISATION_TYPES + ['nhs']): + service.crown = False service.count_as_live = not user.platform_admin for permission in service_permissions: diff --git a/app/models.py b/app/models.py index 3db41ff6d..a0ff1fb71 100644 --- a/app/models.py +++ b/app/models.py @@ -61,8 +61,6 @@ DELIVERY_STATUS_CALLBACK_TYPE = 'delivery_status' COMPLAINT_CALLBACK_TYPE = 'complaint' SERVICE_CALLBACK_TYPES = [DELIVERY_STATUS_CALLBACK_TYPE, COMPLAINT_CALLBACK_TYPE] -ORGANISATION_TYPES = ['central', 'local', 'nhs'] - def filter_null_value_fields(obj): return dict( @@ -326,6 +324,21 @@ class Domain(db.Model): organisation_id = db.Column('organisation_id', UUID(as_uuid=True), db.ForeignKey('organisation.id'), nullable=False) +ORGANISATION_TYPES = [ + "central", "local", "nhs_central", + "nhs_local", "emergency_services", "school_or_college", "other", +] +NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_central", "nhs_local", "emergency_services", "school_or_college"] + + +class OrganisationTypes(db.Model): + __tablename__ = 'organisation_types' + + name = db.Column(db.String(255), primary_key=True) + is_crown = db.Column(db.Boolean, nullable=True) + annual_free_sms_fragment_limit = db.Column(db.BigInteger, nullable=False) + + class Organisation(db.Model): __tablename__ = "organisation" id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4, unique=False) @@ -444,7 +457,7 @@ class Service(db.Model, Versioned): db.String(255), nullable=True, ) - crown = db.Column(db.Boolean, index=False, nullable=False, default=True) + crown = db.Column(db.Boolean, index=False, nullable=True) rate_limit = db.Column(db.Integer, index=False, nullable=False, default=3000) contact_link = db.Column(db.String(255), nullable=True, unique=False) volume_sms = db.Column(db.Integer(), nullable=True, unique=False) diff --git a/app/service/rest.py b/app/service/rest.py index e3f627cb6..997f0d7a6 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -86,7 +86,8 @@ from app.errors import ( ) from app.letters.utils import letter_print_day from app.models import ( - KEY_TYPE_NORMAL, LETTER_TYPE, NOTIFICATION_CANCELLED, Permission, Service, EmailBranding, LetterBranding + KEY_TYPE_NORMAL, LETTER_TYPE, NON_CROWN_ORGANISATION_TYPES, NOTIFICATION_CANCELLED, Permission, Service, + EmailBranding, LetterBranding ) from app.notifications.process_notifications import persist_notification, send_notification_to_queue from app.schema_validation import validate @@ -224,8 +225,8 @@ def update_service(service_id): service = service_schema.load(current_data).data org_type = req_json.get('organisation_type', None) - if org_type: - service.crown = org_type == 'central' + if org_type and service.organisation_type in (NON_CROWN_ORGANISATION_TYPES + ["nhs"]): + service.crown = False if 'email_branding' in req_json: email_branding_id = req_json['email_branding'] diff --git a/migrations/versions/0299_org_types_table_.py b/migrations/versions/0299_org_types_table_.py new file mode 100644 index 000000000..c9c3dca92 --- /dev/null +++ b/migrations/versions/0299_org_types_table_.py @@ -0,0 +1,47 @@ +""" + +Revision ID: 0299_org_types_table +Revises: 0298_add_mou_signed_receipt +Create Date: 2019-07-10 16:07:22.019759 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0299_org_types_table' +down_revision = '0298_add_mou_signed_receipt' + + +def upgrade(): + organisation_types_table = op.create_table( + 'organisation_types', + sa.Column('name', sa.String(), nullable=False), + sa.PrimaryKeyConstraint('name'), + sa.Column('is_crown', sa.Boolean, nullable=True), + sa.Column('annual_free_sms_fragment_limit', sa.BigInteger, nullable=False) + + ) + + op.bulk_insert( + organisation_types_table, + [ + {'name': x, 'is_crown': y, 'annual_free_sms_fragment_limit': z} for x, y, z in [ + ["central", None, 250000], + ["local", False, 25000], + ["nhs_central", False, 250000], + ["nhs_local", False, 25000], + ["emergency_services", False, 25000], + ["school_or_college", False, 25000], + ["other", None, 25000], + ] + ] + ) + op.alter_column('services', 'crown', nullable=True) + op.alter_column('services_history', 'crown', nullable=True) + + +def downgrade(): + op.execute('DROP TABLE organisation_types') + op.alter_column('services', 'crown', nullable=False) + op.alter_column('services_history', 'crown', nullable=False) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 2ed53996b..367b577ee 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -176,7 +176,8 @@ def sample_service( 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, - 'created_by': user + 'created_by': user, + 'crown': True } service = Service.query.filter_by(name=service_name).first() if not service: diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 4b9b6b774..f69a60f3a 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -105,7 +105,7 @@ def test_create_service(notify_db_session): assert service.active is True assert user in service_db.users assert service_db.organisation_type == 'central' - assert service_db.crown is True + assert service_db.crown is None assert not service.letter_branding diff --git a/tests/app/db.py b/tests/app/db.py index 38fa3ed92..9a6250769 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -107,7 +107,8 @@ def create_service( organisation_type='central', check_if_service_exists=False, go_live_user=None, - go_live_at=None + go_live_at=None, + crown=True ): if check_if_service_exists: service = Service.query.filter_by(name=service_name).first() @@ -121,7 +122,8 @@ def create_service( prefix_sms=prefix_sms, organisation_type=organisation_type, go_live_user=go_live_user, - go_live_at=go_live_at + go_live_at=go_live_at, + crown=crown ) dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 0ba6c15a8..f5a40ea68 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -697,10 +697,15 @@ def test_update_service_flags(client, sample_service): @pytest.mark.parametrize("org_type, expected", - [("central", True), - ('local', False), - ("nhs", False)]) -def test_update_service_sets_crown(client, sample_service, org_type, expected): + [("central", None), + ("local", False), + ("nhs_central", False), + ("nhs_local", False), + ("emergency_services", False), + ("school_or_college", False), + ("other", None)]) +def test_update_service_sets_crown_based_on_org_type(client, sample_service, org_type, expected): + sample_service.crown = None data = { 'organisation_type': org_type, } From 8a3ac8400fb559dd7a946d342d5d95d36c77299f Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 15 Jul 2019 14:39:21 +0100 Subject: [PATCH 2/7] Update service type and crown when service is added to organisation --- app/organisation/organisation_schema.py | 2 +- ...rg_types_table_.py => 0299_org_types_table.py} | 0 tests/app/dao/test_organisation_dao.py | 12 ++++++++---- tests/app/organisation/test_rest.py | 15 +++++---------- 4 files changed, 14 insertions(+), 15 deletions(-) rename migrations/versions/{0299_org_types_table_.py => 0299_org_types_table.py} (100%) diff --git a/app/organisation/organisation_schema.py b/app/organisation/organisation_schema.py index 44018385e..0a165caa6 100644 --- a/app/organisation/organisation_schema.py +++ b/app/organisation/organisation_schema.py @@ -8,7 +8,7 @@ post_create_organisation_schema = { "properties": { "name": {"type": "string"}, "active": {"type": ["boolean", "null"]}, - "crown": {"type": "boolean"}, + "crown": {"type": ["boolean", "null"]}, "organisation_type": {"enum": ORGANISATION_TYPES}, }, "required": ["name", "crown", "organisation_type"] diff --git a/migrations/versions/0299_org_types_table_.py b/migrations/versions/0299_org_types_table.py similarity index 100% rename from migrations/versions/0299_org_types_table_.py rename to migrations/versions/0299_org_types_table.py diff --git a/tests/app/dao/test_organisation_dao.py b/tests/app/dao/test_organisation_dao.py index c73c97729..f5862aaab 100644 --- a/tests/app/dao/test_organisation_dao.py +++ b/tests/app/dao/test_organisation_dao.py @@ -158,19 +158,23 @@ def test_update_organisation_updates_the_service_org_type_if_org_type_is_provide def test_add_service_to_organisation(sample_service, sample_organisation): - sample_service.organisation_type = 'local' - sample_organisation.organisation_type = 'central' assert sample_organisation.services == [] + sample_service.organisation_type = "central" + sample_organisation.organisation_type = "local" + sample_organisation.crown = False + dao_add_service_to_organisation(sample_service, sample_organisation.id) assert len(sample_organisation.services) == 1 assert sample_organisation.services[0].id == sample_service.id - assert sample_organisation.services[0].organisation_type == 'central' + + assert sample_service.organisation_type == sample_organisation.organisation_type + assert sample_service.crown == sample_organisation.crown assert Service.get_history_model().query.filter_by( id=sample_service.id, version=2 - ).one().organisation_type == 'central' + ).one().organisation_type == sample_organisation.organisation_type def test_add_service_to_multiple_organisation_raises_error(sample_service, sample_organisation): diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index d016a67f7..3250c57e0 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -132,11 +132,12 @@ def test_get_organisation_by_domain( assert response['result'] == 'error' -def test_post_create_organisation(admin_request, notify_db_session): +@pytest.mark.parametrize('crown', [True, False, None]) +def test_post_create_organisation(admin_request, notify_db_session, crown): data = { 'name': 'test organisation', 'active': True, - 'crown': False, + 'crown': crown, 'organisation_type': 'local', } @@ -192,20 +193,14 @@ def test_post_create_organisation_existing_name_raises_400(admin_request, sample 'name': 'Service name', 'crown': True, }, 'organisation_type is a required property'), - ({ - 'active': False, - 'name': 'Service name', - 'crown': None, - 'organisation_type': 'central', - }, 'crown None is not of type boolean'), ({ 'active': False, 'name': 'Service name', 'crown': False, 'organisation_type': 'foo', - }, 'organisation_type foo is not one of [central, local, nhs]'), + }, 'organisation_type foo is not one of [central, local, nhs_central, nhs_local, emergency_services, school_or_college, other]'), # noqa )) -def test_post_create_organisation_with_missing_name_gives_validation_error( +def test_post_create_organisation_with_missing_data_gives_validation_error( admin_request, notify_db_session, data, From e4cb56b5d3436d80dc530dc807ccf4640545df77 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 15 Jul 2019 16:28:54 +0100 Subject: [PATCH 3/7] Emergency service instead of emergency services --- app/models.py | 4 ++-- app/organisation/organisation_schema.py | 2 +- migrations/versions/0299_org_types_table.py | 2 +- tests/app/organisation/test_rest.py | 10 ++++++++-- tests/app/service/test_rest.py | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/models.py b/app/models.py index a0ff1fb71..90411419c 100644 --- a/app/models.py +++ b/app/models.py @@ -326,9 +326,9 @@ class Domain(db.Model): ORGANISATION_TYPES = [ "central", "local", "nhs_central", - "nhs_local", "emergency_services", "school_or_college", "other", + "nhs_local", "emergency_service", "school_or_college", "other", ] -NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_central", "nhs_local", "emergency_services", "school_or_college"] +NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_central", "nhs_local", "emergency_service", "school_or_college"] class OrganisationTypes(db.Model): diff --git a/app/organisation/organisation_schema.py b/app/organisation/organisation_schema.py index 0a165caa6..44018385e 100644 --- a/app/organisation/organisation_schema.py +++ b/app/organisation/organisation_schema.py @@ -8,7 +8,7 @@ post_create_organisation_schema = { "properties": { "name": {"type": "string"}, "active": {"type": ["boolean", "null"]}, - "crown": {"type": ["boolean", "null"]}, + "crown": {"type": "boolean"}, "organisation_type": {"enum": ORGANISATION_TYPES}, }, "required": ["name", "crown", "organisation_type"] diff --git a/migrations/versions/0299_org_types_table.py b/migrations/versions/0299_org_types_table.py index c9c3dca92..6b273519d 100644 --- a/migrations/versions/0299_org_types_table.py +++ b/migrations/versions/0299_org_types_table.py @@ -31,7 +31,7 @@ def upgrade(): ["local", False, 25000], ["nhs_central", False, 250000], ["nhs_local", False, 25000], - ["emergency_services", False, 25000], + ["emergency_service", False, 25000], ["school_or_college", False, 25000], ["other", None, 25000], ] diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 3250c57e0..b6d0f0341 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -132,7 +132,7 @@ def test_get_organisation_by_domain( assert response['result'] == 'error' -@pytest.mark.parametrize('crown', [True, False, None]) +@pytest.mark.parametrize('crown', [True, False]) def test_post_create_organisation(admin_request, notify_db_session, crown): data = { 'name': 'test organisation', @@ -193,12 +193,18 @@ def test_post_create_organisation_existing_name_raises_400(admin_request, sample 'name': 'Service name', 'crown': True, }, 'organisation_type is a required property'), + ({ + 'active': False, + 'name': 'Service name', + 'crown': None, + 'organisation_type': 'central', + }, 'crown None is not of type boolean'), ({ 'active': False, 'name': 'Service name', 'crown': False, 'organisation_type': 'foo', - }, 'organisation_type foo is not one of [central, local, nhs_central, nhs_local, emergency_services, 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 f5a40ea68..0d02a5140 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -701,7 +701,7 @@ def test_update_service_flags(client, sample_service): ("local", False), ("nhs_central", False), ("nhs_local", False), - ("emergency_services", False), + ("emergency_service", False), ("school_or_college", False), ("other", None)]) def test_update_service_sets_crown_based_on_org_type(client, sample_service, org_type, expected): From d89ef0594f6318fc64a569a4d1f736d1d98848d3 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 16 Jul 2019 14:56:04 +0100 Subject: [PATCH 4/7] nhs_central is crown org and also we do not update org type while updating service anymore. --- app/dao/organisation_dao.py | 3 --- app/dao/services_dao.py | 3 +++ app/models.py | 4 +++- app/service/rest.py | 5 +---- migrations/versions/0299_org_types_table.py | 2 +- tests/app/dao/test_services_dao.py | 4 ++-- tests/app/service/test_rest.py | 25 --------------------- 7 files changed, 10 insertions(+), 36 deletions(-) diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index a7a5211a3..13a60b6f2 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -106,9 +106,6 @@ def dao_add_service_to_organisation(service, organisation_id): db.session.add(service) - - - def dao_get_invited_organisation_user(user_id): return InvitedOrganisationUser.query.filter_by(id=user_id).one() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index f67b038d7..b4fee76c6 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -38,6 +38,7 @@ from app.models import ( TemplateRedacted, User, VerifyCode, + CROWN_ORGANISATION_TYPES, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, KEY_TYPE_TEST, @@ -285,6 +286,8 @@ def dao_create_service( service.research_mode = False if organisation: service.crown = organisation.crown + elif service.organisation_type in CROWN_ORGANISATION_TYPES: + service.crown = True elif service.organisation_type in (NON_CROWN_ORGANISATION_TYPES + ['nhs']): service.crown = False service.count_as_live = not user.platform_admin diff --git a/app/models.py b/app/models.py index 90411419c..a5ee4d4dc 100644 --- a/app/models.py +++ b/app/models.py @@ -328,7 +328,9 @@ ORGANISATION_TYPES = [ "central", "local", "nhs_central", "nhs_local", "emergency_service", "school_or_college", "other", ] -NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_central", "nhs_local", "emergency_service", "school_or_college"] + +CROWN_ORGANISATION_TYPES = ["nhs_central"] +NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_local", "emergency_service", "school_or_college"] class OrganisationTypes(db.Model): diff --git a/app/service/rest.py b/app/service/rest.py index 997f0d7a6..f36d4b729 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -86,7 +86,7 @@ from app.errors import ( ) from app.letters.utils import letter_print_day from app.models import ( - KEY_TYPE_NORMAL, LETTER_TYPE, NON_CROWN_ORGANISATION_TYPES, NOTIFICATION_CANCELLED, Permission, Service, + KEY_TYPE_NORMAL, LETTER_TYPE, NOTIFICATION_CANCELLED, Permission, Service, EmailBranding, LetterBranding ) from app.notifications.process_notifications import persist_notification, send_notification_to_queue @@ -224,9 +224,6 @@ def update_service(service_id): current_data.update(request.get_json()) service = service_schema.load(current_data).data - org_type = req_json.get('organisation_type', None) - if org_type and service.organisation_type in (NON_CROWN_ORGANISATION_TYPES + ["nhs"]): - service.crown = False if 'email_branding' in req_json: email_branding_id = req_json['email_branding'] diff --git a/migrations/versions/0299_org_types_table.py b/migrations/versions/0299_org_types_table.py index 6b273519d..667830bdf 100644 --- a/migrations/versions/0299_org_types_table.py +++ b/migrations/versions/0299_org_types_table.py @@ -29,7 +29,7 @@ def upgrade(): {'name': x, 'is_crown': y, 'annual_free_sms_fragment_limit': z} for x, y, z in [ ["central", None, 250000], ["local", False, 25000], - ["nhs_central", False, 250000], + ["nhs_central", True, 250000], ["nhs_local", False, 25000], ["emergency_service", False, 25000], ["school_or_college", False, 25000], diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index f69a60f3a..72414c2ba 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -389,7 +389,7 @@ def test_get_all_user_services_should_return_empty_list_if_no_services_for_user( @freeze_time('2019-04-23T10:00:00') def test_dao_fetch_live_services_data(sample_user): - org = create_organisation(organisation_type='crown') + org = create_organisation(organisation_type='nhs_central') service = create_service(go_live_user=sample_user, go_live_at='2014-04-20T10:00:00') template = create_template(service=service) service_2 = create_service(service_name='second', go_live_at='2017-04-20T10:00:00', go_live_user=sample_user) @@ -427,7 +427,7 @@ def test_dao_fetch_live_services_data(sample_user): # checks the results and that they are ordered by date: assert results == [ {'service_id': mock.ANY, 'service_name': 'Sample service', 'organisation_name': 'test_org_1', - 'organisation_type': 'crown', 'consent_to_research': None, 'contact_name': 'Test User', + 'organisation_type': 'nhs_central', 'consent_to_research': None, 'contact_name': 'Test User', 'contact_email': 'notify@digital.cabinet-office.gov.uk', 'contact_mobile': '+447700900986', 'live_date': datetime(2014, 4, 20, 10, 0), 'sms_volume_intent': None, 'email_volume_intent': None, 'letter_volume_intent': None, 'sms_totals': 2, 'email_totals': 1, 'letter_totals': 1, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 0d02a5140..39641c996 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -696,31 +696,6 @@ def test_update_service_flags(client, sample_service): assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) -@pytest.mark.parametrize("org_type, expected", - [("central", None), - ("local", False), - ("nhs_central", False), - ("nhs_local", False), - ("emergency_service", False), - ("school_or_college", False), - ("other", None)]) -def test_update_service_sets_crown_based_on_org_type(client, sample_service, org_type, expected): - sample_service.crown = None - data = { - 'organisation_type': org_type, - } - auth_header = create_authorization_header() - - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = resp.json - assert resp.status_code == 200 - assert result['data']['crown'] is expected - - @pytest.mark.parametrize('field', ( 'volume_email', 'volume_sms', From 610bf5d149ba938e81560300582783238e31f7f0 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 18 Jul 2019 15:13:39 +0100 Subject: [PATCH 5/7] For now include nhs type orgs in allowed organisation types --- app/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models.py b/app/models.py index a5ee4d4dc..125136457 100644 --- a/app/models.py +++ b/app/models.py @@ -325,7 +325,7 @@ class Domain(db.Model): ORGANISATION_TYPES = [ - "central", "local", "nhs_central", + "central", "local", "nhs_central", "nhs", "nhs_local", "emergency_service", "school_or_college", "other", ] From aa88252cf7d89ace61d1ad2a463d1fcdecd74747 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 18 Jul 2019 15:16:37 +0100 Subject: [PATCH 6/7] Drop making crown not nullable on downgrade as it would fail anyway --- migrations/versions/0299_org_types_table.py | 2 -- tests/app/organisation/test_rest.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/migrations/versions/0299_org_types_table.py b/migrations/versions/0299_org_types_table.py index 667830bdf..bc7df5cd7 100644 --- a/migrations/versions/0299_org_types_table.py +++ b/migrations/versions/0299_org_types_table.py @@ -43,5 +43,3 @@ def upgrade(): def downgrade(): op.execute('DROP TABLE organisation_types') - op.alter_column('services', 'crown', nullable=False) - op.alter_column('services_history', 'crown', nullable=False) diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index b6d0f0341..fa230e2c9 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_local, emergency_service, school_or_college, other]'), # noqa + }, 'organisation_type foo is not one of [central, local, nhs_central, nhs, nhs_local, emergency_service, school_or_college, other]'), # noqa )) def test_post_create_organisation_with_missing_data_gives_validation_error( admin_request, From edda816ffa2abfb513035e34484ee88d6d7c2871 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 19 Jul 2019 15:13:24 +0100 Subject: [PATCH 7/7] Temporarily add generic 'nhs' --- app/dao/services_dao.py | 2 +- app/models.py | 2 +- migrations/versions/0299_org_types_table.py | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index b4fee76c6..bf20f6f4b 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -288,7 +288,7 @@ def dao_create_service( service.crown = organisation.crown elif service.organisation_type in CROWN_ORGANISATION_TYPES: service.crown = True - elif service.organisation_type in (NON_CROWN_ORGANISATION_TYPES + ['nhs']): + elif service.organisation_type in NON_CROWN_ORGANISATION_TYPES: service.crown = False service.count_as_live = not user.platform_admin diff --git a/app/models.py b/app/models.py index 125136457..3a40d8924 100644 --- a/app/models.py +++ b/app/models.py @@ -330,7 +330,7 @@ ORGANISATION_TYPES = [ ] CROWN_ORGANISATION_TYPES = ["nhs_central"] -NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_local", "emergency_service", "school_or_college"] +NON_CROWN_ORGANISATION_TYPES = ["local", "nhs_local", "emergency_service", "school_or_college", "nhs"] class OrganisationTypes(db.Model): diff --git a/migrations/versions/0299_org_types_table.py b/migrations/versions/0299_org_types_table.py index bc7df5cd7..47468b1a5 100644 --- a/migrations/versions/0299_org_types_table.py +++ b/migrations/versions/0299_org_types_table.py @@ -29,6 +29,7 @@ def upgrade(): {'name': x, 'is_crown': y, 'annual_free_sms_fragment_limit': z} for x, y, z in [ ["central", None, 250000], ["local", False, 25000], + ["nhs", None, 25000], ["nhs_central", True, 250000], ["nhs_local", False, 25000], ["emergency_service", False, 25000],