From 04bfd6bfdb052687f0ebaa9315459970ed98930d Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 15 Oct 2021 11:39:15 +0100 Subject: [PATCH 1/2] Trigger task to publish alerts when sending or cancelling alert When we send or cancel a broadcast message, we now trigger a task in govuk-alerts repo that polls our API for alerts and publishes a fresh list of alerts. Co-authored-by: Pea Tyczynska --- app/celery/broadcast_message_tasks.py | 7 ++- app/config.py | 2 + .../celery/test_broadcast_message_tasks.py | 44 +++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 7d6de73b8..35fb5e8a0 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -7,7 +7,7 @@ from notifications_utils.clients.zendesk.zendesk_client import ( from app import cbc_proxy_client, notify_celery, zendesk_client from app.clients.cbc_proxy import CBCProxyRetryableException -from app.config import QueueNames +from app.config import QueueNames, TaskNames from app.dao.broadcast_message_dao import ( create_broadcast_provider_message, dao_get_broadcast_event_by_id, @@ -157,6 +157,11 @@ def send_broadcast_event(broadcast_event_id): zendesk_client.send_ticket_to_zendesk(ticket) current_app.logger.error(message) + notify_celery.send_task( + name=TaskNames.PUBLISH_GOVUK_ALERTS, + queue=QueueNames.GOVUK_ALERTS + ) + for provider in broadcast_event.service.get_available_broadcast_providers(): send_broadcast_provider_message.apply_async( kwargs={'broadcast_event_id': broadcast_event_id, 'provider': provider}, diff --git a/app/config.py b/app/config.py index ae5de0b6e..0d29a02d1 100644 --- a/app/config.py +++ b/app/config.py @@ -35,6 +35,7 @@ class QueueNames(object): SAVE_API_EMAIL = 'save-api-email-tasks' SAVE_API_SMS = 'save-api-sms-tasks' BROADCASTS = 'broadcast-tasks' + GOVUK_ALERTS = 'govuk-alerts' @staticmethod def all_queues(): @@ -75,6 +76,7 @@ class TaskNames(object): 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' class Config(object): diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index eb1bc882d..8c49b19ba 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -18,6 +18,7 @@ from app.celery.broadcast_message_tasks import ( trigger_link_test, ) from app.clients.cbc_proxy import CBCProxyRetryableException +from app.config import QueueNames, TaskNames from app.models import ( BROADCAST_TYPE, BroadcastEventMessageType, @@ -37,6 +38,9 @@ def test_send_broadcast_event_queues_up_for_active_providers(mocker, notify_api, template = create_template(sample_broadcast_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING) event = create_broadcast_event(broadcast_message) + + mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') + mock_send_ticket_to_zendesk = mocker.patch( 'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk', autospec=True, @@ -57,6 +61,35 @@ def test_send_broadcast_event_queues_up_for_active_providers(mocker, notify_api, assert mock_send_ticket_to_zendesk.called is False +@pytest.mark.parametrize('message_status', [ + BroadcastStatusType.BROADCASTING, + BroadcastStatusType.CANCELLED, +]) +def test_send_broadcast_event_calls_publish_govuk_alerts_task( + mocker, notify_api, sample_broadcast_service, message_status +): + template = create_template(sample_broadcast_service, BROADCAST_TYPE) + broadcast_message = create_broadcast_message(template, status=message_status) + event = create_broadcast_event(broadcast_message) + mocker.patch( + 'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk', + autospec=True, + ) + mocker.patch( + 'app.celery.broadcast_message_tasks.send_broadcast_provider_message', + ) + + mock_celery = mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') + + with set_config(notify_api, 'ENABLED_CBCS', ['ee', 'vodafone']): + send_broadcast_event(event.id) + + mock_celery.assert_called_once_with( + name=TaskNames.PUBLISH_GOVUK_ALERTS, + queue=QueueNames.GOVUK_ALERTS + ) + + def test_send_broadcast_event_only_sends_to_one_provider_if_set_on_service( mocker, notify_db, @@ -71,6 +104,7 @@ def test_send_broadcast_event_only_sends_to_one_provider_if_set_on_service( mock_send_broadcast_provider_message = mocker.patch( 'app.celery.broadcast_message_tasks.send_broadcast_provider_message', ) + mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') with set_config(notify_api, 'ENABLED_CBCS', ['ee', 'vodafone']): send_broadcast_event(event.id) @@ -91,6 +125,8 @@ def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabl broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING) event = create_broadcast_event(broadcast_message) + mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') + mock_send_broadcast_provider_message = mocker.patch( 'app.celery.broadcast_message_tasks.send_broadcast_provider_message', ) @@ -120,6 +156,8 @@ def test_send_broadcast_event_creates_zendesk( ) event = create_broadcast_event(broadcast_message) mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__') + mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') + mock_send_ticket_to_zendesk = mocker.patch( 'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk', autospec=True, @@ -160,6 +198,9 @@ def test_send_broadcast_event_doesnt_create_zendesk_when_cancelling(mocker, noti ) create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.ALERT) cancel_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.CANCEL) + + mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') + mock_send_ticket_to_zendesk = mocker.patch( 'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk', autospec=True, @@ -176,6 +217,9 @@ def test_send_broadcast_event_doesnt_create_zendesk_on_staging(mocker, notify_ap template = create_template(sample_broadcast_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message(template, status=BroadcastStatusType.BROADCASTING) event = create_broadcast_event(broadcast_message) + + mocker.patch('app.celery.broadcast_message_tasks.notify_celery.send_task') + mock_send_ticket_to_zendesk = mocker.patch( 'app.celery.broadcast_message_tasks.zendesk_client.send_ticket_to_zendesk', autospec=True, From 1b6f9505da8ae3515ef8135649ada12a5bdb30d0 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 15 Oct 2021 14:07:01 +0100 Subject: [PATCH 2/2] Call `publish-govuk-alerts` task when alert expires The `auto-expire-broadcast-messages` task checks for expired broadcasts at five minute intervals. This change now calls the `publish-govuk-alerts` task in govuk-alerts if there are expired broadcasts so that the site is updated. Co-authored-by: Katie Smith --- app/celery/scheduled_tasks.py | 10 +++++++-- tests/app/celery/test_scheduled_tasks.py | 27 +++++++++++++++++------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 073624218..28ea285f4 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -16,7 +16,7 @@ from app.celery.tasks import ( process_job, process_row, ) -from app.config import QueueNames +from app.config import QueueNames, TaskNames from app.dao.invited_org_user_dao import ( delete_org_invitations_created_more_than_two_days_ago, ) @@ -322,9 +322,15 @@ def auto_expire_broadcast_messages(): expired_broadcasts = BroadcastMessage.query.filter( BroadcastMessage.finishes_at <= datetime.now(), BroadcastMessage.status == BroadcastStatusType.BROADCASTING, - ) + ).all() for broadcast in expired_broadcasts: broadcast.status = BroadcastStatusType.COMPLETED db.session.commit() + + if expired_broadcasts: + notify_celery.send_task( + name=TaskNames.PUBLISH_GOVUK_ALERTS, + queue=QueueNames.GOVUK_ALERTS + ) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index ab13e6e97..32ae71394 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -24,7 +24,7 @@ from app.celery.scheduled_tasks import ( switch_current_sms_provider_on_slow_delivery, trigger_link_tests, ) -from app.config import Config, QueueNames +from app.config import Config, QueueNames, TaskNames from app.dao.jobs_dao import dao_get_job_by_id from app.dao.provider_details_dao import get_provider_details_by_identifier from app.models import ( @@ -668,24 +668,35 @@ def test_trigger_link_does_nothing_if_cbc_proxy_disabled( @freeze_time('2021-07-19 15:50') -@pytest.mark.parametrize('status, finishes_at, final_status', [ - (BroadcastStatusType.BROADCASTING, '2021-07-19 16:00', BroadcastStatusType.BROADCASTING), - (BroadcastStatusType.BROADCASTING, '2021-07-19 15:40', BroadcastStatusType.COMPLETED), - (BroadcastStatusType.BROADCASTING, None, BroadcastStatusType.BROADCASTING), - (BroadcastStatusType.PENDING_APPROVAL, None, BroadcastStatusType.PENDING_APPROVAL), - (BroadcastStatusType.CANCELLED, '2021-07-19 15:40', BroadcastStatusType.CANCELLED), +@pytest.mark.parametrize('status, finishes_at, final_status, should_call_publish_task', [ + (BroadcastStatusType.BROADCASTING, '2021-07-19 16:00', BroadcastStatusType.BROADCASTING, False), + (BroadcastStatusType.BROADCASTING, '2021-07-19 15:40', BroadcastStatusType.COMPLETED, True), + (BroadcastStatusType.BROADCASTING, None, BroadcastStatusType.BROADCASTING, False), + (BroadcastStatusType.PENDING_APPROVAL, None, BroadcastStatusType.PENDING_APPROVAL, False), + (BroadcastStatusType.CANCELLED, '2021-07-19 15:40', BroadcastStatusType.CANCELLED, False), ]) def test_auto_expire_broadcast_messages( + mocker, status, finishes_at, final_status, - sample_template + sample_template, + should_call_publish_task, ): message = create_broadcast_message( status=status, finishes_at=finishes_at, template=sample_template, ) + mock_celery = mocker.patch('app.celery.scheduled_tasks.notify_celery.send_task') auto_expire_broadcast_messages() assert message.status == final_status + + if should_call_publish_task: + mock_celery.assert_called_once_with( + name=TaskNames.PUBLISH_GOVUK_ALERTS, + queue=QueueNames.GOVUK_ALERTS + ) + else: + assert not mock_celery.called