From d6a232ba8b7b35d6e56035499987dfd4501adf2a Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Fri, 12 Jan 2024 15:02:44 +0100 Subject: [PATCH] Add missing notification log records (#3664) Related to https://github.com/grafana/oncall-private/issues/2347 --- .../tests/tasks/test_new_alert_group.py | 40 +++++++++++++++++++ engine/apps/mobile_app/utils.py | 10 +++-- .../telegram/models/connectors/personal.py | 1 + .../telegram/tests/test_personal_connector.py | 39 +++++++++++++++++- 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py index f33e5c3d..362ab7dd 100644 --- a/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py +++ b/engine/apps/mobile_app/tests/tasks/test_new_alert_group.py @@ -92,6 +92,46 @@ def test_notify_user_about_new_alert_group_no_device_connected( ) +@patch("apps.mobile_app.utils.send_message_to_fcm_device", return_value=False) +@pytest.mark.django_db +def test_notify_user_about_new_alert_group_error_log_if_not_succeeded( + mock_send_message_to_fcm_device, + settings, + make_organization_and_user, + make_user_notification_policy, + make_alert_receive_channel, + make_channel_filter, + make_alert_group, + make_alert, +): + # create a user and connect a mobile device + organization, user = make_organization_and_user() + FCMDevice.objects.create(user=user, registration_id="test_device_id") + # set up notification policy and alert group + notification_policy = make_user_notification_policy( + user, + UserNotificationPolicy.Step.NOTIFY, + notify_by=MOBILE_APP_BACKEND_ID, + ) + alert_receive_channel = make_alert_receive_channel(organization=organization) + channel_filter = make_channel_filter(alert_receive_channel) + alert_group = make_alert_group(alert_receive_channel, channel_filter=channel_filter) + make_alert(alert_group=alert_group, raw_request_data={}) + # check FCM is contacted directly when using the cloud license + settings.LICENSE = CLOUD_LICENSE_NAME + settings.IS_OPEN_SOURCE = False + + notify_user_about_new_alert_group( + user_pk=user.pk, + alert_group_pk=alert_group.pk, + notification_policy_pk=notification_policy.pk, + critical=False, + ) + mock_send_message_to_fcm_device.assert_called_once() + log_record = alert_group.personal_log_records.last() + assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED + + @pytest.mark.django_db def test_fcm_message_user_settings( make_organization_and_user, make_alert_receive_channel, make_alert_group, make_alert diff --git a/engine/apps/mobile_app/utils.py b/engine/apps/mobile_app/utils.py index fbd4db3b..05915542 100644 --- a/engine/apps/mobile_app/utils.py +++ b/engine/apps/mobile_app/utils.py @@ -44,7 +44,7 @@ def _send_push_notification_to_fcm_relay(message: Message) -> requests.Response: return response -def send_message_to_fcm_device(device: "FCMDevice", message: Message) -> None: +def send_message_to_fcm_device(device: "FCMDevice", message: Message) -> bool: """ https://firebase.google.com/docs/cloud-messaging/http-server-ref#interpret-downstream """ @@ -60,9 +60,10 @@ def send_message_to_fcm_device(device: "FCMDevice", message: Message) -> None: if isinstance(response, FIREBASE_ERRORS_TO_NOT_RETRY): logger.warning(f"FCM error {response} is not being retried as we explicitly do not want to retry it") - return + return False raise response + return True def send_push_notification( @@ -97,7 +98,10 @@ def send_push_notification( else: raise else: - send_message_to_fcm_device(device_to_notify, message) + succeeded = send_message_to_fcm_device(device_to_notify, message) + if not succeeded: + _error_cb() + return False # notification succeeded (otherwise raised exception before) return True diff --git a/engine/apps/telegram/models/connectors/personal.py b/engine/apps/telegram/models/connectors/personal.py index 81a704dc..3034bf6f 100644 --- a/engine/apps/telegram/models/connectors/personal.py +++ b/engine/apps/telegram/models/connectors/personal.py @@ -158,6 +158,7 @@ class TelegramToUserConnector(models.Model): ) else: self._nudge_about_alert_group_message(telegram_client, old_alert_group_message) + TelegramToUserConnector.create_telegram_notification_success(alert_group, self.user, notification_policy) # send DM message with the link to the alert group post in channel def send_link_to_channel_message(self, alert_group: AlertGroup, notification_policy: UserNotificationPolicy): diff --git a/engine/apps/telegram/tests/test_personal_connector.py b/engine/apps/telegram/tests/test_personal_connector.py index 11b0edf8..00a2dd7d 100644 --- a/engine/apps/telegram/tests/test_personal_connector.py +++ b/engine/apps/telegram/tests/test_personal_connector.py @@ -5,7 +5,7 @@ from telegram import error from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord from apps.telegram.client import TelegramClient -from apps.telegram.models import TelegramMessage +from apps.telegram.models import TelegramMessage, TelegramToUserConnector @patch.object(TelegramClient, "send_raw_message", side_effect=error.BadRequest("Replied message not found")) @@ -188,6 +188,43 @@ def test_personal_connector_send_full_alert_group( assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS +@patch.object(TelegramToUserConnector, "_nudge_about_alert_group_message") +@pytest.mark.django_db +def test_personal_connector_send_full_alert_group_second_time( + mock_nudge_about_alert_group_message, + make_organization_and_user, + make_telegram_user_connector, + make_user_notification_policy, + make_alert_receive_channel, + make_alert_group, + make_alert, + make_telegram_message, +): + # set up a user with Telegram account connected + organization, user = make_organization_and_user() + connector = make_telegram_user_connector(user) + notification_policy = make_user_notification_policy( + user, + UserNotificationPolicy.Step.NOTIFY, + notify_by=UserNotificationPolicy.NotificationChannel.TELEGRAM, + important=False, + ) + + # create an alert group with an existing Telegram message in channel + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + make_alert(alert_group=alert_group, raw_request_data=alert_receive_channel.config.example_payload) + make_telegram_message( + alert_group=alert_group, + message_type=TelegramMessage.PERSONAL_MESSAGE, + chat_id=connector.telegram_chat_id, + ) + user.telegram_connection.send_full_alert_group(alert_group, notification_policy) + mock_nudge_about_alert_group_message.assert_called_once() + log_record = notification_policy.personal_log_records.last() + assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS + + @patch.object(TelegramClient, "send_message", side_effect=[error.BadRequest("error"), None]) @pytest.mark.django_db def test_personal_connector_send_full_alert_group_formatting_error(