From 5e9d8e5fa0dcf746989e66f1e5f421b77c8de982 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Mon, 19 Jul 2021 16:17:33 +0100 Subject: [PATCH] Auto expire old broadcast messages Since the expiry is sent as part of the message payload, we don't need to invoke the CBC proxies (and indeed there's no way to do so for an expired alert). In future we plan to extend this task so it triggers the regeneration of content on gov.uk/alerts. It's worth noting that 'finishes_at' can theoretically be None, in which case it's unclear when the alert should expire. While alerts from the Admin app should always have an expiry [1], we have many in the DB that don't, so it's worth checking for this scenario. [1]: https://github.com/alphagov/notifications-admin/blob/078ac10c8df855a84c157190d43daa9a8897832b/app/models/broadcast_message.py#L255 --- app/celery/scheduled_tasks.py | 17 +++++++++++- app/config.py | 5 ++++ tests/app/celery/test_scheduled_tasks.py | 33 +++++++++++++++++++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 82ce46cb4..ea299307a 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -4,7 +4,7 @@ from flask import current_app from sqlalchemy import between from sqlalchemy.exc import SQLAlchemyError -from app import notify_celery, zendesk_client +from app import db, notify_celery, zendesk_client from app.celery.broadcast_message_tasks import trigger_link_test from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter from app.celery.tasks import ( @@ -48,6 +48,8 @@ from app.models import ( JOB_STATUS_IN_PROGRESS, JOB_STATUS_PENDING, SMS_TYPE, + BroadcastMessage, + BroadcastStatusType, Job, ) from app.notifications.process_notifications import send_notification_to_queue @@ -304,3 +306,16 @@ def trigger_link_tests(): if current_app.config['CBC_PROXY_ENABLED']: for cbc_name in current_app.config['ENABLED_CBCS']: trigger_link_test.apply_async(kwargs={'provider': cbc_name}, queue=QueueNames.BROADCASTS) + + +@notify_celery.task(name='auto-expire-broadcast-messages') +def auto_expire_broadcast_messages(): + expired_broadcasts = BroadcastMessage.query.filter( + BroadcastMessage.finishes_at <= datetime.now(), + BroadcastMessage.status == BroadcastStatusType.BROADCASTING, + ) + + for broadcast in expired_broadcasts: + broadcast.status = BroadcastStatusType.COMPLETED + + db.session.commit() diff --git a/app/config.py b/app/config.py index 043d82eb8..973eb3e28 100644 --- a/app/config.py +++ b/app/config.py @@ -323,6 +323,11 @@ class Config(object): 'schedule': timedelta(minutes=15), 'options': {'queue': QueueNames.PERIODIC} }, + 'auto-expire-broadcast-messages': { + 'task': 'auto-expire-broadcast-messages', + 'schedule': timedelta(minutes=5), + 'options': {'queue': QueueNames.PERIODIC} + }, } CELERY_QUEUES = [] diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index d72e6d544..06dde61a3 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -8,6 +8,7 @@ from freezegun import freeze_time from app.celery import scheduled_tasks from app.celery.scheduled_tasks import ( + auto_expire_broadcast_messages, check_for_missing_rows_in_completed_jobs, check_for_services_with_high_failure_rates_or_sending_to_tv_numbers, check_if_letters_still_in_created, @@ -30,9 +31,15 @@ from app.models import ( JOB_STATUS_PENDING, NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, + BroadcastStatusType, ) from tests.app import load_example_csv -from tests.app.db import create_job, create_notification, create_template +from tests.app.db import ( + create_broadcast_message, + create_job, + create_notification, + create_template, +) from tests.conftest import set_config @@ -624,3 +631,27 @@ def test_trigger_link_does_nothing_if_cbc_proxy_disabled( trigger_link_tests() assert mock_trigger_link_test.called is False + + +@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), +]) +def test_auto_expire_broadcast_messages( + status, + finishes_at, + final_status, + sample_template +): + message = create_broadcast_message( + status=status, + finishes_at=finishes_at, + template=sample_template, + ) + + auto_expire_broadcast_messages() + assert message.status == final_status