diff --git a/CHANGELOG.md b/CHANGELOG.md index 81904255..eb48d288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Do not retry `firebase.messaging.UnregisteredError` exceptions for FCM relay tasks by @joeyorlando ([#3637](https://github.com/grafana/oncall/pull/3637)) +### Fixed + +- Address HTTP 500s occurring when receiving messages from Telegram user in a discussion group by @joeyorlando ([#3622](https://github.com/grafana/oncall/pull/3622)) + ## v1.3.83 (2024-01-08) ### Changed diff --git a/engine/apps/telegram/tests/test_webhook.py b/engine/apps/telegram/tests/test_webhook.py new file mode 100644 index 00000000..db73b5a7 --- /dev/null +++ b/engine/apps/telegram/tests/test_webhook.py @@ -0,0 +1,114 @@ +from unittest.mock import patch + +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +TELEGRAM_USER_ID = 777000 +TELEGRAM_USER_NAME = "Telegram" +TELEGRAM_USER = {"from_user_id": TELEGRAM_USER_ID, "from_user_name": TELEGRAM_USER_NAME} + +NON_TELEGRAM_USER_ID = 123456789 +NON_TELEGRAM_USER_NAME = "Johnny Appleseed" +NON_TELEGRAM_USER = {"from_user_id": NON_TELEGRAM_USER_ID, "from_user_name": NON_TELEGRAM_USER_NAME} + +CHANNEL_NAME = "Testing Testing Testing" +DISCUSSION_GROUP_NAME = "joey oncall testing" + + +def _base_message(from_user_id, from_user_name): + return { + "update_id": 822154680, + "message": { + "new_chat_members": [], + "sender_chat": { + "type": "channel", + "title": "Asteroid", + "id": -1001672798904, + }, + "text": "foo bar baz", + "channel_chat_created": False, + "group_chat_created": False, + "delete_chat_photo": False, + "new_chat_photo": [], + "forward_date": 1704305084, + "photo": [], + "caption_entities": [], + "chat": { + "type": "supergroup", + "title": CHANNEL_NAME, + "id": -1001760329920, + }, + "is_automatic_forward": True, + "entities": [], + "message_id": 173, + "supergroup_chat_created": False, + "date": 1704350566, + "from": { + "is_bot": False, + "first_name": from_user_name, + "id": from_user_id, + }, + }, + } + + +@pytest.fixture +def make_channel_message_webhook_payload(): + def _make_channel_message_webhook_payload(from_user_id, from_user_name): + return _base_message(from_user_id, from_user_name) | { + "forward_from": { + "is_bot": False, + "username": "foobarbaz", + "first_name": from_user_name, + "is_premium": True, + "id": 352445997, + }, + } + + return _make_channel_message_webhook_payload + + +@pytest.fixture +def make_discussion_group_message_webhook_payload(): + def _make_discussion_group_message_webhook_payload(from_user_id, from_user_name): + return _base_message(from_user_id, from_user_name) | { + "forward_from_message_id": 6, + "forward_signature": from_user_name, + "forward_from_chat": { + "type": "channel", + "id": -1002035090491, + "title": DISCUSSION_GROUP_NAME, + }, + } + + return _make_discussion_group_message_webhook_payload + + +@pytest.mark.django_db +@patch("apps.telegram.updates.update_handlers.channel_to_group_forward.TelegramClient") +@pytest.mark.parametrize("from_user", [TELEGRAM_USER, NON_TELEGRAM_USER]) +def test_we_can_successfully_receive_an_update_for_a_message_within_a_channel( + _MockTelegramClient, + make_channel_message_webhook_payload, + from_user, +): + url = reverse("telegram:incoming_webhook") + client = APIClient() + response = client.post(url, make_channel_message_webhook_payload(**from_user), format="json") + assert response.status_code == status.HTTP_200_OK + + +@pytest.mark.django_db +@patch("apps.telegram.updates.update_handlers.channel_to_group_forward.TelegramClient") +@pytest.mark.parametrize("from_user", [TELEGRAM_USER, NON_TELEGRAM_USER]) +def test_we_can_successfully_receive_an_update_for_a_message_within_a_channels_discussion_group( + _MockTelegramClient, + make_discussion_group_message_webhook_payload, + from_user, +): + url = reverse("telegram:incoming_webhook") + client = APIClient() + response = client.post(url, make_discussion_group_message_webhook_payload(**from_user), format="json") + assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/telegram/updates/update_manager.py b/engine/apps/telegram/updates/update_manager.py index c69401df..a5887a53 100644 --- a/engine/apps/telegram/updates/update_manager.py +++ b/engine/apps/telegram/updates/update_manager.py @@ -59,6 +59,14 @@ class UpdateManager: @staticmethod def _update_channel_and_group_names(update: Update) -> None: + """ + some updates may not necessarily come from the channel + (in which case they would contain the `forward_from_chat` object). Some updates may come directly from the + discussion group, in which case `forward_from_chat` is not present + """ + if update.message.forward_from_chat is None: + return + channel_chat_id = update.message.forward_from_chat.id channel_name = update.message.forward_from_chat.title diff --git a/engine/apps/telegram/urls.py b/engine/apps/telegram/urls.py index a1bb29e6..f6183f94 100644 --- a/engine/apps/telegram/urls.py +++ b/engine/apps/telegram/urls.py @@ -2,6 +2,8 @@ from django.urls import path from .views import WebHookView +app_name = "telegram" + urlpatterns = [ - path("", WebHookView.as_view()), + path("", WebHookView.as_view(), name="incoming_webhook"), ] diff --git a/engine/conftest.py b/engine/conftest.py index fa5ae281..74569268 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -12,7 +12,6 @@ from django.db.models.signals import post_save from django.urls import clear_url_caches from django.utils import timezone from pytest_factoryboy import register -from rest_framework.test import APIClient from telegram import Bot from apps.alerts.models import ( @@ -443,19 +442,6 @@ def make_slack_message(): return _make_slack_message -@pytest.fixture -def client_with_user(): - def _client_with_user(user): - """The client with logged in user""" - - client = APIClient() - client.force_login(user) - - return client - - return _client_with_user - - @pytest.fixture def make_team(): def _make_team(organization, **kwargs):