From 65c21f694c98ea838abab6dd6d01c4cb07aa8ca3 Mon Sep 17 00:00:00 2001 From: sakisv Date: Thu, 9 Sep 2021 16:44:19 +0300 Subject: [PATCH 1/3] Don't raise P1 for broadcasts This is happening on the AWS side now as part of alphagov/notifications-broadcasts-infra#267 - but we still want to keep the zendesk ticket as it contains useful context _and_ provides visibility to the team. --- app/celery/broadcast_message_tasks.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 641e63f21..ddffd8a2a 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -58,7 +58,7 @@ def check_event_is_authorised_to_be_sent(broadcast_event, provider): def check_event_makes_sense_in_sequence(broadcast_event, provider): """ If any previous event hasn't sent yet for that provider, then we shouldn't send the current event. Instead, fail and - raise a P1 - so that a notify team member can assess the state of the previous messages, and if necessary, can + raise a zendesk ticket - so that a notify team member can assess the state of the previous messages, and if necessary, can replay the `send_broadcast_provider_message` task if the previous message has now been sent. Note: This is called before the new broadcast_provider_message is created. @@ -128,7 +128,7 @@ def send_broadcast_event(broadcast_event_id): broadcast_event.message_type == BroadcastEventMessageType.ALERT ): broadcast_message = broadcast_event.broadcast_message - # raise a P1 to alert team that broadcast is going out. + # raise a zendesk ticket to alert team that broadcast is going out. message = '\n'.join([ 'Broadcast Sent', '', @@ -146,7 +146,6 @@ def send_broadcast_event(broadcast_event_id): subject="Live broadcast sent", message=message, ticket_type=zendesk_client.TYPE_INCIDENT, - p1=True, ) current_app.logger.error(message) From 9faa3d34e14f10b059564c103f7b04967c156770 Mon Sep 17 00:00:00 2001 From: sakisv Date: Fri, 10 Sep 2021 10:03:09 +0300 Subject: [PATCH 2/3] Fix tests Specifically, no longer test for a p1 zendesk when sending an alert and drop misleading "p1" from test name when cancelling an alert. We're no longer creating a P1 from the code, but we _do_ create a zendesk ticket when sending out an alert. When cancelling, what we want to test is that we don't create a second ticket when the alert is cancelled. --- tests/app/celery/test_broadcast_message_tasks.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 5411419a5..7d9a32289 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -99,7 +99,7 @@ def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabl ({'names': ['England', 'Scotland']}, ['England', 'Scotland']), ({}, []) ]) -def test_send_broadcast_event_creates_zendesk_p1( +def test_send_broadcast_event_creates_zendesk( area_data, expected_message, mocker, @@ -122,7 +122,6 @@ def test_send_broadcast_event_creates_zendesk_p1( assert mock_create_ticket.call_count == 1 zendesk_args = mock_create_ticket.call_args[1] - assert zendesk_args['p1'] is True assert zendesk_args['ticket_type'] == 'incident' assert str(broadcast_message.id) in zendesk_args['message'] @@ -132,7 +131,7 @@ def test_send_broadcast_event_creates_zendesk_p1( assert "Dear Sir/Madam" in zendesk_args['message'] -def test_send_broadcast_event_doesnt_p1_when_cancelling(mocker, notify_api, sample_broadcast_service): +def test_send_broadcast_event_doesnt_create_zendesk_when_cancelling(mocker, notify_api, sample_broadcast_service): template = create_template(sample_broadcast_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( template, From df739d6b948fbd8576035339336cf9dc257a5c20 Mon Sep 17 00:00:00 2001 From: sakisv Date: Thu, 9 Sep 2021 17:13:59 +0300 Subject: [PATCH 3/3] Fix flake8 --- app/celery/broadcast_message_tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index ddffd8a2a..d18826441 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -58,8 +58,8 @@ def check_event_is_authorised_to_be_sent(broadcast_event, provider): def check_event_makes_sense_in_sequence(broadcast_event, provider): """ If any previous event hasn't sent yet for that provider, then we shouldn't send the current event. Instead, fail and - raise a zendesk ticket - so that a notify team member can assess the state of the previous messages, and if necessary, can - replay the `send_broadcast_provider_message` task if the previous message has now been sent. + raise a zendesk ticket - so that a notify team member can assess the state of the previous messages, and if + necessary, can replay the `send_broadcast_provider_message` task if the previous message has now been sent. Note: This is called before the new broadcast_provider_message is created.