From 8d0adf7f05ba8d1c903c3efbaede9bbcb05f6654 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 1 Oct 2024 10:51:30 +0100 Subject: [PATCH 1/9] migration to delete broadcast data this is a data migration (so no downgrade) that deletes broadcast services and all associated data - this includes obvious things like broadcast events and templates, but also everything connected to those services like user permissions, inbound senders, ft_billing rows, etc Although some of these theoretically shouldn't exist for broadcast services (eg returned_letters), issue the deletes out of an abundance of caution. These migrations need to pass on preview and staging as well as production, and for example there was a broadcast service on staging with an inbound sms number This has been tested against a copy of the staging database. --- migrations/.current-alembic-head | 2 +- .../versions/0465_delete_broadcast_data.py | 100 ++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 migrations/versions/0465_delete_broadcast_data.py diff --git a/migrations/.current-alembic-head b/migrations/.current-alembic-head index 6dc3328442..521b5c414f 100644 --- a/migrations/.current-alembic-head +++ b/migrations/.current-alembic-head @@ -1 +1 @@ -0464_create_svc_join_requests +0465_delete_broadcast_data diff --git a/migrations/versions/0465_delete_broadcast_data.py b/migrations/versions/0465_delete_broadcast_data.py new file mode 100644 index 0000000000..54e680d2ad --- /dev/null +++ b/migrations/versions/0465_delete_broadcast_data.py @@ -0,0 +1,100 @@ +from alembic import op +from sqlalchemy.sql import text +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "0465_delete_broadcast_data" +down_revision = "0464_create_svc_join_requests" + + +def upgrade(): + """ + Removes all existance of broadcast specific services and all their associated data: + + deletes ALL data from the following tables: + + * broadcast_event + * broadcast_message + * broadcast_provider_message + * broadcast_provider_message_number + * service_broadcast_provider_restriction + * service_broadcast_settings + + deletes all data linked to a broadcast service from any table with a `service_id` (or a `template_id` for templates + belonging to a broadcast service). + + this does not touch the following static type tables + + * broadcast_channel_types + * broadcast_provider_message_status_type + * broadcast_provider_types + * broadcast_status_type + + this does not touch the following sequence + + * broadcast_provider_message_number_seq + """ + + conn = op.get_bind() + results = conn.execute("SELECT service_id FROM service_broadcast_settings;") + res = results.fetchall() + broadcast_service_ids = tuple(x.service_id for x in res) + + # these are broadly alphabetical, but with some lines reordered due to precedence (eg needing to delete + # user_folder_permissions before we can delete the template folders they reference) + delete_statements = [ + # step 1: delete the entire tables for the broadcast specific things + "DELETE FROM broadcast_provider_message_number;", + "DELETE FROM broadcast_provider_message;", + "DELETE FROM broadcast_event;", + "DELETE FROM broadcast_message;", + "DELETE FROM service_broadcast_settings;", + "DELETE FROM service_broadcast_provider_restriction;", + # all these tables have service_id, so need to be deleted before we can delete the service, + # (note this includes templates as well, which we need to delete), + "DELETE FROM annual_billing WHERE service_id in :service_ids;", + "DELETE FROM api_keys WHERE service_id in :service_ids;", + "DELETE FROM complaints WHERE service_id in :service_ids;", + "DELETE FROM service_sms_senders WHERE service_id in :service_ids;", + "DELETE FROM inbound_numbers WHERE service_id in :service_ids;", + "DELETE FROM inbound_sms WHERE service_id in :service_ids;", + "DELETE FROM inbound_sms_history WHERE service_id in :service_ids;", + "DELETE FROM invited_users WHERE service_id in :service_ids;", + "DELETE FROM jobs WHERE service_id in :service_ids;", + "DELETE FROM notification_history WHERE service_id in :service_ids;", + "DELETE FROM notifications WHERE service_id in :service_ids;", + "DELETE FROM permissions WHERE service_id in :service_ids;", + "DELETE FROM returned_letters WHERE service_id in :service_ids;", + "DELETE FROM service_callback_api WHERE service_id in :service_ids;", + "DELETE FROM service_contact_list WHERE service_id in :service_ids;", + "DELETE FROM service_data_retention WHERE service_id in :service_ids;", + "DELETE FROM service_email_branding WHERE service_id in :service_ids;", + "DELETE FROM service_email_reply_to WHERE service_id in :service_ids;", + "DELETE FROM service_inbound_api WHERE service_id in :service_ids;", + "DELETE FROM service_join_requests WHERE service_id in :service_ids;", + "DELETE FROM service_letter_branding WHERE service_id in :service_ids;", + "DELETE FROM service_permissions WHERE service_id in :service_ids;", + "DELETE FROM service_whitelist WHERE service_id in :service_ids;", + "DELETE FROM user_folder_permissions WHERE service_id in :service_ids", + "DELETE FROM template_folder_map WHERE template_id in (SELECT id FROM templates WHERE service_id in :service_ids);", + "DELETE FROM template_redacted WHERE template_id in (SELECT id FROM templates WHERE service_id in :service_ids);", + "DELETE FROM template_folder WHERE service_id in :service_ids;", + "DELETE FROM templates WHERE service_id in :service_ids;", + "DELETE FROM templates_history WHERE service_id in :service_ids;", + "DELETE FROM service_letter_contacts WHERE service_id in :service_ids;", + "DELETE FROM unsubscribe_request WHERE service_id in :service_ids;", + "DELETE FROM unsubscribe_request_history WHERE service_id in :service_ids;", + "DELETE FROM unsubscribe_request_report WHERE service_id in :service_ids;", + "DELETE FROM user_to_service WHERE service_id in :service_ids;", + # now we can finally delete the services + "DELETE FROM services WHERE id in :service_ids;", + "DELETE FROM services_history WHERE id in :service_ids;", + ] + + for delete_statement in delete_statements: + conn.execute(text(delete_statement), service_ids=broadcast_service_ids) + + +def downgrade(): + # undowngradeable! + pass From cad920a45cfab47e1457bee09cbb768649759398 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 1 Oct 2024 15:50:34 +0100 Subject: [PATCH 2/9] dont delete service-specific things if theres no services --- .../versions/0465_delete_broadcast_data.py | 78 ++++++++++--------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/migrations/versions/0465_delete_broadcast_data.py b/migrations/versions/0465_delete_broadcast_data.py index 54e680d2ad..f4757e55c0 100644 --- a/migrations/versions/0465_delete_broadcast_data.py +++ b/migrations/versions/0465_delete_broadcast_data.py @@ -52,45 +52,49 @@ def upgrade(): "DELETE FROM service_broadcast_provider_restriction;", # all these tables have service_id, so need to be deleted before we can delete the service, # (note this includes templates as well, which we need to delete), - "DELETE FROM annual_billing WHERE service_id in :service_ids;", - "DELETE FROM api_keys WHERE service_id in :service_ids;", - "DELETE FROM complaints WHERE service_id in :service_ids;", - "DELETE FROM service_sms_senders WHERE service_id in :service_ids;", - "DELETE FROM inbound_numbers WHERE service_id in :service_ids;", - "DELETE FROM inbound_sms WHERE service_id in :service_ids;", - "DELETE FROM inbound_sms_history WHERE service_id in :service_ids;", - "DELETE FROM invited_users WHERE service_id in :service_ids;", - "DELETE FROM jobs WHERE service_id in :service_ids;", - "DELETE FROM notification_history WHERE service_id in :service_ids;", - "DELETE FROM notifications WHERE service_id in :service_ids;", - "DELETE FROM permissions WHERE service_id in :service_ids;", - "DELETE FROM returned_letters WHERE service_id in :service_ids;", - "DELETE FROM service_callback_api WHERE service_id in :service_ids;", - "DELETE FROM service_contact_list WHERE service_id in :service_ids;", - "DELETE FROM service_data_retention WHERE service_id in :service_ids;", - "DELETE FROM service_email_branding WHERE service_id in :service_ids;", - "DELETE FROM service_email_reply_to WHERE service_id in :service_ids;", - "DELETE FROM service_inbound_api WHERE service_id in :service_ids;", - "DELETE FROM service_join_requests WHERE service_id in :service_ids;", - "DELETE FROM service_letter_branding WHERE service_id in :service_ids;", - "DELETE FROM service_permissions WHERE service_id in :service_ids;", - "DELETE FROM service_whitelist WHERE service_id in :service_ids;", - "DELETE FROM user_folder_permissions WHERE service_id in :service_ids", - "DELETE FROM template_folder_map WHERE template_id in (SELECT id FROM templates WHERE service_id in :service_ids);", - "DELETE FROM template_redacted WHERE template_id in (SELECT id FROM templates WHERE service_id in :service_ids);", - "DELETE FROM template_folder WHERE service_id in :service_ids;", - "DELETE FROM templates WHERE service_id in :service_ids;", - "DELETE FROM templates_history WHERE service_id in :service_ids;", - "DELETE FROM service_letter_contacts WHERE service_id in :service_ids;", - "DELETE FROM unsubscribe_request WHERE service_id in :service_ids;", - "DELETE FROM unsubscribe_request_history WHERE service_id in :service_ids;", - "DELETE FROM unsubscribe_request_report WHERE service_id in :service_ids;", - "DELETE FROM user_to_service WHERE service_id in :service_ids;", - # now we can finally delete the services - "DELETE FROM services WHERE id in :service_ids;", - "DELETE FROM services_history WHERE id in :service_ids;", ] + if broadcast_service_ids: + delete_statements += [ + "DELETE FROM annual_billing WHERE service_id in :service_ids;", + "DELETE FROM api_keys WHERE service_id in :service_ids;", + "DELETE FROM complaints WHERE service_id in :service_ids;", + "DELETE FROM service_sms_senders WHERE service_id in :service_ids;", + "DELETE FROM inbound_numbers WHERE service_id in :service_ids;", + "DELETE FROM inbound_sms WHERE service_id in :service_ids;", + "DELETE FROM inbound_sms_history WHERE service_id in :service_ids;", + "DELETE FROM invited_users WHERE service_id in :service_ids;", + "DELETE FROM jobs WHERE service_id in :service_ids;", + "DELETE FROM notification_history WHERE service_id in :service_ids;", + "DELETE FROM notifications WHERE service_id in :service_ids;", + "DELETE FROM permissions WHERE service_id in :service_ids;", + "DELETE FROM returned_letters WHERE service_id in :service_ids;", + "DELETE FROM service_callback_api WHERE service_id in :service_ids;", + "DELETE FROM service_contact_list WHERE service_id in :service_ids;", + "DELETE FROM service_data_retention WHERE service_id in :service_ids;", + "DELETE FROM service_email_branding WHERE service_id in :service_ids;", + "DELETE FROM service_email_reply_to WHERE service_id in :service_ids;", + "DELETE FROM service_inbound_api WHERE service_id in :service_ids;", + "DELETE FROM service_join_requests WHERE service_id in :service_ids;", + "DELETE FROM service_letter_branding WHERE service_id in :service_ids;", + "DELETE FROM service_permissions WHERE service_id in :service_ids;", + "DELETE FROM service_whitelist WHERE service_id in :service_ids;", + "DELETE FROM user_folder_permissions WHERE service_id in :service_ids", + "DELETE FROM template_folder_map WHERE template_id in (SELECT id FROM templates WHERE service_id in :service_ids);", + "DELETE FROM template_redacted WHERE template_id in (SELECT id FROM templates WHERE service_id in :service_ids);", + "DELETE FROM template_folder WHERE service_id in :service_ids;", + "DELETE FROM templates WHERE service_id in :service_ids;", + "DELETE FROM templates_history WHERE service_id in :service_ids;", + "DELETE FROM service_letter_contacts WHERE service_id in :service_ids;", + "DELETE FROM unsubscribe_request WHERE service_id in :service_ids;", + "DELETE FROM unsubscribe_request_history WHERE service_id in :service_ids;", + "DELETE FROM unsubscribe_request_report WHERE service_id in :service_ids;", + "DELETE FROM user_to_service WHERE service_id in :service_ids;", + # now we can finally delete the services + "DELETE FROM services WHERE id in :service_ids;", + "DELETE FROM services_history WHERE id in :service_ids;", + ] + for delete_statement in delete_statements: conn.execute(text(delete_statement), service_ids=broadcast_service_ids) From 2df11c049a7da6a05a368d06a62cf23370e83111 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 12 Dec 2023 10:47:06 +0000 Subject: [PATCH 3/9] remove emergency alerts code no longer used. there might be additional cleanup we can do of other utils functions that dont have "alert" or "broadcast" in the name that are no longer used --- app/__init__.py | 4 - app/celery/nightly_tasks.py | 6 +- app/clients/cbc_proxy.py | 298 --------- app/commands.py | 31 - app/config.py | 26 - app/constants.py | 15 +- app/models.py | 368 +---------- app/schemas.py | 21 - .../service_broadcast_settings_schema.py | 12 - app/service_invite/rest.py | 7 +- app/template/rest.py | 4 +- app/utils.py | 12 +- app/xml_schemas/__init__.py | 19 - docs/writing-public-apis.md | 4 +- entrypoint.sh | 3 - migrations/versions/0323_broadcast_message.py | 2 - .../versions/0336_broadcast_msg_content_2.py | 2 - tests/app/clients/test_cbc_proxy.py | 607 ------------------ tests/app/db.py | 102 --- tests/app/service/test_rest.py | 18 +- tests/app/template/test_rest.py | 26 +- tests/app/test_config.py | 3 +- tests/app/v2/templates/test_get_templates.py | 4 +- .../v2/templates/test_templates_schemas.py | 2 +- tests/conftest.py | 3 - 25 files changed, 27 insertions(+), 1572 deletions(-) delete mode 100644 app/clients/cbc_proxy.py delete mode 100644 app/service/service_broadcast_settings_schema.py delete mode 100644 app/xml_schemas/__init__.py delete mode 100644 tests/app/clients/test_cbc_proxy.py diff --git a/app/__init__.py b/app/__init__.py index ca361bc0fa..2e9389b0de 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -33,7 +33,6 @@ from werkzeug.local import LocalProxy from app.clients import NotificationProviderClients -from app.clients.cbc_proxy import CBCProxyClient from app.clients.document_download import DocumentDownloadClient from app.clients.email.aws_ses import AwsSesClient from app.clients.email.aws_ses_stub import AwsSesStubClient @@ -49,7 +48,6 @@ zendesk_client = ZendeskClient() statsd_client = StatsdClient() redis_store = RedisClient() -cbc_proxy_client = CBCProxyClient() metrics = GDSMetrics() api_user = LocalProxy(lambda: g.api_user) @@ -178,8 +176,6 @@ def create_app(application): signing.init_app(application) redis_store.init_app(application) - cbc_proxy_client.init_app(application) - register_blueprint(application) register_v2_blueprints(application) diff --git a/app/celery/nightly_tasks.py b/app/celery/nightly_tasks.py index 22f250499b..54bdd0b15e 100644 --- a/app/celery/nightly_tasks.py +++ b/app/celery/nightly_tasks.py @@ -343,10 +343,8 @@ def delete_unneeded_notification_history_by_hour(): # We pass datetimes as args to the next task but celery will actually call `isoformat` on these # and send them over as strings [start_datetime, end_datetime], - # We use the broadcasts queue temporarily as it pulled from by a worker doing no work - # We don't want to put the tasks on the periodic queue because they make take up all - # the workers capacity, stopping other important tasks from happening - queue=QueueNames.BROADCASTS, + # We use the reporting queue as it's not used for most of the day + queue=QueueNames.REPORTING, ) current_app.logger.info( "Created delete_unneeded_notification_history_for_specific_hour task between %s and %s", diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py deleted file mode 100644 index 2f5c47bce7..0000000000 --- a/app/clients/cbc_proxy.py +++ /dev/null @@ -1,298 +0,0 @@ -import json -import uuid -from abc import ABC, abstractmethod - -import boto3 -import botocore -from flask import current_app -from notifications_utils.template import non_gsm_characters -from sqlalchemy.schema import Sequence - -from app.config import BroadcastProvider -from app.utils import DATETIME_FORMAT, format_sequential_number - -# The variable names in this file have specific meaning in a CAP message -# -# identifier is a unique field for each CAP message -# -# headline is a field which we are not sure if we will use -# -# description is the body of the message - -# areas is a list of dicts, with the following items -# * description is a string which populates the areaDesc field -# * polygon is a list of lat/long pairs -# -# previous_provider_messages is a list of previous events (models.py::BroadcastProviderMessage) -# ie a Cancel message would have a unique event but have the event of -# the preceeding Alert message in the previous_provider_messages field - - -class CBCProxyRetryableException(Exception): - pass - - -class CBCProxyClient: - _lambda_client = None - - def init_app(self, app): - if app.config.get("CBC_PROXY_ENABLED"): - self._lambda_client = boto3.client( - "lambda", - region_name="eu-west-2", - aws_access_key_id=app.config["CBC_PROXY_AWS_ACCESS_KEY_ID"], - aws_secret_access_key=app.config["CBC_PROXY_AWS_SECRET_ACCESS_KEY"], - ) - - def get_proxy(self, provider): - proxy_classes = { - BroadcastProvider.EE: CBCProxyEE, - BroadcastProvider.THREE: CBCProxyThree, - BroadcastProvider.O2: CBCProxyO2, - BroadcastProvider.VODAFONE: CBCProxyVodafone, - } - return proxy_classes[provider](self._lambda_client) - - -class CBCProxyClientBase(ABC): - @property - @abstractmethod - def lambda_name(self): - pass - - @property - @abstractmethod - def failover_lambda_name(self): - pass - - @property - @abstractmethod - def LANGUAGE_ENGLISH(self): - pass - - @property - @abstractmethod - def LANGUAGE_WELSH(self): - pass - - def __init__(self, lambda_client): - self._lambda_client = lambda_client - - def send_link_test(self): - self._send_link_test(self.lambda_name) - self._send_link_test(self.failover_lambda_name) - - @abstractmethod - def _send_link_test( - self, - lambda_name, - ): - pass - - @abstractmethod - def create_and_send_broadcast( - self, identifier, headline, description, areas, sent, expires, channel, message_number=None - ): - pass - - # We have not implemented updating a broadcast - def update_and_send_broadcast( # noqa: B027 - self, - identifier, - previous_provider_messages, - headline, - description, - areas, - sent, - expires, - channel, - message_number=None, - ): - pass - - @abstractmethod - def cancel_broadcast( - self, identifier, previous_provider_messages, headline, description, areas, sent, expires, message_number=None - ): - pass - - def _invoke_lambda_with_failover(self, payload): - result = self._invoke_lambda(self.lambda_name, payload) - - if not result: - failover_result = self._invoke_lambda(self.failover_lambda_name, payload) - if not failover_result: - raise CBCProxyRetryableException( - f"Lambda failed for both {self.lambda_name} and {self.failover_lambda_name}" - ) - - return result - - def _invoke_lambda(self, lambda_name, payload): - payload_bytes = bytes(json.dumps(payload), encoding="utf8") - try: - current_app.logger.info("Calling lambda %s with payload %s", lambda_name, str(payload)[:1000]) - - result = self._lambda_client.invoke( - FunctionName=lambda_name, - InvocationType="RequestResponse", - Payload=payload_bytes, - ) - except botocore.exceptions.ClientError: - current_app.logger.exception("Boto ClientError calling lambda %s", lambda_name) - success = False - return success - - if result["StatusCode"] > 299: - current_app.logger.info( - "Error calling lambda %s with status code %s, %s", - lambda_name, - result["StatusCode"], - result.get("Payload"), - ) - success = False - - elif "FunctionError" in result: - current_app.logger.info( - "Error calling lambda %s with function error %s", lambda_name, result["Payload"].read() - ) - success = False - - else: - success = True - - return success - - def infer_language_from(self, content): - if non_gsm_characters(content): - return self.LANGUAGE_WELSH - return self.LANGUAGE_ENGLISH - - -class CBCProxyOne2ManyClient(CBCProxyClientBase): - LANGUAGE_ENGLISH = "en-GB" - LANGUAGE_WELSH = "cy-GB" - - def _send_link_test( - self, - lambda_name, - ): - """ - link test - open up a connection to a specific provider, and send them an xml payload with a of - test. - """ - payload = {"message_type": "test", "identifier": str(uuid.uuid4()), "message_format": "cap"} - - self._invoke_lambda(lambda_name=lambda_name, payload=payload) - - def create_and_send_broadcast( - self, identifier, headline, description, areas, sent, expires, channel, message_number=None - ): - payload = { - "message_type": "alert", - "identifier": identifier, - "message_format": "cap", - "headline": headline, - "description": description, - "areas": areas, - "sent": sent, - "expires": expires, - "language": self.infer_language_from(description), - "channel": channel, - } - self._invoke_lambda_with_failover(payload=payload) - - def cancel_broadcast(self, identifier, previous_provider_messages, sent, message_number=None): - payload = { - "message_type": "cancel", - "identifier": identifier, - "message_format": "cap", - "references": [ - {"message_id": str(message.id), "sent": message.created_at.strftime(DATETIME_FORMAT)} - for message in previous_provider_messages - ], - "sent": sent, - } - self._invoke_lambda_with_failover(payload=payload) - - -class CBCProxyEE(CBCProxyOne2ManyClient): - lambda_name = "ee-1-proxy" - failover_lambda_name = "ee-2-proxy" - - -class CBCProxyThree(CBCProxyOne2ManyClient): - lambda_name = "three-1-proxy" - failover_lambda_name = "three-2-proxy" - - -class CBCProxyO2(CBCProxyOne2ManyClient): - lambda_name = "o2-1-proxy" - failover_lambda_name = "o2-2-proxy" - - -class CBCProxyVodafone(CBCProxyClientBase): - lambda_name = "vodafone-1-proxy" - failover_lambda_name = "vodafone-2-proxy" - - LANGUAGE_ENGLISH = "English" - LANGUAGE_WELSH = "Welsh" - - def _send_link_test( - self, - lambda_name, - ): - """ - link test - open up a connection to a specific provider, and send them an xml payload with a of - test. - """ - from app import db - - sequence = Sequence("broadcast_provider_message_number_seq") - sequential_number = db.session.connection().execute(sequence) - formatted_seq_number = format_sequential_number(sequential_number) - - payload = { - "message_type": "test", - "identifier": str(uuid.uuid4()), - "message_number": formatted_seq_number, - "message_format": "ibag", - } - - self._invoke_lambda(lambda_name=lambda_name, payload=payload) - - def create_and_send_broadcast( - self, identifier, message_number, headline, description, areas, sent, expires, channel - ): - payload = { - "message_type": "alert", - "identifier": identifier, - "message_number": message_number, - "message_format": "ibag", - "headline": headline, - "description": description, - "areas": areas, - "sent": sent, - "expires": expires, - "language": self.infer_language_from(description), - "channel": channel, - } - self._invoke_lambda_with_failover(payload=payload) - - def cancel_broadcast(self, identifier, previous_provider_messages, sent, message_number): - payload = { - "message_type": "cancel", - "identifier": identifier, - "message_number": message_number, - "message_format": "ibag", - "references": [ - { - "message_id": str(message.id), - "message_number": format_sequential_number(message.message_number), - "sent": message.created_at.strftime(DATETIME_FORMAT), - } - for message in previous_provider_messages - ], - "sent": sent, - } - self._invoke_lambda_with_failover(payload=payload) diff --git a/app/commands.py b/app/commands.py index 26616ceee5..a69506ff04 100644 --- a/app/commands.py +++ b/app/commands.py @@ -47,7 +47,6 @@ dao_get_organisation_by_email_address, dao_get_organisation_by_id, ) -from app.dao.permissions_dao import permission_dao from app.dao.services_dao import ( dao_create_service, dao_fetch_all_services_by_user, @@ -69,7 +68,6 @@ LetterBranding, Notification, Organisation, - Permission, Service, Template, User, @@ -810,35 +808,6 @@ def functional_test_fixtures(): raise SystemExit(1) -@click.option("-u", "--user-id", required=True) -@notify_command(name="local-dev-broadcast-permissions") -def local_dev_broadcast_permissions(user_id): - if os.getenv("NOTIFY_ENVIRONMENT", "") not in ["development", "test"]: - current_app.logger.error("Can only be run in development") - return - - user = User.query.filter_by(id=user_id).one() - - user_broadcast_services = Service.query.filter( - Service.permissions.any(permission="broadcast"), Service.users.any(id=user_id) - ) - - for service in user_broadcast_services: - permission_list = [ - Permission(service_id=service.id, user_id=user_id, permission=permission) - for permission in [ - "reject_broadcasts", - "cancel_broadcasts", # required to create / approve - "create_broadcasts", - "approve_broadcasts", # minimum for testing - "manage_templates", # unlikely but might be useful - "view_activity", # normally added on invite / service creation - ] - ] - - permission_dao.set_user_service_permission(user, service, permission_list, _commit=True, replace=True) - - @click.option("-u", "--user-id", required=True) @notify_command(name="generate-bulktest-data") def generate_bulktest_data(user_id): diff --git a/app/config.py b/app/config.py index 6fe61c9a4d..69d78a9163 100644 --- a/app/config.py +++ b/app/config.py @@ -25,8 +25,6 @@ class QueueNames: SMS_CALLBACKS = "sms-callbacks" ANTIVIRUS = "antivirus-tasks" SANITISE_LETTERS = "sanitise-letter-tasks" - BROADCASTS = "broadcast-tasks" - GOVUK_ALERTS = "govuk-alerts" @staticmethod def all_queues(): @@ -47,26 +45,15 @@ def all_queues(): QueueNames.LETTERS, QueueNames.SES_CALLBACKS, QueueNames.SMS_CALLBACKS, - QueueNames.BROADCASTS, ] -class BroadcastProvider: - EE = "ee" - VODAFONE = "vodafone" - THREE = "three" - O2 = "o2" - - PROVIDERS = [EE, VODAFONE, THREE, O2] - - class TaskNames: PROCESS_INCOMPLETE_JOBS = "process-incomplete-jobs" ZIP_AND_SEND_LETTER_PDFS = "zip-and-send-letter-pdfs" SCAN_FILE = "scan-file" SANITISE_LETTER = "sanitise-and-upload-letter" CREATE_PDF_FOR_TEMPLATED_LETTER = "create-pdf-for-templated-letter" - PUBLISH_GOVUK_ALERTS = "publish-govuk-alerts" RECREATE_PDF_FOR_PRECOMPILED_LETTER = "recreate-pdf-for-precompiled-letter" @@ -159,7 +146,6 @@ class Config: NOTIFY_SERVICE_ID = "d6aa2c68-a2d9-4437-ab19-3ae8eb202553" NOTIFY_USER_ID = "6af522d0-2915-4e52-83a3-3690455a5fe6" INVITATION_EMAIL_TEMPLATE_ID = "4f46df42-f795-4cc4-83bb-65ca312f49cc" - BROADCAST_INVITATION_EMAIL_TEMPLATE_ID = "46152f7c-6901-41d5-8590-a5624d0d4359" SMS_CODE_TEMPLATE_ID = "36fb0730-6259-4da1-8a80-c8de22ad4246" EMAIL_2FA_TEMPLATE_ID = "299726d2-dba6-42b8-8209-30e1d66ea164" NEW_USER_EMAIL_VERIFICATION_TEMPLATE_ID = "ece42649-22a8-4d06-b87f-d52d5d3f0a27" @@ -417,15 +403,6 @@ class Config: FIRETEXT_URL = os.environ.get("FIRETEXT_URL", "https://www.firetext.co.uk/api/sendsms/json") SES_STUB_URL = os.environ.get("SES_STUB_URL") - CBC_PROXY_ENABLED = True - CBC_PROXY_AWS_ACCESS_KEY_ID = os.environ.get("CBC_PROXY_AWS_ACCESS_KEY_ID", "") - CBC_PROXY_AWS_SECRET_ACCESS_KEY = os.environ.get("CBC_PROXY_AWS_SECRET_ACCESS_KEY", "") - - ENABLED_CBCS = {BroadcastProvider.EE, BroadcastProvider.THREE, BroadcastProvider.O2, BroadcastProvider.VODAFONE} - - # as defined in api db migration 0331_add_broadcast_org.py - BROADCAST_ORGANISATION_ID = "38e4bf69-93b0-445d-acee-53ea53fe02df" - DVLA_API_BASE_URL = os.environ.get("DVLA_API_BASE_URL", "https://uat.driver-vehicle-licensing.api.gov.uk") DVLA_API_TLS_CIPHERS = os.environ.get("DVLA_API_TLS_CIPHERS") @@ -506,8 +483,6 @@ class Development(Config): API_RATE_LIMIT_ENABLED = True DVLA_EMAIL_ADDRESSES = ["success@simulator.amazonses.com"] - CBC_PROXY_ENABLED = False - REGISTER_FUNCTIONAL_TESTING_BLUEPRINT = True FROM_NUMBER = "development" @@ -548,7 +523,6 @@ class Test(Development): MMG_URL = "https://example.com/mmg" FIRETEXT_URL = "https://example.com/firetext" - CBC_PROXY_ENABLED = True DVLA_EMAIL_ADDRESSES = ["success@simulator.amazonses.com", "success+2@simulator.amazonses.com"] DVLA_API_BASE_URL = "https://test-dvla-api.com" diff --git a/app/constants.py b/app/constants.py index 9ce6d21b16..ce16605164 100644 --- a/app/constants.py +++ b/app/constants.py @@ -140,9 +140,8 @@ SMS_TYPE = "sms" EMAIL_TYPE = "email" LETTER_TYPE = "letter" -BROADCAST_TYPE = "broadcast" -TEMPLATE_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, BROADCAST_TYPE] -NOTIFICATION_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] # not broadcast +TEMPLATE_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] +NOTIFICATION_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] NOTIFICATION_TYPE = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE] # duplicate that can probably be cleaned up @@ -186,10 +185,6 @@ class LetterLanguageOptions(str, enum.Enum): MANAGE_API_KEYS = "manage_api_keys" PLATFORM_ADMIN = "platform_admin" VIEW_ACTIVITY = "view_activity" -CREATE_BROADCASTS = "create_broadcasts" -APPROVE_BROADCASTS = "approve_broadcasts" -CANCEL_BROADCASTS = "cancel_broadcasts" -REJECT_BROADCASTS = "reject_broadcasts" INTERNATIONAL_SMS_TYPE = "international_sms" INBOUND_SMS_TYPE = "inbound_sms" SCHEDULE_NOTIFICATIONS = "schedule_notifications" @@ -207,7 +202,6 @@ class LetterLanguageOptions(str, enum.Enum): EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, - BROADCAST_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, SCHEDULE_NOTIFICATIONS, @@ -233,10 +227,6 @@ class LetterLanguageOptions(str, enum.Enum): MANAGE_API_KEYS, PLATFORM_ADMIN, VIEW_ACTIVITY, - CREATE_BROADCASTS, - APPROVE_BROADCASTS, - CANCEL_BROADCASTS, - REJECT_BROADCASTS, ] @@ -288,7 +278,6 @@ class OrganisationUserPermissionTypes(enum.Enum): SMS_PROVIDERS = [MMG_PROVIDER, FIRETEXT_PROVIDER] EMAIL_PROVIDERS = [SES_PROVIDER] PROVIDERS = SMS_PROVIDERS + EMAIL_PROVIDERS -ALL_BROADCAST_PROVIDERS = "all" # Jobs JOB_STATUS_PENDING = "pending" diff --git a/app/models.py b/app/models.py index 11597a95b6..6d698b9ecb 100644 --- a/app/models.py +++ b/app/models.py @@ -34,13 +34,10 @@ from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm.collections import attribute_mapped_collection -from sqlalchemy.schema import Sequence from app import db, redis_store, signing from app.constants import ( - ALL_BROADCAST_PROVIDERS, BRANDING_ORG, - BROADCAST_TYPE, EMAIL_TYPE, GUEST_LIST_RECIPIENT_TYPE, INVITE_PENDING, @@ -137,11 +134,7 @@ def can_use_webauthn(self): if self.auth_type == "webauthn_auth": return True - return any( - str(service.organisation_id) == current_app.config["BROADCAST_ORGANISATION_ID"] - or str(service.id) == current_app.config["NOTIFY_SERVICE_ID"] - for service in self.services - ) + return any(str(service.id) == current_app.config["NOTIFY_SERVICE_ID"] for service in self.services) @password.setter def password(self, password): @@ -578,9 +571,6 @@ class Service(db.Model, Versioned): backref=db.backref("services", lazy="dynamic"), ) - allowed_broadcast_provider = association_proxy("service_broadcast_settings", "provider") - broadcast_channel = association_proxy("service_broadcast_settings", "channel") - @hybrid_property # a hybrid_property enables us to still use it in queries def name(self): return self._name @@ -660,13 +650,6 @@ def serialize_for_org_dashboard(self): "restricted": self.restricted, } - def get_available_broadcast_providers(self): - # There may be future checks here if we add, for example, platform admin level provider killswitches. - if self.allowed_broadcast_provider != ALL_BROADCAST_PROVIDERS: - return [x for x in current_app.config["ENABLED_CBCS"] if x == self.allowed_broadcast_provider] - else: - return current_app.config["ENABLED_CBCS"] - class DefaultAnnualAllowance(db.Model): """This table represents default allowances that organisations will get for free notifications. @@ -1038,7 +1021,6 @@ def __init__(self, **kwargs): hidden = db.Column(db.Boolean, nullable=False, default=False) subject = db.Column(db.Text) postage = db.Column(db.String, nullable=True) - broadcast_data = db.Column(JSONB(none_as_null=True), nullable=True) letter_welsh_content = db.Column(db.Text) letter_welsh_subject = db.Column(db.Text) @@ -1145,8 +1127,6 @@ def _as_utils_template(self): return PlainTextEmailTemplate(self.__dict__) if self.template_type == SMS_TYPE: return SMSMessageTemplate(self.__dict__) - if self.template_type == BROADCAST_TYPE: - return SMSMessageTemplate(self.__dict__ | {"template_type": "sms"}) if self.template_type == LETTER_TYPE: return LetterPrintTemplate( self.__dict__, @@ -2288,352 +2268,6 @@ def serialize(self): return contact_list -class BroadcastStatusType(db.Model): - __tablename__ = "broadcast_status_type" - DRAFT = "draft" - PENDING_APPROVAL = "pending-approval" - REJECTED = "rejected" - BROADCASTING = "broadcasting" - COMPLETED = "completed" - CANCELLED = "cancelled" - TECHNICAL_FAILURE = "technical-failure" - - STATUSES = [DRAFT, PENDING_APPROVAL, REJECTED, BROADCASTING, COMPLETED, CANCELLED, TECHNICAL_FAILURE] - - # a broadcast message can be edited while in one of these states - PRE_BROADCAST_STATUSES = [DRAFT, PENDING_APPROVAL, REJECTED] - LIVE_STATUSES = [BROADCASTING, COMPLETED, CANCELLED] - - # these are only the transitions we expect to administer via the API code. - ALLOWED_STATUS_TRANSITIONS = { - DRAFT: {PENDING_APPROVAL}, - PENDING_APPROVAL: {REJECTED, DRAFT, BROADCASTING}, - REJECTED: {DRAFT, PENDING_APPROVAL}, - BROADCASTING: {COMPLETED, CANCELLED}, - COMPLETED: {}, - CANCELLED: {}, - TECHNICAL_FAILURE: {}, - } - - name = db.Column(db.String, primary_key=True) - - -class BroadcastMessage(db.Model): - """ - This is for creating a message, viewing it in notify, adding areas, approvals, drafts, etc. Notify logic before - hitting send. - """ - - __tablename__ = "broadcast_message" - __table_args__ = ( - db.ForeignKeyConstraint( - ["template_id", "template_version"], - ["templates_history.id", "templates_history.version"], - ), - CheckConstraint("created_by_id is not null or created_by_api_key_id is not null"), - {}, - ) - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey("services.id")) - service = db.relationship("Service", backref="broadcast_messages") - - template_id = db.Column(UUID(as_uuid=True), nullable=True) - template_version = db.Column(db.Integer, nullable=True) - template = db.relationship("TemplateHistory", backref="broadcast_messages") - - _personalisation = db.Column(db.String, nullable=True) - content = db.Column(db.Text, nullable=False) - # defaults to empty list - areas = db.Column(JSONB(none_as_null=True), nullable=False, default=list) - - status = db.Column( - db.String, db.ForeignKey("broadcast_status_type.name"), nullable=False, default=BroadcastStatusType.DRAFT - ) - - # these times are related to the actual broadcast, rather than auditing purposes - starts_at = db.Column(db.DateTime, nullable=True) - finishes_at = db.Column(db.DateTime, nullable=True) # isn't updated if user cancels - - # these times correspond to when - created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) - approved_at = db.Column(db.DateTime, nullable=True) - cancelled_at = db.Column(db.DateTime, nullable=True) - updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - - created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey("users.id"), nullable=True) - approved_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey("users.id"), nullable=True) - cancelled_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey("users.id"), nullable=True) - - created_by = db.relationship("User", foreign_keys=[created_by_id]) - approved_by = db.relationship("User", foreign_keys=[approved_by_id]) - cancelled_by = db.relationship("User", foreign_keys=[cancelled_by_id]) - - created_by_api_key_id = db.Column(UUID(as_uuid=True), db.ForeignKey("api_keys.id"), nullable=True) - cancelled_by_api_key_id = db.Column(UUID(as_uuid=True), db.ForeignKey("api_keys.id"), nullable=True) - created_by_api_key = db.relationship("ApiKey", foreign_keys=[created_by_api_key_id]) - cancelled_by_api_key = db.relationship("ApiKey", foreign_keys=[cancelled_by_api_key_id]) - - reference = db.Column(db.String(255), nullable=True) - cap_event = db.Column(db.String(255), nullable=True) - - stubbed = db.Column(db.Boolean, nullable=False) - - @property - def personalisation(self): - if self._personalisation: - return signing.decode(self._personalisation) - return {} - - @personalisation.setter - def personalisation(self, personalisation): - self._personalisation = signing.encode(personalisation or {}) - - def serialize(self): - return { - "id": str(self.id), - "reference": self.reference, - "cap_event": self.cap_event, - "service_id": str(self.service_id), - "template_id": str(self.template_id) if self.template else None, - "template_version": self.template_version, - "template_name": self.template.name if self.template else None, - "personalisation": self.personalisation if self.template else None, - "content": self.content, - "areas": self.areas, - "status": self.status, - "starts_at": get_dt_string_or_none(self.starts_at), - "finishes_at": get_dt_string_or_none(self.finishes_at), - "created_at": get_dt_string_or_none(self.created_at), - "approved_at": get_dt_string_or_none(self.approved_at), - "cancelled_at": get_dt_string_or_none(self.cancelled_at), - "updated_at": get_dt_string_or_none(self.updated_at), - "created_by_id": get_uuid_string_or_none(self.created_by_id), - "approved_by_id": get_uuid_string_or_none(self.approved_by_id), - "cancelled_by_id": get_uuid_string_or_none(self.cancelled_by_id), - } - - -class BroadcastEventMessageType: - ALERT = "alert" - UPDATE = "update" - CANCEL = "cancel" - - MESSAGE_TYPES = [ALERT, UPDATE, CANCEL] - - -class BroadcastEvent(db.Model): - """ - This table represents an instruction that we will send to the broadcast providers. It directly correlates with an - instruction from the admin - to broadcast a message, to cancel an existing message, or to update an existing one. - - We should be able to create the complete CAP message without joining from this to any other tables, eg - template, service, or broadcast_message. - - The only exception to this is that we will have to join to itself to find other broadcast_events with the - same broadcast_message_id when building up the `` xml field for updating/cancelling an existing message. - - As such, this shouldn't have foreign keys to things that can change or be deleted. - """ - - __tablename__ = "broadcast_event" - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey("services.id")) - service = db.relationship("Service") - - broadcast_message_id = db.Column(UUID(as_uuid=True), db.ForeignKey("broadcast_message.id"), nullable=False) - broadcast_message = db.relationship("BroadcastMessage", backref="events") - - # this is used for in the cap xml - sent_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) - - # msgType. alert, cancel, or update. (other options in the spec are "ack" and "error") - message_type = db.Column(db.String, nullable=False) - - # this will be json containing anything that isnt hardcoded in utils/cbc proxy. for now just body but may grow to - # include, eg, title, headline, instructions. - transmitted_content = db.Column(JSONB(none_as_null=True), nullable=True) - # unsubstantiated reckon: even if we're sending a cancel, we'll still need to provide areas - transmitted_areas = db.Column(JSONB(none_as_null=True), nullable=False, default=list) - transmitted_sender = db.Column(db.String(), nullable=False) - - # we may only need this starts_at if this is scheduled for the future. Interested to see how this affects - # updates/cancels (ie: can you schedule an update for the future?) - transmitted_starts_at = db.Column(db.DateTime, nullable=True) - transmitted_finishes_at = db.Column(db.DateTime, nullable=True) - - @property - def reference(self): - notify_email_domain = current_app.config["NOTIFY_EMAIL_DOMAIN"] - return f"https://www.{notify_email_domain}/,{self.id},{self.sent_at_as_cap_datetime_string}" - - @property - def sent_at_as_cap_datetime_string(self): - return self.formatted_datetime_for("sent_at") - - @property - def transmitted_finishes_at_as_cap_datetime_string(self): - return self.formatted_datetime_for("transmitted_finishes_at") - - def formatted_datetime_for(self, property_name): - return self.convert_naive_utc_datetime_to_cap_standard_string(getattr(self, property_name)) - - @staticmethod - def convert_naive_utc_datetime_to_cap_standard_string(dt): - """ - As defined in section 3.3.2 of - http://docs.oasis-open.org/emergency/cap/v1.2/CAP-v1.2-os.html - They define the standard "YYYY-MM-DDThh:mm:ssXzh:zm", where X is - `+` if the timezone is > UTC, otherwise `-` - """ - return f"{dt.strftime('%Y-%m-%dT%H:%M:%S')}-00:00" - - def get_provider_message(self, provider): - return next( - (provider_message for provider_message in self.provider_messages if provider_message.provider == provider), - None, - ) - - def serialize(self): - return { - "id": str(self.id), - "service_id": str(self.service_id), - "broadcast_message_id": str(self.broadcast_message_id), - # sent_at is required by BroadcastMessageTemplate.from_broadcast_event - "sent_at": self.sent_at.strftime(DATETIME_FORMAT), - "message_type": self.message_type, - "transmitted_content": self.transmitted_content, - "transmitted_areas": self.transmitted_areas, - "transmitted_sender": self.transmitted_sender, - "transmitted_starts_at": get_dt_string_or_none(self.transmitted_starts_at), - # transmitted_finishes_at is required by BroadcastMessageTemplate.from_broadcast_event - "transmitted_finishes_at": self.transmitted_finishes_at.strftime(DATETIME_FORMAT), - } - - -class BroadcastProvider: - EE = "ee" - VODAFONE = "vodafone" - THREE = "three" - O2 = "o2" - - PROVIDERS = [EE, VODAFONE, THREE, O2] - - -class BroadcastProviderMessageStatus: - TECHNICAL_FAILURE = "technical-failure" # Couldn’t send (cbc proxy 5xx/4xx) - SENDING = "sending" # Sent to cbc, awaiting response - ACK = "returned-ack" # Received ack response - ERR = "returned-error" # Received error response - - STATES = [TECHNICAL_FAILURE, SENDING, ACK, ERR] - - -class BroadcastProviderMessage(db.Model): - """ - A row in this table represents the XML blob sent to a single provider. - """ - - __tablename__ = "broadcast_provider_message" - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - - broadcast_event_id = db.Column(UUID(as_uuid=True), db.ForeignKey("broadcast_event.id")) - broadcast_event = db.relationship("BroadcastEvent", backref="provider_messages") - - # 'ee', 'three', 'vodafone', etc - provider = db.Column(db.String) - - status = db.Column(db.String) - - created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) - updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - - UniqueConstraint(broadcast_event_id, provider) - - message_number = association_proxy("broadcast_provider_message_number", "broadcast_provider_message_number") - - -class BroadcastProviderMessageNumber(db.Model): - """ - To send IBAG messages via the CBC proxy to Nokia CBC appliances, Notify must generate and store a numeric - message_number alongside the message ID (GUID). - Subsequent messages (Update, Cancel) in IBAG format must reference the original message_number & message_id. - This model relates broadcast_provider_message_id to that numeric message_number. - """ - - __tablename__ = "broadcast_provider_message_number" - - sequence = Sequence("broadcast_provider_message_number_seq") - broadcast_provider_message_number = db.Column( - db.Integer, sequence, server_default=sequence.next_value(), primary_key=True - ) - broadcast_provider_message_id = db.Column( - UUID(as_uuid=True), db.ForeignKey("broadcast_provider_message.id"), nullable=False - ) - broadcast_provider_message = db.relationship( - "BroadcastProviderMessage", backref=db.backref("broadcast_provider_message_number", uselist=False) - ) - - -class ServiceBroadcastSettings(db.Model): - """ - Every broadcast service should have one and only one row in this table. - """ - - __tablename__ = "service_broadcast_settings" - - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey("services.id"), primary_key=True, nullable=False) - service = db.relationship(Service, backref=db.backref("service_broadcast_settings", uselist=False)) - channel = db.Column(db.String(255), db.ForeignKey("broadcast_channel_types.name"), nullable=False) - provider = db.Column(db.String, db.ForeignKey("broadcast_provider_types.name"), nullable=False) - created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) - updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - - -class BroadcastChannelTypes(db.Model): - __tablename__ = "broadcast_channel_types" - - name = db.Column(db.String(255), primary_key=True) - - -class BroadcastProviderTypes(db.Model): - __tablename__ = "broadcast_provider_types" - - name = db.Column(db.String(255), primary_key=True) - - -class BroadcastProviderMessageStatusType(db.Model): - __tablename__ = "broadcast_provider_message_status_type" - - name = db.Column(db.String(), primary_key=True) - - -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. - - This table links those services. There should only be one row per service in this table, and this is enforced by - the service_id being a primary key. - """ - - __tablename__ = "service_broadcast_provider_restriction" - - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey("services.id"), primary_key=True, nullable=False) - service = db.relationship(Service, backref=db.backref("service_broadcast_provider_restriction", uselist=False)) - - provider = db.Column(db.String, nullable=False) - - created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) - - class WebauthnCredential(db.Model): """ A table that stores data for registered webauthn credentials. diff --git a/app/schemas.py b/app/schemas.py index 4d367d6873..534caa06dd 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -220,8 +220,6 @@ class ServiceSchema(BaseSchema, UUIDsAsStringsMixin): sms_message_limit = field_for(models.Service, "sms_message_limit", required=True) letter_message_limit = field_for(models.Service, "letter_message_limit", required=True) go_live_at = field_for(models.Service, "go_live_at", format=DATETIME_FORMAT_NO_TIMEZONE) - allowed_broadcast_provider = fields.Method(dump_only=True, serialize="_get_allowed_broadcast_provider") - broadcast_channel = fields.Method(dump_only=True, serialize="_get_broadcast_channel") name = fields.String(required=True) custom_email_sender_name = fields.String(allow_none=True) # this can only be set via custom_email_sender_name or name @@ -235,12 +233,6 @@ def service_delivery_status_callback_api(self, service): if callback.callback_type == app.constants.DELIVERY_STATUS_CALLBACK_TYPE ] - def _get_allowed_broadcast_provider(self, service): - return service.allowed_broadcast_provider - - def _get_broadcast_channel(self, service): - return service.broadcast_channel - def get_letter_logo_filename(self, service): return service.letter_branding and service.letter_branding.filename @@ -268,7 +260,6 @@ class Meta(BaseSchema.Meta): "all_template_folders", "annual_billing", "api_keys", - "broadcast_messages", "complaints", "contact_list", "created_at", @@ -282,8 +273,6 @@ class Meta(BaseSchema.Meta): "letter_logo_filename", "reply_to_email_addresses", "returned_letters", - "service_broadcast_provider_restriction", - "service_broadcast_settings", "service_sms_senders", "templates", "updated_at", @@ -346,7 +335,6 @@ class Meta(BaseSchema.Meta): "all_template_folders", "annual_billing", "api_keys", - "broadcast_messages", "contact_list", "created_by", "crown", @@ -443,7 +431,6 @@ class TemplateSchemaNoDetail(TemplateSchema): class Meta(TemplateSchema.Meta): exclude = TemplateSchema.Meta.exclude + ( "archived", - "broadcast_data", "created_at", "created_by", "created_by_id", @@ -466,13 +453,6 @@ class Meta(TemplateSchema.Meta): "letter_languages", ) - @pre_dump - def remove_content_for_non_broadcast_templates(self, template, **kwargs): - if template.template_type != app.constants.BROADCAST_TYPE: - template.content = None - - return template - class TemplateHistorySchema(BaseSchema): reply_to = fields.Method("get_reply_to", allow_none=True) @@ -499,7 +479,6 @@ def get_is_precompiled_letter(self, template): class Meta(BaseSchema.Meta): model = models.TemplateHistory - exclude = ("broadcast_messages",) class ApiKeySchema(BaseSchema): diff --git a/app/service/service_broadcast_settings_schema.py b/app/service/service_broadcast_settings_schema.py deleted file mode 100644 index 0a333c2f28..0000000000 --- a/app/service/service_broadcast_settings_schema.py +++ /dev/null @@ -1,12 +0,0 @@ -service_broadcast_settings_schema = { - "$schema": "http://json-schema.org/draft-07/schema#", - "description": "Set a services broadcast settings", - "type": "object", - "title": "Set a services broadcast settings", - "properties": { - "broadcast_channel": {"enum": ["operator", "test", "severe", "government"]}, - "service_mode": {"enum": ["training", "live"]}, - "provider_restriction": {"enum": ["three", "o2", "vodafone", "ee", "all"]}, - }, - "required": ["broadcast_channel", "service_mode", "provider_restriction"], -} diff --git a/app/service_invite/rest.py b/app/service_invite/rest.py index ba08f9e1f2..ed14f63ff9 100644 --- a/app/service_invite/rest.py +++ b/app/service_invite/rest.py @@ -3,7 +3,7 @@ from notifications_utils.url_safe_token import check_token, generate_token from app.config import QueueNames -from app.constants import BROADCAST_TYPE, EMAIL_TYPE, KEY_TYPE_NORMAL +from app.constants import EMAIL_TYPE, KEY_TYPE_NORMAL from app.dao.invited_user_dao import ( get_invited_user_by_id, get_invited_user_by_service_and_id, @@ -34,10 +34,7 @@ def create_invited_user(service_id): invited_user = invited_user_schema.load(request_json) save_invited_user(invited_user) - if invited_user.service.has_permission(BROADCAST_TYPE): - template_id = current_app.config["BROADCAST_INVITATION_EMAIL_TEMPLATE_ID"] - else: - template_id = current_app.config["INVITATION_EMAIL_TEMPLATE_ID"] + template_id = current_app.config["INVITATION_EMAIL_TEMPLATE_ID"] template = dao_get_template_by_id(template_id) service = Service.query.get(current_app.config["NOTIFY_SERVICE_ID"]) diff --git a/app/template/rest.py b/app/template/rest.py index b3405ffe34..26a84e715a 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -14,7 +14,7 @@ from requests import post as requests_post from sqlalchemy.orm.exc import NoResultFound -from app.constants import BROADCAST_TYPE, LETTER_TYPE, QR_CODE_TOO_LONG, SECOND_CLASS, SMS_TYPE +from app.constants import LETTER_TYPE, QR_CODE_TOO_LONG, SECOND_CLASS, SMS_TYPE from app.dao.notifications_dao import get_notification_by_id from app.dao.services_dao import dao_fetch_service_by_id from app.dao.template_folder_dao import ( @@ -52,7 +52,7 @@ def _content_count_greater_than_limit(content, template_type): - if template_type in {SMS_TYPE, BROADCAST_TYPE}: + if template_type == SMS_TYPE: template = SMSMessageTemplate({"content": content, "template_type": SMS_TYPE}) return template.is_message_too_long() return False diff --git a/app/utils.py b/app/utils.py index e651e22394..f4a703b106 100644 --- a/app/utils.py +++ b/app/utils.py @@ -13,7 +13,6 @@ from sqlalchemy import func from app.constants import ( - BROADCAST_TYPE, EMAIL_TYPE, LETTER_TYPE, PRECOMPILED_LETTER, @@ -60,12 +59,9 @@ def url_with_token(data, url, base_url=None): def get_template_instance(template, values): - return { - SMS_TYPE: SMSMessageTemplate, - EMAIL_TYPE: HTMLEmailTemplate, - LETTER_TYPE: LetterPrintTemplate, - BROADCAST_TYPE: SMSMessageTemplate, - }[template["template_type"]](template, values) + return {SMS_TYPE: SMSMessageTemplate, EMAIL_TYPE: HTMLEmailTemplate, LETTER_TYPE: LetterPrintTemplate}[ + template["template_type"] + ](template, values) def get_london_midnight_in_utc(date): @@ -104,8 +100,6 @@ def get_public_notify_type_text(notify_type, plural=False): notify_type_text = "document" elif notify_type == PRECOMPILED_LETTER: notify_type_text = "precompiled letter" - elif notify_type == BROADCAST_TYPE: - notify_type_text = "broadcast message" return "{}{}".format(notify_type_text, "s" if plural else "") diff --git a/app/xml_schemas/__init__.py b/app/xml_schemas/__init__.py deleted file mode 100644 index 1ba15cb978..0000000000 --- a/app/xml_schemas/__init__.py +++ /dev/null @@ -1,19 +0,0 @@ -from pathlib import Path - -from lxml import etree - - -def validate_xml(document, schema_file_name): - path = Path(__file__).resolve().parent / schema_file_name - contents = path.read_text() - - schema_root = etree.XML(contents.encode("utf-8")) - schema = etree.XMLSchema(schema_root) - parser = etree.XMLParser(schema=schema) - - try: - etree.fromstring(document, parser) - except etree.XMLSyntaxError: - return False - - return True diff --git a/docs/writing-public-apis.md b/docs/writing-public-apis.md index 3bca466834..53413a7618 100644 --- a/docs/writing-public-apis.md +++ b/docs/writing-public-apis.md @@ -45,6 +45,6 @@ Each adapter should be documented in each client ([example](https://github.com/a This is done as part of registering the blueprint in `app/__init__.py` e.g. ``` -post_broadcast.before_request(requires_auth) -application.register_blueprint(post_broadcast) +v2_notification_blueprint.before_request(requires_auth) +application.register_blueprint(v2_notification_blueprint) ``` diff --git a/entrypoint.sh b/entrypoint.sh index 7059dfc358..52b5023492 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -50,9 +50,6 @@ case "$1" in # Only consume the notify-internal-tasks queue on this app so that Notify messages are processed as a priority exec $COMMON_CMD notify-internal-tasks ;; - api-worker-broadcasts) - exec $COMMON_CMD broadcast-tasks - ;; api-worker-receipts) exec $COMMON_CMD ses-callbacks,sms-callbacks ;; diff --git a/migrations/versions/0323_broadcast_message.py b/migrations/versions/0323_broadcast_message.py index 3707914e12..d9bd8dbc01 100644 --- a/migrations/versions/0323_broadcast_message.py +++ b/migrations/versions/0323_broadcast_message.py @@ -11,8 +11,6 @@ from sqlalchemy.dialects import postgresql from sqlalchemy.sql import column, func -from app.models import BroadcastMessage - revision = "0323_broadcast_message" down_revision = "0322_broadcast_service_perm" diff --git a/migrations/versions/0336_broadcast_msg_content_2.py b/migrations/versions/0336_broadcast_msg_content_2.py index c5dd9dd4d9..ba5864ca7b 100644 --- a/migrations/versions/0336_broadcast_msg_content_2.py +++ b/migrations/versions/0336_broadcast_msg_content_2.py @@ -11,8 +11,6 @@ from sqlalchemy.dialects import postgresql from sqlalchemy.orm.session import Session -from app.models import BroadcastMessage - revision = "0336_broadcast_msg_content_2" down_revision = "0335_broadcast_msg_content" diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py deleted file mode 100644 index 40aa811e24..0000000000 --- a/tests/app/clients/test_cbc_proxy.py +++ /dev/null @@ -1,607 +0,0 @@ -import json -import uuid -from collections import namedtuple -from datetime import datetime -from io import BytesIO -from unittest.mock import Mock, call - -import pytest -from botocore.exceptions import ClientError as BotoClientError - -from app import db -from app.clients.cbc_proxy import ( - CBCProxyClient, - CBCProxyEE, - CBCProxyO2, - CBCProxyRetryableException, - CBCProxyThree, - CBCProxyVodafone, -) -from app.utils import DATETIME_FORMAT - -EXAMPLE_AREAS = [ - { - "description": "london", - "polygon": [ - [51.12, -1.2], - [51.12, 1.2], - [51.74, 1.2], - [51.74, -1.2], - [51.12, -1.2], - ], - } -] - - -@pytest.fixture(scope="function") -def cbc_proxy_client(client, mocker): - client = CBCProxyClient() - current_app = mocker.Mock( - config={ - "CBC_PROXY_AWS_ACCESS_KEY_ID": "cbc-proxy-aws-access-key-id", - "CBC_PROXY_AWS_SECRET_ACCESS_KEY": "cbc-proxy-aws-secret-access-key", - "CBC_PROXY_ENABLED": True, - } - ) - client.init_app(current_app) - return client - - -@pytest.fixture -def cbc_proxy_ee(cbc_proxy_client): - return cbc_proxy_client.get_proxy("ee") - - -@pytest.fixture -def cbc_proxy_vodafone(cbc_proxy_client): - return cbc_proxy_client.get_proxy("vodafone") - - -@pytest.mark.parametrize( - "provider_name, expected_provider_class", - [ - ("ee", CBCProxyEE), - ("three", CBCProxyThree), - ("o2", CBCProxyO2), - ("vodafone", CBCProxyVodafone), - ], -) -def test_cbc_proxy_client_returns_correct_client(provider_name, expected_provider_class): - mock_lambda = Mock() - cbc_proxy_client = CBCProxyClient() - cbc_proxy_client._lambda_client = mock_lambda - - ret = cbc_proxy_client.get_proxy(provider_name) - - assert type(ret) == expected_provider_class - assert ret._lambda_client == mock_lambda - - -def test_cbc_proxy_lambda_client_has_correct_region(cbc_proxy_ee): - assert cbc_proxy_ee._lambda_client._client_config.region_name == "eu-west-2" - - -def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy_ee): - key = cbc_proxy_ee._lambda_client._request_signer._credentials.access_key - secret = cbc_proxy_ee._lambda_client._request_signer._credentials.secret_key - - assert key == "cbc-proxy-aws-access-key-id" - assert secret == "cbc-proxy-aws-secret-access-key" - - -def test_cbc_proxy_send_link_test(mocker, cbc_proxy_ee): - mock_send_link_test = mocker.patch.object(cbc_proxy_ee, "_send_link_test") - cbc_proxy_ee.send_link_test() - - mock_send_link_test.assert_any_call(cbc_proxy_ee.lambda_name) - mock_send_link_test.assert_any_call(cbc_proxy_ee.failover_lambda_name) - - -@pytest.mark.parametrize( - "description, expected_language", - ( - ("my-description", "en-GB"), - ("mŷ-description", "cy-GB"), - ), -) -@pytest.mark.parametrize("cbc", ["ee", "three", "o2"]) -def test_cbc_proxy_one_2_many_create_and_send_invokes_function( - mocker, - cbc_proxy_client, - description, - cbc, - expected_language, -): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - identifier = "my-identifier" - headline = "my-headline" - - sent = "a-passed-through-sent-value" - expires = "a-passed-through-expires-value" - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - } - - cbc_proxy.create_and_send_broadcast( - identifier=identifier, - message_number="0000007b", - headline=headline, - description=description, - areas=EXAMPLE_AREAS, - sent=sent, - expires=expires, - channel="severe", - ) - - ld_client_mock.invoke.assert_called_once_with( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ) - - kwargs = ld_client_mock.invoke.mock_calls[0][-1] - payload_bytes = kwargs["Payload"] - payload = json.loads(payload_bytes) - - assert payload["identifier"] == identifier - assert "message_number" not in payload - assert payload["message_format"] == "cap" - assert payload["message_type"] == "alert" - assert payload["headline"] == headline - assert payload["description"] == description - assert payload["areas"] == EXAMPLE_AREAS - assert payload["sent"] == sent - assert payload["expires"] == expires - assert payload["language"] == expected_language - assert payload["channel"] == "severe" - - -@pytest.mark.parametrize("cbc", ["ee", "three", "o2"]) -def test_cbc_proxy_one_2_many_cancel_invokes_function(mocker, cbc_proxy_client, cbc): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - identifier = "my-identifier" - MockProviderMessage = namedtuple("BroadcastProviderMessage", ["id", "message_number", "created_at"]) - - provider_messages = [ - MockProviderMessage(uuid.uuid4(), "0000007b", datetime(2020, 12, 16)), - MockProviderMessage(uuid.uuid4(), "0000004e", datetime(2020, 12, 17)), - ] - sent = "2020-12-17 14:19:44.130585" - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - } - - cbc_proxy.cancel_broadcast( - identifier=identifier, message_number="00000050", previous_provider_messages=provider_messages, sent=sent - ) - - ld_client_mock.invoke.assert_called_once_with( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ) - - kwargs = ld_client_mock.invoke.mock_calls[0][-1] - payload_bytes = kwargs["Payload"] - payload = json.loads(payload_bytes) - - assert payload["identifier"] == identifier - assert "message_number" not in payload - assert payload["message_format"] == "cap" - assert payload["message_type"] == "cancel" - assert payload["references"] == [ - {"message_id": str(provider_messages[0].id), "sent": provider_messages[0].created_at.strftime(DATETIME_FORMAT)}, - {"message_id": str(provider_messages[1].id), "sent": provider_messages[1].created_at.strftime(DATETIME_FORMAT)}, - ] - assert payload["sent"] == sent - - -@pytest.mark.parametrize( - "description, expected_language", - ( - ("my-description", "English"), - ("mŷ-description", "Welsh"), - ), -) -def test_cbc_proxy_vodafone_create_and_send_invokes_function( - mocker, - cbc_proxy_vodafone, - description, - expected_language, -): - identifier = "my-identifier" - headline = "my-headline" - - sent = "a-passed-through-sent-value" - expires = "a-passed-through-expires-value" - - ld_client_mock = mocker.patch.object( - cbc_proxy_vodafone, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - } - - cbc_proxy_vodafone.create_and_send_broadcast( - identifier=identifier, - message_number="0000007b", - headline=headline, - description=description, - areas=EXAMPLE_AREAS, - sent=sent, - expires=expires, - channel="test", - ) - - ld_client_mock.invoke.assert_called_once_with( - FunctionName="vodafone-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ) - - kwargs = ld_client_mock.invoke.mock_calls[0][-1] - payload_bytes = kwargs["Payload"] - payload = json.loads(payload_bytes) - - assert payload["identifier"] == identifier - assert payload["message_number"] == "0000007b" - assert payload["message_format"] == "ibag" - assert payload["message_type"] == "alert" - assert payload["headline"] == headline - assert payload["description"] == description - assert payload["areas"] == EXAMPLE_AREAS - assert payload["sent"] == sent - assert payload["expires"] == expires - assert payload["language"] == expected_language - assert payload["channel"] == "test" - - -def test_cbc_proxy_vodafone_cancel_invokes_function(mocker, cbc_proxy_vodafone): - identifier = "my-identifier" - MockProviderMessage = namedtuple("BroadcastProviderMessage", ["id", "message_number", "created_at"]) - - provider_messages = [ - MockProviderMessage(uuid.uuid4(), 78, datetime(2020, 12, 16)), - MockProviderMessage(uuid.uuid4(), 123, datetime(2020, 12, 17)), - ] - sent = "2020-12-18 14:19:44.130585" - - ld_client_mock = mocker.patch.object( - cbc_proxy_vodafone, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - } - - cbc_proxy_vodafone.cancel_broadcast( - identifier=identifier, message_number="00000050", previous_provider_messages=provider_messages, sent=sent - ) - - ld_client_mock.invoke.assert_called_once_with( - FunctionName="vodafone-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ) - - kwargs = ld_client_mock.invoke.mock_calls[0][-1] - payload_bytes = kwargs["Payload"] - payload = json.loads(payload_bytes) - - assert payload["identifier"] == identifier - assert payload["message_number"] == "00000050" - assert payload["message_format"] == "ibag" - assert payload["message_type"] == "cancel" - assert payload["references"] == [ - { - "message_id": str(provider_messages[0].id), - "message_number": "0000004e", - "sent": provider_messages[0].created_at.strftime(DATETIME_FORMAT), - }, - { - "message_id": str(provider_messages[1].id), - "message_number": "0000007b", - "sent": provider_messages[1].created_at.strftime(DATETIME_FORMAT), - }, - ] - assert payload["sent"] == sent - - -@pytest.mark.parametrize("cbc", ["ee", "vodafone", "three", "o2"]) -def test_cbc_proxy_will_failover_to_second_lambda_if_boto_client_error(mocker, cbc_proxy_client, cbc): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.side_effect = BotoClientError({}, "error") - - with pytest.raises(CBCProxyRetryableException) as e: - cbc_proxy.create_and_send_broadcast( - identifier="my-identifier", - message_number="0000007b", - headline="my-headline", - description="test-description", - areas=EXAMPLE_AREAS, - sent="a-passed-through-sent-value", - expires="a-passed-through-expires-value", - channel="severe", - ) - - assert e.match(f"Lambda failed for both {cbc}-1-proxy and {cbc}-2-proxy") - - assert ld_client_mock.invoke.call_args_list == [ - call( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - call( - FunctionName=f"{cbc}-2-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - ] - - -@pytest.mark.parametrize("cbc", ["ee", "vodafone", "three", "o2"]) -def test_cbc_proxy_will_failover_to_second_lambda_if_function_error(mocker, cbc_proxy_client, cbc): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.side_effect = [ - { - "StatusCode": 200, - "FunctionError": "Handled", - "Payload": BytesIO(json.dumps({"errorMessage": "", "errorType": "CBCNewConnectionError"}).encode("utf-8")), - }, - {"StatusCode": 200}, - ] - - cbc_proxy.create_and_send_broadcast( - identifier="my-identifier", - message_number="0000007b", - headline="my-headline", - description="test-description", - areas=EXAMPLE_AREAS, - sent="a-passed-through-sent-value", - expires="a-passed-through-expires-value", - channel="severe", - ) - - assert ld_client_mock.invoke.call_args_list == [ - call( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - call( - FunctionName=f"{cbc}-2-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - ] - - -@pytest.mark.parametrize("cbc", ["ee", "vodafone", "three", "o2"]) -def test_cbc_proxy_will_failover_to_second_lambda_if_invoke_error(mocker, cbc_proxy_client, cbc): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.side_effect = [{"StatusCode": 400}, {"StatusCode": 200}] - - cbc_proxy.create_and_send_broadcast( - identifier="my-identifier", - message_number="0000007b", - headline="my-headline", - description="test-description", - areas=EXAMPLE_AREAS, - sent="a-passed-through-sent-value", - expires="a-passed-through-expires-value", - channel="test", - ) - - assert ld_client_mock.invoke.call_args_list == [ - call( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - call( - FunctionName=f"{cbc}-2-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - ] - - -@pytest.mark.parametrize("cbc", ["ee", "vodafone", "three", "o2"]) -def test_cbc_proxy_create_and_send_tries_failover_lambda_on_invoke_error_and_raises_if_both_invoke_error( - mocker, cbc_proxy_client, cbc -): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 400, - } - - with pytest.raises(CBCProxyRetryableException) as e: - cbc_proxy.create_and_send_broadcast( - identifier="my-identifier", - message_number="0000007b", - headline="my-headline", - description="my-description", - areas=EXAMPLE_AREAS, - sent="a-passed-through-sent-value", - expires="a-passed-through-expires-value", - channel="test", - ) - - assert e.match(f"Lambda failed for both {cbc}-1-proxy and {cbc}-2-proxy") - - assert ld_client_mock.invoke.call_args_list == [ - call( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - call( - FunctionName=f"{cbc}-2-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - ] - - -@pytest.mark.parametrize("cbc", ["ee", "vodafone", "three", "o2"]) -def test_cbc_proxy_create_and_send_tries_failover_lambda_on_function_error_and_raises_if_both_function_error( - mocker, cbc_proxy_client, cbc -): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - "FunctionError": "something", - "Payload": BytesIO(json.dumps({"errorMessage": "some message", "errorType": "SomeErrorType"}).encode("utf-8")), - } - - with pytest.raises(CBCProxyRetryableException) as e: - cbc_proxy.create_and_send_broadcast( - identifier="my-identifier", - message_number="0000007b", - headline="my-headline", - description="my-description", - areas=EXAMPLE_AREAS, - sent="a-passed-through-sent-value", - expires="a-passed-through-expires-value", - channel="severe", - ) - - assert e.match(f"Lambda failed for both {cbc}-1-proxy and {cbc}-2-proxy") - - assert ld_client_mock.invoke.call_args_list == [ - call( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - call( - FunctionName=f"{cbc}-2-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ), - ] - - -@pytest.mark.parametrize("cbc", ["ee", "three", "o2"]) -def test_cbc_proxy_one_2_many_send_link_test_invokes_function(mocker, cbc_proxy_client, cbc): - cbc_proxy = cbc_proxy_client.get_proxy(cbc) - - mocker.patch("app.clients.cbc_proxy.uuid.uuid4", return_value=123) - - ld_client_mock = mocker.patch.object( - cbc_proxy, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - } - - cbc_proxy._send_link_test(lambda_name=f"{cbc}-1-proxy") - - ld_client_mock.invoke.assert_called_once_with( - FunctionName=f"{cbc}-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ) - - kwargs = ld_client_mock.invoke.mock_calls[0][-1] - payload_bytes = kwargs["Payload"] - payload = json.loads(payload_bytes) - - assert payload["identifier"] == "123" - assert payload["message_type"] == "test" - assert "message_number" not in payload - assert payload["message_format"] == "cap" - - -@pytest.mark.skip -def test_cbc_proxy_vodafone_send_link_test_invokes_function(mocker, cbc_proxy_vodafone): - mocker.patch("app.clients.cbc_proxy.uuid.uuid4", return_value=123) - - db.session.connection().execute("ALTER SEQUENCE broadcast_provider_message_number_seq RESTART WITH 1") - - ld_client_mock = mocker.patch.object( - cbc_proxy_vodafone, - "_lambda_client", - create=True, - ) - - ld_client_mock.invoke.return_value = { - "StatusCode": 200, - } - - cbc_proxy_vodafone._send_link_test(lambda_name="vodafone-1-proxy") - - ld_client_mock.invoke.assert_called_once_with( - FunctionName="vodafone-1-proxy", - InvocationType="RequestResponse", - Payload=mocker.ANY, - ) - - kwargs = ld_client_mock.invoke.mock_calls[0][-1] - payload_bytes = kwargs["Payload"] - payload = json.loads(payload_bytes) - - assert payload["identifier"] == "123" - assert payload["message_type"] == "test" - assert payload["message_number"] == "00000001" - assert payload["message_format"] == "ibag" diff --git a/tests/app/db.py b/tests/app/db.py index 64b9746bcf..7d90fe036e 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -2,8 +2,6 @@ import uuid from datetime import date, datetime, timedelta -import pytest - from app import db from app.constants import ( EMAIL_TYPE, @@ -39,12 +37,6 @@ from app.models import ( AnnualBilling, ApiKey, - BroadcastEvent, - BroadcastMessage, - BroadcastProvider, - BroadcastProviderMessage, - BroadcastProviderMessageNumber, - BroadcastStatusType, Complaint, DailySortedLetter, Domain, @@ -1163,100 +1155,6 @@ def create_service_contact_list( return contact_list -def create_broadcast_message( - template=None, - *, - service=None, # only used if template is not provided - created_by=None, - personalisation=None, - content=None, - status=BroadcastStatusType.DRAFT, - starts_at=None, - finishes_at=None, - areas=None, - stubbed=False, - cap_event=None, -): - if template: - service = template.service - template_id = template.id - template_version = template.version - personalisation = personalisation or {} - content = template._as_utils_template_with_personalisation(personalisation).content_with_placeholders_filled_in - elif content: - template_id = None - template_version = None - personalisation = None - content = content - else: - pytest.fail("Provide template or content") - - broadcast_message = BroadcastMessage( - service_id=service.id, - template_id=template_id, - template_version=template_version, - personalisation=personalisation, - status=status, - starts_at=starts_at, - finishes_at=finishes_at, - created_by_id=created_by.id if created_by else service.created_by_id, - areas=areas or {"ids": [], "simple_polygons": []}, - content=content, - stubbed=stubbed, - cap_event=cap_event, - ) - db.session.add(broadcast_message) - db.session.commit() - return broadcast_message - - -def create_broadcast_event( - broadcast_message, - sent_at=None, - message_type="alert", - transmitted_content=None, - transmitted_areas=None, - transmitted_sender=None, - transmitted_starts_at=None, - transmitted_finishes_at=None, -): - b_e = BroadcastEvent( - service=broadcast_message.service, - broadcast_message=broadcast_message, - sent_at=sent_at or datetime.utcnow(), - message_type=message_type, - transmitted_content=transmitted_content or {"body": "this is an emergency broadcast message"}, - transmitted_areas=transmitted_areas or broadcast_message.areas, - transmitted_sender=transmitted_sender or "www.notifications.service.gov.uk", - transmitted_starts_at=transmitted_starts_at, - transmitted_finishes_at=transmitted_finishes_at or datetime.utcnow() + timedelta(hours=24), - ) - db.session.add(b_e) - db.session.commit() - return b_e - - -def create_broadcast_provider_message(broadcast_event, provider, status="sending"): - broadcast_provider_message_id = uuid.uuid4() - provider_message = BroadcastProviderMessage( - id=broadcast_provider_message_id, - broadcast_event=broadcast_event, - provider=provider, - status=status, - ) - db.session.add(provider_message) - db.session.commit() - - provider_message_number = None - if provider == BroadcastProvider.VODAFONE: - provider_message_number = BroadcastProviderMessageNumber( - broadcast_provider_message_id=broadcast_provider_message_id - ) - db.session.add(provider_message_number) - db.session.commit() - return provider_message - - def create_webauthn_credential( user, name="my key", diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ea21202d71..92f392befd 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -10,7 +10,6 @@ from sqlalchemy.exc import SQLAlchemyError from app.constants import ( - BROADCAST_TYPE, EMAIL_AUTH_TYPE, EMAIL_TYPE, INBOUND_SMS_TYPE, @@ -243,16 +242,12 @@ def test_get_service_by_id(admin_request, sample_service): assert json_resp["data"]["id"] == str(sample_service.id) assert json_resp["data"]["email_branding"] is None assert json_resp["data"]["prefix_sms"] is True - assert json_resp["data"]["allowed_broadcast_provider"] is None - assert json_resp["data"]["broadcast_channel"] is None assert set(json_resp["data"].keys()) == { "active", - "allowed_broadcast_provider", "billing_contact_email_addresses", "billing_contact_names", "billing_reference", - "broadcast_channel", "consent_to_research", "contact_link", "count_as_live", @@ -929,13 +924,12 @@ def test_update_service_permissions_will_add_service_permissions(client, sample_ @pytest.mark.parametrize( "permission_to_add", [ - (EMAIL_TYPE), - (SMS_TYPE), - (INTERNATIONAL_SMS_TYPE), - (LETTER_TYPE), - (INBOUND_SMS_TYPE), - (EMAIL_AUTH_TYPE), - (BROADCAST_TYPE), # TODO: remove this ability to set broadcast permission this way + EMAIL_TYPE, + SMS_TYPE, + INTERNATIONAL_SMS_TYPE, + LETTER_TYPE, + INBOUND_SMS_TYPE, + EMAIL_AUTH_TYPE, ], ) def test_add_service_permission_will_add_permission(client, service_with_no_permissions, permission_to_add): diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 9d0ec1fc7e..1c7bf6a630 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -12,7 +12,7 @@ from notifications_utils import SMS_CHAR_COUNT_LIMIT from pypdf.errors import PdfReadError -from app.constants import BROADCAST_TYPE, EMAIL_TYPE, LETTER_TYPE, SMS_TYPE +from app.constants import EMAIL_TYPE, LETTER_TYPE, SMS_TYPE from app.dao.templates_dao import ( dao_get_template_by_id, dao_get_template_versions, @@ -35,7 +35,6 @@ @pytest.mark.parametrize( "template_type, subject", [ - (BROADCAST_TYPE, None), (SMS_TYPE, None), (EMAIL_TYPE, "subject"), (LETTER_TYPE, "subject"), @@ -209,12 +208,6 @@ def test_should_raise_error_if_service_does_not_exist_on_create(client, sample_u @pytest.mark.parametrize( "permissions, template_type, subject, expected_error", [ - ( - [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE], - BROADCAST_TYPE, - None, - {"template_type": ["Creating broadcast message templates is not allowed"]}, - ), # noqa ([EMAIL_TYPE], SMS_TYPE, None, {"template_type": ["Creating text message templates is not allowed"]}), ([SMS_TYPE], EMAIL_TYPE, "subject", {"template_type": ["Creating email templates is not allowed"]}), ([SMS_TYPE], LETTER_TYPE, "subject", {"template_type": ["Creating letter templates is not allowed"]}), @@ -592,7 +585,6 @@ def test_should_get_return_all_fields_by_default( ) assert json_response["data"][0].keys() == { "archived", - "broadcast_data", "content", "created_at", "created_by", @@ -634,7 +626,6 @@ def test_should_get_return_all_fields_by_default( (EMAIL_TYPE, None), (SMS_TYPE, None), (LETTER_TYPE, None), - (BROADCAST_TYPE, "This is a test"), ), ) def test_should_not_return_content_and_subject_if_requested( @@ -783,25 +774,16 @@ def test_should_return_404_if_no_templates_for_service_with_id(client, sample_se assert json_resp["result"] == "error" assert json_resp["message"] == "No result found" - -@pytest.mark.parametrize( - "template_type", - ( - SMS_TYPE, - BROADCAST_TYPE, - ), -) def test_create_400_for_over_limit_content( client, notify_api, - sample_user, - template_type, + sample_user ): - sample_service = create_service(service_permissions=[template_type]) + sample_service = create_service(service_permissions=[SMS_TYPE]) content = "".join(random.choice(string.ascii_uppercase + string.digits) for _ in range(SMS_CHAR_COUNT_LIMIT + 1)) data = { "name": "too big template", - "template_type": template_type, + "template_type": SMS_TYPE, "content": content, "service": str(sample_service.id), "created_by": str(sample_service.created_by.id), diff --git a/tests/app/test_config.py b/tests/app/test_config.py index ec6185c436..73a5ca9b91 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -7,7 +7,7 @@ def test_queue_names_all_queues_correct(): # Need to ensure that all_queues() only returns queue names used in API queues = QueueNames.all_queues() - assert len(queues) == 17 + assert len(queues) == 16 assert { QueueNames.PERIODIC, QueueNames.DATABASE, @@ -25,7 +25,6 @@ def test_queue_names_all_queues_correct(): QueueNames.LETTERS, QueueNames.SES_CALLBACKS, QueueNames.SMS_CALLBACKS, - QueueNames.BROADCASTS, } == set(queues) diff --git a/tests/app/v2/templates/test_get_templates.py b/tests/app/v2/templates/test_get_templates.py index ff517fbeae..ab13a42bd3 100644 --- a/tests/app/v2/templates/test_get_templates.py +++ b/tests/app/v2/templates/test_get_templates.py @@ -82,7 +82,5 @@ def test_get_all_templates_for_invalid_type_returns_400(api_client_request, samp assert json_response == { "status_code": 400, - "errors": [ - {"message": "type coconut is not one of [sms, email, letter, broadcast]", "error": "ValidationError"} - ], + "errors": [{"message": "type coconut is not one of [sms, email, letter]", "error": "ValidationError"}], } diff --git a/tests/app/v2/templates/test_templates_schemas.py b/tests/app/v2/templates/test_templates_schemas.py index fdde71c6f9..90739214d3 100644 --- a/tests/app/v2/templates/test_templates_schemas.py +++ b/tests/app/v2/templates/test_templates_schemas.py @@ -274,7 +274,7 @@ def test_get_all_template_request_schema_against_invalid_args_is_invalid(): assert errors["status_code"] == 400 assert len(errors["errors"]) == 1 - assert errors["errors"][0]["message"] == "type unknown is not one of [sms, email, letter, broadcast]" + assert errors["errors"][0]["message"] == "type unknown is not one of [sms, email, letter]" @pytest.mark.parametrize("response", valid_json_get_all_response) diff --git a/tests/conftest.py b/tests/conftest.py index 96019d3d06..012bcc42b6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -164,11 +164,8 @@ def _clean_database(_db): "organisation_types", "service_permission_types", "auth_type", - "broadcast_status_type", "invite_status_type", "service_callback_type", - "broadcast_channel_types", - "broadcast_provider_types", "default_annual_allowance", ]: _db.engine.execute(tbl.delete()) From 11ab66fddea88cd3a8c1da787adf73fc485cbb96 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 12 Dec 2023 19:07:21 +0000 Subject: [PATCH 4/9] make TemplateSchemaNoDetail tidier now that it doesn't have content the schema was originally created so that it could not show content. However, then it needed content conditionally for broadcast messages so content was added back in. Now that broadcasts no longer exist we can revert that - however, for a template that has a specific list of expected keys, it's nicer to just list those rather than maintain a huge list of exceptions that needs to be modified every time a field changes on the template model --- app/schemas.py | 31 ++++++------------------ tests/app/template/test_rest.py | 43 ++++++--------------------------- 2 files changed, 16 insertions(+), 58 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 534caa06dd..f5597101f0 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -429,29 +429,14 @@ def validate_type(self, data, **kwargs): class TemplateSchemaNoDetail(TemplateSchema): class Meta(TemplateSchema.Meta): - exclude = TemplateSchema.Meta.exclude + ( - "archived", - "created_at", - "created_by", - "created_by_id", - "has_unsubscribe_link", - "hidden", - "letter_attachment", - "postage", - "process_type", - "redact_personalisation", - "reply_to", - "reply_to_text", - "service", - "service_letter_contact", - "subject", - "template_redacted", - "updated_at", - "version", - "letter_welsh_subject", - "letter_welsh_content", - "letter_languages", - ) + fields = [ + "folder", + "id", + "is_precompiled_letter", + "name", + "template_type", + ] + exclude = [] class TemplateHistorySchema(BaseSchema): diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 1c7bf6a630..a99024fadf 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -613,45 +613,21 @@ def test_should_get_return_all_fields_by_default( } -@pytest.mark.parametrize( - "extra_args", - ( - {"detailed": False}, - {"detailed": "False"}, - ), -) -@pytest.mark.parametrize( - "template_type, expected_content", - ( - (EMAIL_TYPE, None), - (SMS_TYPE, None), - (LETTER_TYPE, None), - ), -) -def test_should_not_return_content_and_subject_if_requested( - admin_request, - sample_service, - extra_args, - template_type, - expected_content, -): - create_template( - sample_service, - template_type=template_type, - content="This is a test", - ) +@pytest.mark.parametrize("template_type", (EMAIL_TYPE, SMS_TYPE, LETTER_TYPE)) +def test_should_not_return_content_and_subject_if_requested(admin_request, sample_service, template_type): + create_template(sample_service, template_type=template_type) json_response = admin_request.get( - "template.get_all_templates_for_service", service_id=sample_service.id, **extra_args + "template.get_all_templates_for_service", + service_id=sample_service.id, + detailed=False, ) assert json_response["data"][0].keys() == { - "content", "folder", "id", "is_precompiled_letter", "name", "template_type", } - assert json_response["data"][0]["content"] == expected_content @pytest.mark.parametrize( @@ -774,11 +750,8 @@ def test_should_return_404_if_no_templates_for_service_with_id(client, sample_se assert json_resp["result"] == "error" assert json_resp["message"] == "No result found" -def test_create_400_for_over_limit_content( - client, - notify_api, - sample_user -): + +def test_create_400_for_over_limit_content(client, notify_api, sample_user): sample_service = create_service(service_permissions=[SMS_TYPE]) content = "".join(random.choice(string.ascii_uppercase + string.digits) for _ in range(SMS_CHAR_COUNT_LIMIT + 1)) data = { From 189b9b33e42c03dc84ee54385ae4d230d236a171 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 1 Oct 2024 15:54:36 +0100 Subject: [PATCH 5/9] fix flaky test --- tests/app/clients/test_aws_ses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/clients/test_aws_ses.py b/tests/app/clients/test_aws_ses.py index cc374a5af9..5b9c8dec2f 100644 --- a/tests/app/clients/test_aws_ses.py +++ b/tests/app/clients/test_aws_ses.py @@ -190,7 +190,7 @@ def test_send_email_does_not_raise_AwsSesClientThrottlingSendRateException_if_no ) -def test_send_email_raises_other_errs_as_AwsSesClientException(mocker): +def test_send_email_raises_other_errs_as_AwsSesClientException(mocker, notify_api): boto_mock = mocker.patch.object(aws_ses_client, "_client", create=True) mocker.patch.object(aws_ses_client, "statsd_client", create=True) error_response = { From 5135d38c74aec43be580cf84dce4e289a0657552 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 1 Oct 2024 12:57:02 +0100 Subject: [PATCH 6/9] remove database tables now that there's no data that depends on these, we can delete the tables completely. this was important to do as separate steps, or we could end up in a case where have have fragments of broadcast services sitting in our database but without the broadcast-specific tables, which would leave us unsure as to what those services were. --- .../versions/0466_delete_broadcast_tables.py | 185 ++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 migrations/versions/0466_delete_broadcast_tables.py diff --git a/migrations/versions/0466_delete_broadcast_tables.py b/migrations/versions/0466_delete_broadcast_tables.py new file mode 100644 index 0000000000..f0d3947800 --- /dev/null +++ b/migrations/versions/0466_delete_broadcast_tables.py @@ -0,0 +1,185 @@ +""" +Create Date: 2024-10-01 11:08:46.900469 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "0466_delete_broadcast_tables" +down_revision = "0465_delete_broadcast_data" + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("service_broadcast_settings") + op.drop_table("broadcast_channel_types") + op.drop_table("broadcast_provider_message_number") + op.drop_table("broadcast_provider_message") + op.drop_table("broadcast_event") + op.drop_table("broadcast_provider_message_status_type") + op.drop_table("broadcast_message") + op.drop_table("broadcast_provider_types") + op.drop_table("service_broadcast_provider_restriction") + op.drop_table("broadcast_status_type") + op.drop_column("templates", "broadcast_data") + op.drop_column("templates_history", "broadcast_data") + + +def downgrade(): + op.add_column( + "templates_history", + sa.Column("broadcast_data", postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=True), + ) + op.add_column( + "templates", + sa.Column("broadcast_data", postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=True), + ) + + op.create_table( + "broadcast_provider_message_status_type", + sa.Column("name", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint("name", name="broadcast_provider_message_status_type_pkey"), + ) + op.create_table( + "broadcast_status_type", + sa.Column("name", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint("name", name="broadcast_status_type_pkey"), + postgresql_ignore_search_path=False, + ) + op.create_table( + "service_broadcast_provider_restriction", + sa.Column("service_id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column("provider", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.Column("created_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint( + ["service_id"], ["services.id"], name="service_broadcast_provider_restriction_service_id_fkey" + ), + sa.PrimaryKeyConstraint("service_id", name="service_broadcast_provider_restriction_pkey"), + ) + op.create_table( + "broadcast_provider_types", + sa.Column("name", sa.VARCHAR(length=255), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint("name", name="broadcast_provider_types_pkey"), + postgresql_ignore_search_path=False, + ) + op.create_table( + "broadcast_message", + sa.Column("id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column("service_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("template_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("template_version", sa.INTEGER(), autoincrement=False, nullable=True), + sa.Column("_personalisation", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("areas", postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=False), + sa.Column("status", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.Column("starts_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("finishes_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("created_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.Column("approved_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("cancelled_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("updated_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("created_by_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("approved_by_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("cancelled_by_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("content", sa.TEXT(), autoincrement=False, nullable=False), + sa.Column("reference", sa.VARCHAR(length=255), autoincrement=False, nullable=True), + sa.Column("stubbed", sa.BOOLEAN(), autoincrement=False, nullable=False), + sa.Column("cap_event", sa.VARCHAR(length=255), autoincrement=False, nullable=True), + sa.Column("created_by_api_key_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("cancelled_by_api_key_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.CheckConstraint( + "(created_by_id IS NOT NULL) OR (created_by_api_key_id IS NOT NULL)", + name="ck_broadcast_message_created_by_not_null", + ), + sa.ForeignKeyConstraint(["approved_by_id"], ["users.id"], name="broadcast_message_approved_by_id_fkey"), + sa.ForeignKeyConstraint( + ["cancelled_by_api_key_id"], ["api_keys.id"], name="broadcast_message_cancelled_by_api_key_id_fkey" + ), + sa.ForeignKeyConstraint(["cancelled_by_id"], ["users.id"], name="broadcast_message_cancelled_by_id_fkey"), + sa.ForeignKeyConstraint( + ["created_by_api_key_id"], ["api_keys.id"], name="broadcast_message_created_by_api_key_id_fkey" + ), + sa.ForeignKeyConstraint(["created_by_id"], ["users.id"], name="broadcast_message_created_by_id_fkey"), + sa.ForeignKeyConstraint(["service_id"], ["services.id"], name="broadcast_message_service_id_fkey"), + sa.ForeignKeyConstraint(["status"], ["broadcast_status_type.name"], name="broadcast_message_status_fkey"), + sa.ForeignKeyConstraint( + ["template_id", "template_version"], + ["templates_history.id", "templates_history.version"], + name="broadcast_message_template_id_fkey", + ), + sa.PrimaryKeyConstraint("id", name="broadcast_message_pkey"), + ) + op.create_table( + "broadcast_event", + sa.Column("id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column("service_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("broadcast_message_id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column("sent_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.Column("message_type", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.Column("transmitted_content", postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=True), + sa.Column("transmitted_areas", postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=False), + sa.Column("transmitted_sender", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.Column("transmitted_starts_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("transmitted_finishes_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.ForeignKeyConstraint( + ["broadcast_message_id"], ["broadcast_message.id"], name="broadcast_event_broadcast_message_id_fkey" + ), + sa.ForeignKeyConstraint(["service_id"], ["services.id"], name="broadcast_event_service_id_fkey"), + sa.PrimaryKeyConstraint("id", name="broadcast_event_pkey"), + postgresql_ignore_search_path=False, + ) + op.create_table( + "broadcast_provider_message", + sa.Column("id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column("broadcast_event_id", postgresql.UUID(), autoincrement=False, nullable=True), + sa.Column("provider", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("status", sa.VARCHAR(), autoincrement=False, nullable=True), + sa.Column("created_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.Column("updated_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.ForeignKeyConstraint( + ["broadcast_event_id"], ["broadcast_event.id"], name="broadcast_provider_message_broadcast_event_id_fkey" + ), + sa.PrimaryKeyConstraint("id", name="broadcast_provider_message_pkey"), + sa.UniqueConstraint( + "broadcast_event_id", "provider", name="broadcast_provider_message_broadcast_event_id_provider_key" + ), + postgresql_ignore_search_path=False, + ) + op.create_table( + "broadcast_provider_message_number", + sa.Column( + "broadcast_provider_message_number", + sa.INTEGER(), + server_default=sa.text("nextval('broadcast_provider_message_number_seq'::regclass)"), + autoincrement=True, + nullable=False, + ), + sa.Column("broadcast_provider_message_id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint( + ["broadcast_provider_message_id"], + ["broadcast_provider_message.id"], + name="broadcast_provider_message_nu_broadcast_provider_message_i_fkey", + ), + sa.PrimaryKeyConstraint("broadcast_provider_message_number", name="broadcast_provider_message_number_pkey"), + ) + op.create_table( + "broadcast_channel_types", + sa.Column("name", sa.VARCHAR(length=255), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint("name", name="broadcast_channel_types_pkey"), + ) + op.create_table( + "service_broadcast_settings", + sa.Column("service_id", postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column("channel", sa.VARCHAR(length=255), autoincrement=False, nullable=False), + sa.Column("created_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.Column("updated_at", postgresql.TIMESTAMP(), autoincrement=False, nullable=True), + sa.Column("provider", sa.VARCHAR(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint( + ["channel"], ["broadcast_channel_types.name"], name="service_broadcast_settings_channel_fkey" + ), + sa.ForeignKeyConstraint( + ["provider"], ["broadcast_provider_types.name"], name="service_broadcast_settings_provider_fkey" + ), + sa.ForeignKeyConstraint(["service_id"], ["services.id"], name="service_broadcast_settings_service_id_fkey"), + sa.PrimaryKeyConstraint("service_id", name="service_broadcast_settings_pkey"), + ) From 39f87924aec5df11fecf989f3eeedb6131fce93a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 1 Oct 2024 12:58:45 +0100 Subject: [PATCH 7/9] remove broadcast permissions the enum dance - rename the enum to have a temp name, create a new type with the proper name that doesn't have the unwanted values, and then alter the column on the permissions table to change the type (casting the old value from old_enum -> text -> new_enum), then we drop the old type with the temp name. --- .../versions/0467_remove_broadcast_perms.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 migrations/versions/0467_remove_broadcast_perms.py diff --git a/migrations/versions/0467_remove_broadcast_perms.py b/migrations/versions/0467_remove_broadcast_perms.py new file mode 100644 index 0000000000..d915081a5f --- /dev/null +++ b/migrations/versions/0467_remove_broadcast_perms.py @@ -0,0 +1,49 @@ +""" +Create Date: 2024-10-01 11:08:46.900469 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "0467_remove_broadcast_perms" +down_revision = "0466_delete_broadcast_tables" + +# code copied from 0359 (but switched upgrade/downgrade) +enum_name = "permission_types" +tmp_name = "tmp_" + enum_name + +old_options = ( + "manage_users", + "manage_templates", + "manage_settings", + "send_texts", + "send_emails", + "send_letters", + "manage_api_keys", + "platform_admin", + "view_activity", +) +old_type = sa.Enum(*old_options, name=enum_name) + + +def upgrade(): + op.execute( + "DELETE FROM permissions WHERE permission in " + "('create_broadcasts', 'approve_broadcasts', 'cancel_broadcasts', 'reject_broadcasts')" + ) + + op.execute(f"ALTER TYPE {enum_name} RENAME TO {tmp_name}") + old_type.create(op.get_bind()) + op.execute(f"ALTER TABLE permissions ALTER COLUMN permission TYPE {enum_name} USING permission::text::{enum_name}") + op.execute(f"DROP TYPE {tmp_name}") + + +def downgrade(): + # ALTER TYPE must be run outside of a transaction block (see link below for details) + # https://alembic.sqlalchemy.org/en/latest/api/runtime.html#alembic.runtime.migration.MigrationContext.autocommit_block + with op.get_context().autocommit_block(): + op.execute("ALTER TYPE permission_types ADD VALUE 'create_broadcasts'") + op.execute("ALTER TYPE permission_types ADD VALUE 'approve_broadcasts'") + op.execute("ALTER TYPE permission_types ADD VALUE 'cancel_broadcasts'") + op.execute("ALTER TYPE permission_types ADD VALUE 'reject_broadcasts'") From 0823bd02a4fd25316dbd4f7ad246863e3ed25012 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 1 Oct 2024 14:29:58 +0100 Subject: [PATCH 8/9] remove broadcast from template_type we can't _just_ do the same process as 0467 because there are check constraints that depend on these. When you say "alter column template_type set type = new_type using old_type::text::new_type", it takes that "using" statement as a way of translating the prev enum into the new enum, in our case just casting to a str as an intermediate step. however, the template_type column has several check constraints against it (to say only letters can have a letter_attachment for example). Postgres isn't smart enough to take the `using` clause and apply it to these constraints. There are two ways around this: * You could create a comparator function that tells postgres "if you have to compare old_type with new_type, here's a custom function"[^1] * You can drop the old constraints and recreate them after the migration is done We've gone with the second option as it's a bit simpler and I could copy and paste from our previous scripts [^1]: https://stackoverflow.com/a/57952345 was very helpful for this --- migrations/.current-alembic-head | 2 +- .../versions/0468_remove_broadcast_type.py | 132 ++++++++++++++++++ .../versions/0469_validate_template_ck.py | 26 ++++ 3 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 migrations/versions/0468_remove_broadcast_type.py create mode 100644 migrations/versions/0469_validate_template_ck.py diff --git a/migrations/.current-alembic-head b/migrations/.current-alembic-head index 521b5c414f..b0802b1595 100644 --- a/migrations/.current-alembic-head +++ b/migrations/.current-alembic-head @@ -1 +1 @@ -0465_delete_broadcast_data +0468_remove_broadcast_type diff --git a/migrations/versions/0468_remove_broadcast_type.py b/migrations/versions/0468_remove_broadcast_type.py new file mode 100644 index 0000000000..6478d5414f --- /dev/null +++ b/migrations/versions/0468_remove_broadcast_type.py @@ -0,0 +1,132 @@ +""" +Create Date: 2024-10-01 11:08:46.900469 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "0468_remove_broadcast_type" +down_revision = "0467_remove_broadcast_perms" + + +template_type_new_values = ["email", "sms", "letter"] +template_type_new = postgresql.ENUM(*template_type_new_values, name="template_type") + + +def drop_check_constraints(): + op.drop_constraint("chk_templates_letter_languages", "templates") + op.drop_constraint("chk_templates_history_letter_languages", "templates_history") + + op.drop_constraint("ck_templates_letter_attachments", "templates") + op.drop_constraint("ck_templates_history_letter_attachments", "templates_history") + + op.drop_constraint("ck_templates_non_email_has_unsubscribe_false", "templates") + op.drop_constraint("ck_templates_history_non_email_has_unsubscribe_false", "templates_history") + + +def add_check_constraints(): + op.create_check_constraint( + "chk_templates_letter_languages", + "templates", + """ + CASE WHEN template_type = 'letter' THEN + letter_languages is not null + ELSE + letter_languages is null + END + """, + postgresql_not_valid=True, + ) + op.create_check_constraint( + "chk_templates_history_letter_languages", + "templates_history", + """ + CASE WHEN template_type = 'letter' THEN + letter_languages is not null + ELSE + letter_languages is null + END + """, + postgresql_not_valid=True, + ) + + op.create_check_constraint( + "ck_templates_letter_attachments", + "templates", + "template_type = 'letter' OR letter_attachment_id IS NULL", + postgresql_not_valid=True, + ) + op.create_check_constraint( + "ck_templates_history_letter_attachments", + "templates_history", + "template_type = 'letter' OR letter_attachment_id IS NULL", + postgresql_not_valid=True, + ) + + op.create_check_constraint( + "ck_templates_non_email_has_unsubscribe_false", + "templates", + "template_type = 'email' OR has_unsubscribe_link IS false", + postgresql_not_valid=True, + ) + op.create_check_constraint( + "ck_templates_history_non_email_has_unsubscribe_false", + "templates_history", + "template_type = 'email' OR has_unsubscribe_link IS false", + postgresql_not_valid=True, + ) + + +def upgrade(): + """ + there are three check constraints on the template_type column (across both templates and templates_history). + + These check constraints say things like "if type == 'letter'" - this is an enum comparison, so if we change the + type of the column, we get an error + + > (psycopg2.errors.UndefinedFunction) operator does not exist: template_type = template_type_old + > HINT: No operator matches the given name and argument types. You might need to add explicit type casts. + + This appears confusing because we're passing in a `using` clause to tell postgres exactly how to do this, but + this using clause only applies to the column type, and not other usages of that column within the check constraints. + + To get round this, we "simply" drop all the constraints and then recreate them afterwards, referring to the new + """ + conn = op.get_bind() + conn.execute("ALTER TYPE template_type RENAME TO template_type_old;") + template_type_new.create(conn) + + drop_check_constraints() + + op.alter_column( + table_name="templates", + column_name="template_type", + type_=template_type_new, + postgresql_using="template_type::text::template_type", + ) + op.alter_column( + table_name="templates_history", + column_name="template_type", + type_=template_type_new, + postgresql_using="template_type::text::template_type", + ) + op.alter_column( + table_name="service_contact_list", + column_name="template_type", + type_=template_type_new, + postgresql_using="template_type::text::template_type", + ) + + conn.execute("DROP TYPE template_type_old;") + # note: need to revalidate these constraints in a separate migration to avoid a lengthy access exclusive lock + add_check_constraints() + + +def downgrade(): + # don't need to do the constraints dance, or the enum type dance, when adding a new value + + # ALTER TYPE must be run outside of a transaction block (see link below for details) + # https://alembic.sqlalchemy.org/en/latest/api/runtime.html#alembic.runtime.migration.MigrationContext.autocommit_block + with op.get_context().autocommit_block(): + op.execute("ALTER TYPE template_type ADD VALUE 'broadcast'") diff --git a/migrations/versions/0469_validate_template_ck.py b/migrations/versions/0469_validate_template_ck.py new file mode 100644 index 0000000000..21b800f1ae --- /dev/null +++ b/migrations/versions/0469_validate_template_ck.py @@ -0,0 +1,26 @@ +""" +Create Date: 2024-10-01 11:08:46.900469 +""" + +revision = "0469_validate_template_ck" +down_revision = "0468_remove_broadcast_type" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +def upgrade(): + + op.execute("ALTER TABLE templates VALIDATE CONSTRAINT chk_templates_letter_languages") + op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT chk_templates_history_letter_languages") + + op.execute("ALTER TABLE templates VALIDATE CONSTRAINT ck_templates_letter_attachments") + op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT ck_templates_history_letter_attachments") + + op.execute("ALTER TABLE templates VALIDATE CONSTRAINT ck_templates_non_email_has_unsubscribe_false") + op.execute("ALTER TABLE templates_history VALIDATE CONSTRAINT ck_templates_history_non_email_has_unsubscribe_false") + + +def downgrade(): + pass From ed4671c49b3ef6ee2f3045f347b6516411b1ae53 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 3 Oct 2024 16:51:26 +0100 Subject: [PATCH 9/9] wip --- migrations/enum_utils.py | 66 +++++++++++++++++++ .../versions/0467_remove_broadcast_perms.py | 6 +- .../versions/0468_remove_broadcast_type.py | 1 + 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 migrations/enum_utils.py diff --git a/migrations/enum_utils.py b/migrations/enum_utils.py new file mode 100644 index 0000000000..6789961c91 --- /dev/null +++ b/migrations/enum_utils.py @@ -0,0 +1,66 @@ +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + + +def enum_type_remove_value_step_1_create_new_enum( + existing_enum_values: list[str], value_to_remove: str +) -> postgresql.ENUM: + """ + If there any check constraints that use this column, they must be DROPPED beforehand and RECREATED afterwards + """ + new_enum_values = existing_enum_values.copy() + new_enum_values.remove(value_to_remove) + enum_new = postgresql.ENUM(*new_enum_values, name=tmp_enum_name) + + pass + + +def enum_type_remove_value_step_2_update_columns(table: str, column: str, new_enum_type): + """ + This acquires an ACCESS EXCLUSIVE lock so you should only one run of these in a + single migration or you run the risk of deadlocks + """ + pass + + +def enum_type_remove_value_step_3_remove_old_enum(): + pass + + +def add_type_to_enum(tables_and_columns: list[tuple[str, str]], existing_values: list[str], new_value: str = None): + """ + tables_and_columns: a list of table-column pairs that use this enum, eg + [("templates", "template_type"), ("templates_history", "template_type"), ...] + + existing_values: the existing values in the old enum + new_values: the new values to add + """ + conn = op.get_bind() + conn.execute("ALTER TYPE template_type RENAME TO template_type_old;") + template_type_new.create(conn) + + drop_check_constraints() + + op.alter_column( + table_name="templates", + column_name="template_type", + type_=template_type_new, + postgresql_using="template_type::text::template_type", + ) + op.alter_column( + table_name="templates_history", + column_name="template_type", + type_=template_type_new, + postgresql_using="template_type::text::template_type", + ) + op.alter_column( + table_name="service_contact_list", + column_name="template_type", + type_=template_type_new, + postgresql_using="template_type::text::template_type", + ) + + conn.execute("DROP TYPE template_type_old;") + # note: need to revalidate these constraints in a separate migration to avoid a lengthy access exclusive lock + add_check_constraints() diff --git a/migrations/versions/0467_remove_broadcast_perms.py b/migrations/versions/0467_remove_broadcast_perms.py index d915081a5f..6694f9cd2d 100644 --- a/migrations/versions/0467_remove_broadcast_perms.py +++ b/migrations/versions/0467_remove_broadcast_perms.py @@ -13,7 +13,7 @@ enum_name = "permission_types" tmp_name = "tmp_" + enum_name -old_options = ( +new_options = ( "manage_users", "manage_templates", "manage_settings", @@ -24,7 +24,7 @@ "platform_admin", "view_activity", ) -old_type = sa.Enum(*old_options, name=enum_name) +new_type = sa.Enum(*new_options, name=enum_name) def upgrade(): @@ -34,7 +34,7 @@ def upgrade(): ) op.execute(f"ALTER TYPE {enum_name} RENAME TO {tmp_name}") - old_type.create(op.get_bind()) + new_type.create(op.get_bind()) op.execute(f"ALTER TABLE permissions ALTER COLUMN permission TYPE {enum_name} USING permission::text::{enum_name}") op.execute(f"DROP TYPE {tmp_name}") diff --git a/migrations/versions/0468_remove_broadcast_type.py b/migrations/versions/0468_remove_broadcast_type.py index 6478d5414f..366146ae90 100644 --- a/migrations/versions/0468_remove_broadcast_type.py +++ b/migrations/versions/0468_remove_broadcast_type.py @@ -99,6 +99,7 @@ def upgrade(): drop_check_constraints() + # TODO: split into three separate migrations. figure out downgrade steps. op.alter_column( table_name="templates", column_name="template_type",