diff --git a/app/commands.py b/app/commands.py index 5a831c741..40118bd7c 100644 --- a/app/commands.py +++ b/app/commands.py @@ -276,14 +276,13 @@ def update_jobs_archived_flag(start_date, end_date): @notify_command(name='populate-organisations-from-file') @click.option('-f', '--file_name', required=True, - help="Pipe delimited file containing organisation name, sector, crown, argeement_signed, domains") + help="Pipe delimited file containing organisation name, sector, agreement_signed, domains") def populate_organisations_from_file(file_name): # [0] organisation name:: name of the organisation insert if organisation is missing. # [1] sector:: Federal | State only - # [2] crown:: TRUE | FALSE only - # [3] argeement_signed:: TRUE | FALSE - # [4] domains:: comma separated list of domains related to the organisation - # [5] email branding name: name of the default email branding for the org + # [2] agreement_signed:: TRUE | FALSE + # [3] domains:: comma separated list of domains related to the organisation + # [4] email branding name: name of the default email branding for the org # The expectation is that the organisation, organisation_to_service # and user_to_organisation will be cleared before running this command. @@ -308,7 +307,6 @@ def populate_organisations_from_file(file_name): 'name': columns[0], 'active': True, 'agreement_signed': boolean_or_none(columns[3]), - 'crown': boolean_or_none(columns[2]), 'organisation_type': columns[1].lower(), 'email_branding_id': email_branding.id if email_branding else None } diff --git a/app/dao/fact_billing_dao.py b/app/dao/fact_billing_dao.py index d6bbd54b4..187b2c860 100644 --- a/app/dao/fact_billing_dao.py +++ b/app/dao/fact_billing_dao.py @@ -357,7 +357,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): def _email_query(): return db.session.query( NotificationAllTimeView.template_id, - literal(service.crown).label('crown'), literal(service.id).label('service_id'), literal(notification_type).label('notification_type'), literal('ses').label('sent_by'), @@ -382,7 +381,6 @@ def _query_for_billing_data(notification_type, start_date, end_date, service): international = func.coalesce(NotificationAllTimeView.international, False) return db.session.query( NotificationAllTimeView.template_id, - literal(service.crown).label('crown'), literal(service.id).label('service_id'), literal(notification_type).label('notification_type'), sent_by.label('sent_by'), @@ -430,7 +428,7 @@ def get_service_ids_that_need_billing_populated(start_date, end_date): def get_rate( - rates, notification_type, date, crown=None + rates, notification_type, date ): start_of_day = get_local_midnight_in_utc(date) @@ -450,8 +448,7 @@ def update_fact_billing(data, process_day): rates = get_rates_for_billing() rate = get_rate(rates, data.notification_type, - process_day, - data.crown) + process_day) billing_record = create_billing_record(data, rate, process_day) table = FactBilling.__table__ diff --git a/app/dao/organisation_dao.py b/app/dao/organisation_dao.py index d489a8a54..2bb29b175 100644 --- a/app/dao/organisation_dao.py +++ b/app/dao/organisation_dao.py @@ -83,9 +83,6 @@ def dao_update_organisation(organisation_id, **kwargs): if 'organisation_type' in kwargs: _update_organisation_services(organisation, 'organisation_type', only_where_none=False) - if 'crown' in kwargs: - _update_organisation_services(organisation, 'crown', only_where_none=False) - if 'email_branding_id' in kwargs: _update_organisation_services(organisation, 'email_branding') @@ -111,7 +108,6 @@ def dao_add_service_to_organisation(service, organisation_id): service.organisation_id = organisation_id service.organisation_type = organisation.organisation_type - service.crown = organisation.crown db.session.add(service) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index e10a702b5..029763738 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -302,8 +302,6 @@ def dao_create_service( if organisation.email_branding: service.email_branding = organisation.email_branding - if organisation: - service.crown = organisation.crown service.count_as_live = not user.platform_admin db.session.add(service) diff --git a/app/models.py b/app/models.py index 72e5f158a..63f9cb377 100644 --- a/app/models.py +++ b/app/models.py @@ -330,7 +330,6 @@ 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) @@ -352,7 +351,6 @@ class Organisation(db.Model): agreement_signed_on_behalf_of_name = db.Column(db.String(255), nullable=True) 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), db.ForeignKey('organisation_types.name'), @@ -396,7 +394,6 @@ class Organisation(db.Model): "id": str(self.id), "name": self.name, "active": self.active, - "crown": self.crown, "organisation_type": self.organisation_type, "email_branding_id": self.email_branding_id, "agreement_signed": self.agreement_signed, @@ -457,7 +454,6 @@ class Service(db.Model, Versioned): unique=False, nullable=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/organisation/organisation_schema.py b/app/organisation/organisation_schema.py index acc90d42d..bfdec6df1 100644 --- a/app/organisation/organisation_schema.py +++ b/app/organisation/organisation_schema.py @@ -8,10 +8,9 @@ post_create_organisation_schema = { "properties": { "name": {"type": "string"}, "active": {"type": ["boolean", "null"]}, - "crown": {"type": "boolean"}, "organisation_type": {"enum": ORGANISATION_TYPES}, }, - "required": ["name", "crown", "organisation_type"] + "required": ["name", "organisation_type"] } post_update_organisation_schema = { @@ -21,7 +20,6 @@ post_update_organisation_schema = { "properties": { "name": {"type": ["string", "null"]}, "active": {"type": ["boolean", "null"]}, - "crown": {"type": ["boolean", "null"]}, "organisation_type": {"enum": ORGANISATION_TYPES}, }, "required": [] diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 30d819131..776940200 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -209,9 +209,8 @@ def send_notifications_on_mou_signed(organisation_id): send_notification_to_queue(saved_notification, research_mode=False, queue=QueueNames.NOTIFY) personalisation = { - 'mou_link': '{}/agreement/{}.pdf'.format( - current_app.config['ADMIN_BASE_URL'], - 'crown' if organisation.crown else 'non-crown' + 'mou_link': '{}/agreement/agreement.pdf'.format( + current_app.config['ADMIN_BASE_URL'] ), 'org_name': organisation.name, 'org_dashboard_link': '{}/organisations/{}'.format( diff --git a/app/schemas.py b/app/schemas.py index 6acf85396..3ba2ef677 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -258,7 +258,6 @@ class ServiceSchema(BaseSchema, UUIDsAsStringsMixin): 'complaints', 'contact_list', 'created_at', - 'crown', 'data_retention', 'guest_list', 'inbound_number', @@ -312,7 +311,6 @@ class DetailedServiceSchema(BaseSchema): 'api_keys', 'contact_list', 'created_by', - 'crown', 'email_branding', 'email_from', 'guest_list', diff --git a/migrations/versions/0393_remove_crown.py b/migrations/versions/0393_remove_crown.py new file mode 100644 index 000000000..73908a4f0 --- /dev/null +++ b/migrations/versions/0393_remove_crown.py @@ -0,0 +1,30 @@ +""" + +Revision ID: 0393_remove_crown +Revises: 0392_drop_letter_permissions +Create Date: 2023-04-10 14:13:38.207790 + +""" +from alembic import op +import sqlalchemy as sa + +revision = '0393_remove_crown' +down_revision = '0392_drop_letter_permissions' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('organisation', 'crown') + op.drop_column('organisation_types', 'is_crown') + op.drop_column('services', 'crown') + op.drop_column('services_history', 'crown') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('services_history', sa.Column('crown', sa.BOOLEAN(), autoincrement=False, nullable=True)) + op.add_column('services', sa.Column('crown', sa.BOOLEAN(), autoincrement=False, nullable=True)) + op.add_column('organisation_types', sa.Column('is_crown', sa.BOOLEAN(), autoincrement=False, nullable=True)) + op.add_column('organisation', sa.Column('crown', sa.BOOLEAN(), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 926c3d415..987e4bc8f 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -34,7 +34,7 @@ from tests.app.db import ( def mocker_get_rate( - non_letter_rates, notification_type, local_date, crown=None, rate_multiplier=None + non_letter_rates, notification_type, local_date, rate_multiplier=None ): if notification_type == SMS_TYPE: return Decimal(1.33) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index b3a2ff1c1..c08a0d5d6 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -211,8 +211,7 @@ def sample_service(sample_user): 'message_limit': 1000, 'restricted': False, 'email_from': email_from, - 'created_by': sample_user, - 'crown': True + 'created_by': sample_user } service = Service.query.filter_by(name=service_name).first() if not service: diff --git a/tests/app/dao/test_fact_billing_dao.py b/tests/app/dao/test_fact_billing_dao.py index 3ba91f2d4..edb4e3692 100644 --- a/tests/app/dao/test_fact_billing_dao.py +++ b/tests/app/dao/test_fact_billing_dao.py @@ -289,7 +289,7 @@ def test_get_rate_chooses_right_rate_depending_on_date(notify_db_session, date, create_rate(start_date=datetime(2018, 9, 30, 23, 0), value=2.2, notification_type='sms') rates = get_rates_for_billing() - rate = get_rate(rates, "sms", date, True) + rate = get_rate(rates, "sms", date) assert rate == expected_rate diff --git a/tests/app/dao/test_organisation_dao.py b/tests/app/dao/test_organisation_dao.py index 100778636..45cf5f589 100644 --- a/tests/app/dao/test_organisation_dao.py +++ b/tests/app/dao/test_organisation_dao.py @@ -62,7 +62,6 @@ def test_update_organisation(notify_db_session): data = { 'name': 'new name', - "crown": True, "organisation_type": 'state', "agreement_signed": True, "agreement_signed_at": datetime.datetime.utcnow(), @@ -193,26 +192,11 @@ def test_update_organisation_does_not_override_service_branding( assert sample_service.email_branding == custom_email_branding -def test_update_organisation_updates_services_with_new_crown_type( - sample_service, - sample_organisation -): - sample_organisation.services.append(sample_service) - db.session.commit() - - assert Service.query.get(sample_service.id).crown - - dao_update_organisation(sample_organisation.id, crown=False) - - assert not Service.query.get(sample_service.id).crown - - def test_add_service_to_organisation(sample_service, sample_organisation): assert sample_organisation.services == [] sample_service.organisation_type = "federal" sample_organisation.organisation_type = "state" - sample_organisation.crown = False dao_add_service_to_organisation(sample_service, sample_organisation.id) @@ -220,7 +204,6 @@ def test_add_service_to_organisation(sample_service, sample_organisation): assert sample_organisation.services[0].id == sample_service.id 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 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 0be894ef2..d6ec13c3f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -102,7 +102,6 @@ def test_create_service(notify_db_session): assert service.active is True assert user in service_db.users assert service_db.organisation_type == 'federal' - assert service_db.crown is None assert not service.organisation_id @@ -129,7 +128,6 @@ def test_create_service_with_organisation(notify_db_session): assert service.active is True assert user in service_db.users assert service_db.organisation_type == 'state' - assert service_db.crown is None assert service.organisation_id == organisation.id assert service.organisation == organisation diff --git a/tests/app/db.py b/tests/app/db.py index 78a8d845e..6bbfa708c 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -112,7 +112,6 @@ def create_service( check_if_service_exists=False, go_live_user=None, go_live_at=None, - crown=True, organisation=None, purchase_order_number=None, billing_contact_names=None, @@ -133,7 +132,6 @@ def create_service( organisation=organisation, go_live_user=go_live_user, go_live_at=go_live_at, - crown=crown, purchase_order_number=purchase_order_number, billing_contact_names=billing_contact_names, billing_contact_email_addresses=billing_contact_email_addresses, diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 650bcacf0..5aa2763f6 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -67,7 +67,6 @@ def test_get_organisation_by_id(admin_request, notify_db_session): 'id', 'name', 'active', - 'crown', 'organisation_type', 'agreement_signed', 'agreement_signed_at', @@ -88,7 +87,6 @@ def test_get_organisation_by_id(admin_request, notify_db_session): 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 @@ -160,12 +158,10 @@ def test_get_organisation_by_domain( assert response['result'] == 'error' -@pytest.mark.parametrize('crown', [True, False]) -def test_post_create_organisation(admin_request, notify_db_session, crown): +def test_post_create_organisation(admin_request, notify_db_session): data = { 'name': 'test organisation', 'active': True, - 'crown': crown, 'organisation_type': 'state', } @@ -179,7 +175,6 @@ def test_post_create_organisation(admin_request, notify_db_session, crown): assert data['name'] == response['name'] assert data['active'] == response['active'] - assert data['crown'] == response['crown'] assert data['organisation_type'] == response['organisation_type'] assert len(organisations) == 1 @@ -195,7 +190,6 @@ def test_post_create_organisation_sets_default_nhs_branding_for_nhs_orgs( data = { 'name': 'test organisation', 'active': True, - 'crown': False, 'organisation_type': org_type, } @@ -215,7 +209,6 @@ def test_post_create_organisation_existing_name_raises_400(admin_request, sample data = { 'name': sample_organisation.name, 'active': True, - 'crown': True, 'organisation_type': 'federal', } @@ -234,29 +227,15 @@ def test_post_create_organisation_existing_name_raises_400(admin_request, sample @pytest.mark.parametrize('data, expected_error', ( ({ 'active': False, - 'crown': True, 'organisation_type': 'federal', }, 'name is a required property'), ({ 'active': False, 'name': 'Service name', - 'organisation_type': 'federal', - }, 'crown is a required property'), - ({ - 'active': False, - 'name': 'Service name', - 'crown': True, }, 'organisation_type is a required property'), ({ 'active': False, 'name': 'Service name', - 'crown': None, - 'organisation_type': 'federal', - }, 'crown None is not of type boolean'), - ({ - 'active': False, - 'name': 'Service name', - 'crown': False, 'organisation_type': 'foo', }, ( 'organisation_type foo is not one of ' @@ -280,22 +259,16 @@ def test_post_create_organisation_with_missing_data_gives_validation_error( assert response['errors'][0]['message'] == expected_error -@pytest.mark.parametrize('crown', ( - None, True, False -)) def test_post_update_organisation_updates_fields( admin_request, notify_db_session, - crown, ): org = create_organisation() data = { 'name': 'new organisation name', 'active': False, - 'crown': crown, 'organisation_type': 'federal', } - assert org.crown is None admin_request.post( 'organisation.update_organisation', @@ -310,7 +283,6 @@ def test_post_update_organisation_updates_fields( assert organisation[0].id == org.id assert organisation[0].name == data['name'] assert organisation[0].active == data['active'] - assert organisation[0].crown == crown assert organisation[0].domains == [] assert organisation[0].organisation_type == 'federal' @@ -327,7 +299,7 @@ def test_post_update_organisation_updates_domains( ): org = create_organisation(name='test_org_2') data = { - 'domains': domain_list, + 'domains': domain_list } admin_request.post( @@ -354,9 +326,7 @@ def test_update_other_organisation_attributes_doesnt_clear_domains( admin_request.post( 'organisation.update_organisation', - _data={ - 'crown': True, - }, + _data={'domains': ['example.gov.uk']}, organisation_id=org.id, _expected_status=204 ) @@ -565,7 +535,7 @@ def test_post_update_organisation_set_mou_emails_signed_by( for n in notifications: # we pass in the same personalisation for all templates (though some templates don't use all fields) assert n.personalisation == { - 'mou_link': 'http://localhost:6012/agreement/non-crown.pdf', + 'mou_link': 'http://localhost:6012/agreement/agreement.pdf', 'org_name': 'sample organisation', 'org_dashboard_link': 'http://localhost:6012/organisations/{}'.format(sample_organisation.id), 'signed_by_name': 'Test User',