From b2213dad192bf5f4af3d851bb51792854dca1434 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Tue, 9 Feb 2021 09:38:43 +0000 Subject: [PATCH] Move provider restriction into broadcast settings This means we will have a much easier way of knowing what the settings are for a broadcast service. Note, we can just move data directly into the newer table as there is nothing on the API or admin app that is putting data in the `service_broadcast_provider_restriction` table, this was being done manually for the few services that needed it. --- app/models.py | 8 ++-- .../versions/0344_move_broadcast_provider.py | 39 +++++++++++++++++++ .../celery/test_broadcast_message_tasks.py | 13 ++----- tests/app/service/test_rest.py | 9 ++--- 4 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 migrations/versions/0344_move_broadcast_provider.py diff --git a/app/models.py b/app/models.py index e1e69dc22..7c51d6d7b 100644 --- a/app/models.py +++ b/app/models.py @@ -515,7 +515,7 @@ class Service(db.Model, Versioned): uselist=False, backref=db.backref('services', lazy='dynamic')) - allowed_broadcast_provider = association_proxy('service_broadcast_provider_restriction', 'provider') + allowed_broadcast_provider = association_proxy('service_broadcast_settings', 'provider') broadcast_channel = association_proxy('service_broadcast_settings', 'channel') @classmethod @@ -2543,9 +2543,6 @@ class ServiceBroadcastSettings(db.Model): this when the admin turns a service into a broadcast service, it inserts a row into this table and adds the service permission for broadcasts for the service. Once that is up and running, we then should write a DB migration to create rows for all broadcast services that do not have one yet in this table. - - TODO: Move functionality on the ServiceBroadcastProviderRestriction into this table and remove the - ServiceBroadcastProviderRestriction table """ __tablename__ = "service_broadcast_settings" @@ -2554,6 +2551,7 @@ class ServiceBroadcastSettings(db.Model): channel = db.Column( db.String(255), db.ForeignKey('broadcast_channel_types.name'), nullable=False ) + provider = db.Column(db.String, nullable=True) created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) @@ -2566,6 +2564,8 @@ class BroadcastChannelTypes(db.Model): class ServiceBroadcastProviderRestriction(db.Model): """ + TODO: Drop this table as no longer used + Most services don't send broadcasts. Of those that do, most send to all broadcast providers. However, some services don't send to all providers. These services are test services that we or the providers themselves use. diff --git a/migrations/versions/0344_move_broadcast_provider.py b/migrations/versions/0344_move_broadcast_provider.py new file mode 100644 index 000000000..259ec2fa9 --- /dev/null +++ b/migrations/versions/0344_move_broadcast_provider.py @@ -0,0 +1,39 @@ +""" + +Revision ID: 0344_move_broadcast_provider +Revises: 0343_org_billing_details +Create Date: 2021-02-09 09:19:07.957980 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0344_move_broadcast_provider' +down_revision = '0343_org_billing_details' + + +def upgrade(): + op.add_column('service_broadcast_settings', sa.Column('provider', sa.String(), nullable=True)) + + sql = """ + select service_id, provider + from service_broadcast_provider_restriction + where service_id NOT IN (select service_id from service_broadcast_settings) + """ + insert_sql = """ + insert into service_broadcast_settings(service_id, channel, provider, created_at, updated_at) + values('{}', 'test', '{}', now(), null) + """ + conn = op.get_bind() + results = conn.execute(sql) + restrictions = results.fetchall() + for x in restrictions: + f = insert_sql.format(x.service_id, x.provider) + conn.execute(f) + + +def downgrade(): + # Downgrade does not try and fully undo the upgrade, in particular it does not + # delete the rows added to the service_broadcast_settings table + op.drop_column('service_broadcast_settings', 'provider') diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index ade0d238e..a303b0448 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -9,7 +9,6 @@ from app.models import ( BroadcastStatusType, BroadcastEventMessageType, BroadcastProviderMessageStatus, - ServiceBroadcastProviderRestriction, ServiceBroadcastSettings, ) from app.celery.broadcast_message_tasks import send_broadcast_event, send_broadcast_provider_message, trigger_link_test @@ -47,10 +46,8 @@ def test_send_broadcast_event_only_sends_to_one_provider_if_set_on_service( notify_api, sample_service ): - notify_db.session.add(ServiceBroadcastProviderRestriction( - service=sample_service, - provider='vodafone' - )) + settings = ServiceBroadcastSettings(service=sample_service, channel="test", provider="vodafone") + notify_db.session.add(settings) template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING) @@ -74,10 +71,8 @@ def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabl notify_api, sample_service ): - notify_db.session.add(ServiceBroadcastProviderRestriction( - service=sample_service, - provider='three' - )) + settings = ServiceBroadcastSettings(service=sample_service, channel="test", provider="three") + notify_db.session.add(settings) template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 937518a2a..efcb45ff7 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -19,7 +19,7 @@ from app.models import ( Notification, Permission, Service, - ServiceBroadcastProviderRestriction, + ServiceBroadcastSettings, ServiceEmailReplyTo, ServiceLetterContact, ServicePermission, @@ -280,11 +280,8 @@ def test_get_service_by_id(admin_request, sample_service): def test_get_service_by_id_returns_allowed_broadcast_provider(notify_db, admin_request, sample_service): - notify_db.session.add(ServiceBroadcastProviderRestriction( - service=sample_service, - provider='ee' - )) - notify_db.session.commit() + settings = ServiceBroadcastSettings(service=sample_service, channel="severe", provider="ee") + notify_db.session.add(settings) json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id) assert json_resp['data']['id'] == str(sample_service.id)