From bf0bf4e31cb909623341939c00ddb1742c7a96cb Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 27 Aug 2021 14:11:59 +0100 Subject: [PATCH] Favour new "areas" format for PagerDuty alerts Broadcasts created via the API [1] and the Admin app [2] should both now have this field set. It's also more informative to show this, and broadcasts created via the API don't have IDs anyway. There's a small risk that an old broadcast that gets approved won't have this data, but it's for information only and we intend to backfill all old broadcasts in the near future. [1]: https://github.com/alphagov/notifications-api/pull/3312/commits/023a06d5fb4c20e6bf5a1c9e65b473cb376b2b6b [2]: https://github.com/alphagov/notifications-admin/pull/4004/commits/7dbe3afa19e29ff793213ced0d1360d0e677f70c --- app/celery/broadcast_message_tasks.py | 2 +- tests/app/celery/test_broadcast_message_tasks.py | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 6a0b06886..641e63f21 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -135,7 +135,7 @@ def send_broadcast_event(broadcast_event_id): f'https://www.notifications.service.gov.uk/services/{broadcast_message.service_id}/current-alerts/{broadcast_message.id}', # noqa '', f'This broacast has been sent on channel {broadcast_message.service.broadcast_channel}.', - f'This broadcast is targeted at areas {broadcast_message.areas.get("ids", broadcast_message.areas.get("areas"))}.', # noqa + f'This broadcast is targeted at areas {broadcast_message.areas.get("names", [])}.', # noqa '' f'This broadcast\'s content starts "{broadcast_message.content[:100]}"' '', diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index e10f6cbf8..5411419a5 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -95,12 +95,22 @@ def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabl assert mock_send_broadcast_provider_message.apply_async.called is False -def test_send_broadcast_event_creates_zendesk_p1(mocker, notify_api, sample_broadcast_service): +@pytest.mark.parametrize('area_data,expected_message', [ + ({'names': ['England', 'Scotland']}, ['England', 'Scotland']), + ({}, []) +]) +def test_send_broadcast_event_creates_zendesk_p1( + area_data, + expected_message, + mocker, + notify_api, + sample_broadcast_service +): template = create_template(sample_broadcast_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( template, status=BroadcastStatusType.BROADCASTING, - areas={'ids': ['wd20-S13002775', 'wd20-S13002773'], 'simple_polygons': []}, + areas={**area_data, 'simple_polygons': []}, ) event = create_broadcast_event(broadcast_message) mock_create_ticket = mocker.patch("app.celery.broadcast_message_tasks.zendesk_client.create_ticket") @@ -117,7 +127,7 @@ def test_send_broadcast_event_creates_zendesk_p1(mocker, notify_api, sample_broa assert str(broadcast_message.id) in zendesk_args['message'] assert 'channel severe' in zendesk_args['message'] - assert "areas ['wd20-S13002775', 'wd20-S13002773']" in zendesk_args['message'] + assert f"areas {expected_message}" in zendesk_args['message'] # the start of the content from the broadcast template assert "Dear Sir/Madam" in zendesk_args['message']