#224 remove crown (#228)

Co-authored-by: Kenneth Kehl <@kkehl@flexion.us>

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
+    # [2] agreement_signed:: TRUE | FALSE
-    # [3] argeement_signed:: TRUE | FALSE
+    # [3] domains:: comma separated list of domains related to the organisation
-    # [4] domains:: comma separated list of domains related to the organisation
+    # [4] email branding name: name of the default email branding for the org
-    # [5] 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
             }

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,
+                    process_day)
-                    data.crown)
     billing_record = create_billing_record(data, rate, process_day)
 
     table = FactBilling.__table__

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)
 

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)

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)

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": []

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(
+        'mou_link': '{}/agreement/agreement.pdf'.format(
-            current_app.config['ADMIN_BASE_URL'],
+            current_app.config['ADMIN_BASE_URL']
-            'crown' if organisation.crown else 'non-crown'
         ),
         'org_name': organisation.name,
         'org_dashboard_link': '{}/organisations/{}'.format(

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',

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 ###

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)

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,
+        'created_by': sample_user
-        'crown': True
     }
     service = Service.query.filter_by(name=service_name).first()
     if not service:

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
 
 

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

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
 

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,

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):
-def test_post_create_organisation(admin_request, notify_db_session, crown):
     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={
+        _data={'domains': ['example.gov.uk']},
-            'crown': True,
-        },
         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',