Skip to content

Commit

Permalink
Address Telegram HTTP 500s when receiving message from Telegram in di…
Browse files Browse the repository at this point in the history
…scussion group (#3622)

# Which issue(s) this PR fixes

Closes #3621

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [ ] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
  • Loading branch information
joeyorlando authored Jan 9, 2024
1 parent 72e7224 commit 4cc4099
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
114 changes: 114 additions & 0 deletions engine/apps/telegram/tests/test_webhook.py
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions engine/apps/telegram/updates/update_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ def _update_entity_names(cls, update: Update) -> None:

@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

Expand Down
4 changes: 3 additions & 1 deletion engine/apps/telegram/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from .views import WebHookView

app_name = "telegram"

urlpatterns = [
path("", WebHookView.as_view()),
path("", WebHookView.as_view(), name="incoming_webhook"),
]
14 changes: 0 additions & 14 deletions engine/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
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 (
Expand Down Expand Up @@ -443,19 +442,6 @@ def _make_slack_message(alert_group=None, organization=None, **kwargs):
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):
Expand Down

0 comments on commit 4cc4099

Please sign in to comment.