Merge pull request #3042 from grafana/dev

v1.3.38

CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+## v1.3.38 (2023-09-19)
+
+### Fixed
+
+- Fix Slack access token length issue by @toolchainX ([#3016](https://github.com/grafana/oncall/pull/3016))
+- Fix shifts for current user internal endpoint to return the right shift PK ([#3036](https://github.com/grafana/oncall/pull/3036))
+- Handle Slack ratelimit on alert group deletion by @vadimkerr ([#3038](https://github.com/grafana/oncall/pull/3038))
+
 ## v1.3.37 (2023-09-12)
 
 ### Added

dev/README.md

@@ -1,6 +1,6 @@
 # Developer quickstart
 
-- [Developing using kubernetes (beta)](#developing-using-kubernetes-beta)
+- [Developing using kubernetes and tilt (beta)](#developing-using-kubernetes-and-tilt-beta)
 - [Running the project with docker-compose](#running-the-project-with-docker-compose)
   - [`COMPOSE_PROFILES`](#compose_profiles)
   - [`GRAFANA_IMAGE`](#grafana_image)
@@ -28,7 +28,9 @@
 
 Related: [How to develop integrations](/engine/config_integrations/README.md)
 
-## Developing using kubernetes (beta)
+## Developing using kubernetes and tilt (beta)
+
+> It is recommended to use alternative [docker-compose environment](#running-the-project-with-docker-compose).
 
 ### Install
 
@@ -37,26 +39,31 @@ This project uses:
 - [Tilt | Kubernetes for Prod, Tilt for Dev](https://tilt.dev/)
 - [tilt-dev/ctlptl: Making local Kubernetes clusters fun and easy to set up](https://github.com/tilt-dev/ctlptl)
 - [Kind](https://kind.sigs.k8s.io)
+- [Yarn](https://classic.yarnpkg.com/lang/en/docs/install/#mac-stable)
 
 ### Quick Start
 
-Create local k8s cluster:
+1. Create local k8s cluster:
 
-```bash
-make cluster/up
-```
+    ```bash
+    make cluster/up
+    ```
 
-Deploy the project:
+2. Deploy the project:
 
-```bash
-tilt up
-```
+    ```bash
+    tilt up
+    ```
 
-Clean up the project by deleting the local k8s cluster:
+3. Wait until all resources are green and open <http://localhost:3000/a/grafana-oncall-app>
 
-```bash
-make cluster/down
-```
+4. Modify source code, backend and frontend will be hot reloaded
+
+5. Clean up the project by deleting the local k8s cluster:
+
+    ```bash
+    make cluster/down
+    ```
 
 ## Running the project with docker-compose
 

docs/make-docs

@@ -6,6 +6,12 @@
 # [Semantic versioning](https://semver.org/) is used to help the reader identify the significance of changes.
 # Changes are relevant to this script and the support docs.mk GNU Make interface.
 
+# ## 4.2.1 (2023-09-13)
+
+# ## Fixed
+
+# - Improved consistency of the webserver request loop by polling the Hugo port rather than the proxy port.
+
 # ## 4.2.0 (2023-09-01)
 
 # ### Added
@@ -469,7 +475,7 @@ POSIX_HERESTRING
         fi
       done
       echo
-      echo 'Press Ctrl+C to stop the server'
+      echo 'Press Ctrl+c to stop the server'
 
       unset i max req url
       return
@@ -478,6 +484,8 @@ POSIX_HERESTRING
 
   echo
   errr 'The build was interrupted or a build error occurred, check the previous logs for possible causes.'
+  note 'You might need to use Ctrl+c to end the process.'
+
   unset i max req url
 }
 
@@ -626,7 +634,7 @@ ${PODMAN} run \
   ${DOCS_IMAGE} \
   /entrypoint
 EOF
-    await_build http://localhost:3002 &
+    await_build http://localhost:3003 &
 
     if [ -n "${DEBUG}" ]; then
       ${cmd}

engine/apps/alerts/tasks/delete_alert_group.py

@@ -1,6 +1,7 @@
 from celery.utils.log import get_task_logger
 from django.conf import settings
 
+from apps.slack.errors import SlackAPIRatelimitError
 from common.custom_celery_tasks import shared_dedicated_queue_retry_task
 
 logger = get_task_logger(__name__)
@@ -23,4 +24,8 @@ def delete_alert_group(alert_group_pk, user_pk):
         logger.debug("User not found, skipping delete_alert_group")
         return
 
-    alert_group.delete_by_user(user)
+    try:
+        alert_group.delete_by_user(user)
+    except SlackAPIRatelimitError as e:
+        # Handle Slack API ratelimit raised in apps.slack.scenarios.distribute_alerts.DeleteGroupStep.process_signal
+        delete_alert_group.apply_async((alert_group_pk, user_pk), countdown=e.retry_after)

engine/apps/alerts/tests/test_alert_group.py

@@ -1,11 +1,14 @@
-from unittest.mock import patch
+from unittest.mock import call, patch
 
 import pytest
 
 from apps.alerts.incident_appearance.renderers.phone_call_renderer import AlertGroupPhoneCallRenderer
 from apps.alerts.models import AlertGroup, AlertGroupLogRecord
 from apps.alerts.tasks.delete_alert_group import delete_alert_group
+from apps.slack.client import SlackClient
+from apps.slack.errors import SlackAPIMessageNotFoundError, SlackAPIRatelimitError
 from apps.slack.models import SlackMessage
+from apps.slack.tests.conftest import build_slack_response
 
 
 @pytest.mark.django_db
@@ -46,56 +49,180 @@ def test_render_for_phone_call(
     assert expected_verbose_name in rendered_text
 
 
+@patch.object(SlackClient, "reactions_remove")
+@patch.object(SlackClient, "chat_delete")
 @pytest.mark.django_db
 def test_delete(
+    mock_chat_delete,
+    mock_reactions_remove,
     make_organization_with_slack_team_identity,
     make_user,
-    make_slack_channel,
     make_alert_receive_channel,
     make_alert_group,
     make_alert,
+    make_slack_message,
+    make_resolution_note_slack_message,
 ):
     """test alert group deleting"""
 
     organization, slack_team_identity = make_organization_with_slack_team_identity()
-    slack_channel = make_slack_channel(slack_team_identity, name="general", slack_id="CWER1ASD")
     user = make_user(organization=organization)
 
     alert_receive_channel = make_alert_receive_channel(organization)
 
     alert_group = make_alert_group(alert_receive_channel)
-    SlackMessage.objects.create(channel_id="CWER1ASD", alert_group=alert_group)
+    make_alert(alert_group, raw_request_data={})
 
-    make_alert(
-        alert_group,
-        raw_request_data={
-            "evalMatches": [
-                {"value": 100, "metric": "High value", "tags": None},
-                {"value": 200, "metric": "Higher Value", "tags": None},
-            ],
-            "message": "Someone is testing the alert notification within grafana.",
-            "ruleId": 0,
-            "ruleName": "Test notification",
-            "ruleUrl": "http://localhost:3000/",
-            "state": "alerting",
-            "title": f"Incident for channel <#{slack_channel.slack_id}> Where a > b & c < d",
-        },
+    # Create Slack messages
+    slack_message = make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+    resolution_note_1 = make_resolution_note_slack_message(
+        alert_group=alert_group,
+        user=user,
+        added_by_user=user,
+        posted_by_bot=True,
+        slack_channel_id="test1_channel_id",
+        ts="test1_ts",
+    )
+    resolution_note_2 = make_resolution_note_slack_message(
+        alert_group=alert_group,
+        user=user,
+        added_by_user=user,
+        added_to_resolution_note=True,
+        slack_channel_id="test2_channel_id",
+        ts="test2_ts",
     )
 
-    alerts = alert_group.alerts
-    slack_messages = alert_group.slack_messages
-
-    assert alerts.count() > 0
-    assert slack_messages.count() > 0
+    assert alert_group.alerts.count() == 1
+    assert alert_group.slack_messages.count() == 1
+    assert alert_group.resolution_note_slack_messages.count() == 2
 
     delete_alert_group(alert_group.pk, user.pk)
 
-    assert alerts.count() == 0
-    assert slack_messages.count() == 0
+    assert not alert_group.alerts.exists()
+    assert not alert_group.slack_messages.exists()
+    assert not alert_group.resolution_note_slack_messages.exists()
 
     with pytest.raises(AlertGroup.DoesNotExist):
         alert_group.refresh_from_db()
 
+    # Check that appropriate Slack API calls are made
+    assert mock_chat_delete.call_count == 2
+    assert mock_chat_delete.call_args_list[0] == call(
+        channel=resolution_note_1.slack_channel_id, ts=resolution_note_1.ts
+    )
+    assert mock_chat_delete.call_args_list[1] == call(channel=slack_message.channel_id, ts=slack_message.slack_id)
+    mock_reactions_remove.assert_called_once_with(
+        channel=resolution_note_2.slack_channel_id, name="memo", timestamp=resolution_note_2.ts
+    )
+
+
+@pytest.mark.parametrize("api_method", ["reactions_remove", "chat_delete"])
+@patch.object(delete_alert_group, "apply_async")
+@pytest.mark.django_db
+def test_delete_slack_ratelimit(
+    mock_delete_alert_group,
+    api_method,
+    make_organization_with_slack_team_identity,
+    make_user,
+    make_alert_receive_channel,
+    make_alert_group,
+    make_alert,
+    make_slack_message,
+    make_resolution_note_slack_message,
+):
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
+    user = make_user(organization=organization)
+
+    alert_receive_channel = make_alert_receive_channel(organization)
+
+    alert_group = make_alert_group(alert_receive_channel)
+    make_alert(alert_group, raw_request_data={})
+
+    # Create Slack messages
+    make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+    make_resolution_note_slack_message(
+        alert_group=alert_group,
+        user=user,
+        added_by_user=user,
+        posted_by_bot=True,
+        slack_channel_id="test1_channel_id",
+        ts="test1_ts",
+    )
+    make_resolution_note_slack_message(
+        alert_group=alert_group,
+        user=user,
+        added_by_user=user,
+        added_to_resolution_note=True,
+        slack_channel_id="test2_channel_id",
+        ts="test2_ts",
+    )
+
+    with patch.object(
+        SlackClient,
+        api_method,
+        side_effect=SlackAPIRatelimitError(
+            response=build_slack_response({"ok": False, "error": "ratelimited"}, headers={"Retry-After": 42})
+        ),
+    ):
+        delete_alert_group(alert_group.pk, user.pk)
+
+    # Check task is retried gracefully
+    mock_delete_alert_group.assert_called_once_with((alert_group.pk, user.pk), countdown=42)
+
+
+@pytest.mark.parametrize("api_method", ["reactions_remove", "chat_delete"])
+@patch.object(delete_alert_group, "apply_async")
+@pytest.mark.django_db
+def test_delete_slack_api_error_other_than_ratelimit(
+    mock_delete_alert_group,
+    api_method,
+    make_organization_with_slack_team_identity,
+    make_user,
+    make_alert_receive_channel,
+    make_alert_group,
+    make_alert,
+    make_slack_message,
+    make_resolution_note_slack_message,
+):
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
+    user = make_user(organization=organization)
+
+    alert_receive_channel = make_alert_receive_channel(organization)
+
+    alert_group = make_alert_group(alert_receive_channel)
+    make_alert(alert_group, raw_request_data={})
+
+    # Create Slack messages
+    make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+    make_resolution_note_slack_message(
+        alert_group=alert_group,
+        user=user,
+        added_by_user=user,
+        posted_by_bot=True,
+        slack_channel_id="test1_channel_id",
+        ts="test1_ts",
+    )
+    make_resolution_note_slack_message(
+        alert_group=alert_group,
+        user=user,
+        added_by_user=user,
+        added_to_resolution_note=True,
+        slack_channel_id="test2_channel_id",
+        ts="test2_ts",
+    )
+
+    with patch.object(
+        SlackClient,
+        api_method,
+        side_effect=SlackAPIMessageNotFoundError(
+            response=build_slack_response({"ok": False, "error": "message_not_found"})
+        ),
+    ):
+        delete_alert_group(alert_group.pk, user.pk)  # check no exception is raised
+
+    # Check task is not retried
+    mock_delete_alert_group.assert_not_called()
+
 
 @pytest.mark.django_db
 def test_alerts_count_gt(

engine/apps/api/tests/test_schedules.py

@@ -2115,8 +2115,8 @@ def test_current_user_events(
 
     shifts = (
         # schedule, user, priority, start time (h), duration (seconds)
-        (schedule_with_current_user, current_user, 1, 0, (24 * 60 * 60) - 1),  # r1-1: 0-23:59:59
         (other_schedule, other_user, 1, 0, (24 * 60 * 60) - 1),  # r1-1: 0-23:59:59
+        (schedule_with_current_user, current_user, 1, 0, (24 * 60 * 60) - 1),  # r1-1: 0-23:59:59
     )
     now = timezone.now()
     today = now.replace(hour=0, minute=0, second=0, microsecond=0)
@@ -2146,6 +2146,9 @@ def test_current_user_events(
     assert result["schedules"][0]["id"] == schedule_with_current_user.public_primary_key
     assert result["schedules"][0]["name"] == schedule_with_current_user.name
     assert len(result["schedules"][0]["events"]) > 0
+    for event in result["schedules"][0]["events"]:
+        # check the current user shift pk is set in the event
+        assert event["shift"]["pk"] == on_call_shift.public_primary_key
 
 
 @pytest.mark.django_db

engine/apps/api_for_grafana_incident/serializers.py

@@ -2,7 +2,9 @@ import logging
 
 from rest_framework import serializers
 
+from apps.alerts.incident_appearance.renderers.web_renderer import AlertGroupWebRenderer
 from apps.alerts.models import Alert, AlertGroup
+from apps.api.serializers.alert_group import AlertGroupFieldsCacheSerializerMixin
 
 logger = logging.getLogger(__name__)
 
@@ -26,9 +28,22 @@ class AlertGroupSerializer(serializers.ModelSerializer):
     title = serializers.CharField(read_only=True, source="long_verbose_name_without_formatting")
     alerts = AlertSerializer(many=True, read_only=True)
 
+    render_for_web = serializers.SerializerMethodField()
+
     def get_status(self, obj):
         return next(filter(lambda status: status[0] == obj.status, AlertGroup.STATUS_CHOICES))[1].lower()
 
+    def get_render_for_web(self, obj):
+        last_alert = obj.alerts.last()
+        if last_alert is None:
+            return {}
+        return AlertGroupFieldsCacheSerializerMixin.get_or_set_web_template_field(
+            obj,
+            last_alert,
+            AlertGroupFieldsCacheSerializerMixin.RENDER_FOR_WEB_FIELD_NAME,
+            AlertGroupWebRenderer,
+        )
+
     class Meta:
         model = AlertGroup
         fields = [
@@ -37,4 +52,5 @@ class AlertGroupSerializer(serializers.ModelSerializer):
             "status",
             "alerts",
             "title",
+            "render_for_web",
         ]

engine/apps/api_for_grafana_incident/tests/test_views.py

@@ -0,0 +1,50 @@
+import pytest
+from django.urls import reverse
+from rest_framework.test import APIClient
+
+
+@pytest.mark.django_db
+def test_alert_group_details(
+    make_organization,
+    make_alert_receive_channel,
+    make_alert_group,
+    make_alert,
+    settings,
+):
+    settings.GRAFANA_INCIDENT_STATIC_API_KEY = "test-key"
+    headers = {"HTTP_AUTHORIZATION": "test-key"}
+    organization = make_organization()
+    alert_receive_channel = make_alert_receive_channel(
+        organization,
+        slack_title_template=None,
+        web_title_template="title: {{ payload.field2 }}",
+        web_message_template="Something {{ payload.field1 }} + {{ payload.field3 }}",
+        web_image_url_template="http://{{ payload.field1 }}",
+    )
+    alert_group = make_alert_group(alert_receive_channel)
+    alert_payload = {"field1": "foo", "field2": "bar", "field3": "baz"}
+    alert = make_alert(alert_group, alert_payload)
+
+    client = APIClient()
+
+    url = reverse("api-gi:alert-groups-detail", kwargs={"public_primary_key": alert_group.public_primary_key})
+    response = client.get(url, format="json", **headers)
+    expected = {
+        "id": alert_group.public_primary_key,
+        "link": alert_group.web_link,
+        "status": "new",
+        "title": alert_group.long_verbose_name_without_formatting,
+        "alerts": [
+            {
+                "id_oncall": alert.public_primary_key,
+                "payload": alert_payload,
+            }
+        ],
+        "render_for_web": {
+            "title": "title: bar",
+            "message": "<p>Something foo + baz</p>",
+            "image_url": "http://foo",
+            "source_link": None,
+        },
+    }
+    assert response.json() == expected

engine/apps/schedules/constants.py

@@ -18,6 +18,7 @@ ICAL_STATUS = "STATUS"
 ICAL_STATUS_CANCELLED = "CANCELLED"
 ICAL_COMPONENT_VEVENT = "VEVENT"
 RE_PRIORITY = re.compile(r"^\[L(\d+)\]")
+RE_EVENT_UID_EXPORT = re.compile(r"([\w\d]+)-(\d+)-([\w\d]+)")
 RE_EVENT_UID_V1 = re.compile(r"amixr-([\w\d-]+)-U(\d+)-E(\d+)-S(\d+)")
 RE_EVENT_UID_V2 = re.compile(r"oncall-([\w\d-]+)-PK([\w\d]+)-U(\d+)-E(\d+)-S(\d+)")
 

engine/apps/schedules/ical_utils.py

@@ -29,6 +29,7 @@ from apps.schedules.constants import (
     ICAL_STATUS_CANCELLED,
     ICAL_SUMMARY,
     ICAL_UID,
+    RE_EVENT_UID_EXPORT,
     RE_EVENT_UID_V1,
     RE_EVENT_UID_V2,
     RE_PRIORITY,
@@ -421,21 +422,25 @@ def parse_event_uid(string: str, sequence: str = None, recurrence_id: str = None
     if match:
         _, pk, _, _, source = match.groups()
     else:
-        # eventually this path would be automatically deprecated
-        # once all ical representations are refreshed
-        match = RE_EVENT_UID_V1.match(string)
+        match = RE_EVENT_UID_EXPORT.match(string)
         if match:
-            _, _, _, source = match.groups()
+            pk, _, _ = match.groups()
         else:
-            # fallback to use the UID string as the rotation ID
-            pk = string
-            # in ical imported calendars, sequence and/or recurrence_id
-            # distinguish main recurring event vs instance modification
-            # (see https://icalendar.org/iCalendar-RFC-5545/3-8-4-4-recurrence-id.html)
-            if sequence:
-                pk = f"{pk}_{sequence}"
-            if recurrence_id:
-                pk = f"{pk}_{recurrence_id}"
+            # eventually this path would be automatically deprecated
+            # once all ical representations are refreshed
+            match = RE_EVENT_UID_V1.match(string)
+            if match:
+                _, _, _, source = match.groups()
+            else:
+                # fallback to use the UID string as the rotation ID
+                pk = string
+                # in ical imported calendars, sequence and/or recurrence_id
+                # distinguish main recurring event vs instance modification
+                # (see https://icalendar.org/iCalendar-RFC-5545/3-8-4-4-recurrence-id.html)
+                if sequence:
+                    pk = f"{pk}_{sequence}"
+                if recurrence_id:
+                    pk = f"{pk}_{recurrence_id}"
 
     if source is not None:
         source = int(source)

engine/apps/schedules/tests/test_ical_utils.py

@@ -317,6 +317,15 @@ def test_shifts_dict_from_cached_final(
     assert shifts == expected_events
 
 
+def test_parse_event_uid_from_export():
+    shift_pk = "OUCE6WAHL35PP"
+    user_pk = "UHZ38D6AQXXBY"
+    event_uid = f"{shift_pk}-202309200300-{user_pk}"
+    pk, source = parse_event_uid(event_uid)
+    assert pk == shift_pk
+    assert source is None
+
+
 def test_parse_event_uid_v1():
     uuid = uuid4()
     event_uid = f"amixr-{uuid}-U1-E2-S1"

engine/apps/slack/migrations/0004_auto_20230913_1020.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.2.20 on 2023-09-13 10:20
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('slack', '0003_delete_slackactionrecord'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='slackteamidentity',
+            name='access_token',
+            field=models.CharField(default=None, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='slackteamidentity',
+            name='bot_access_token',
+            field=models.CharField(default=None, max_length=255, null=True),
+        ),
+    ]

engine/apps/slack/models/slack_team_identity.py

@@ -31,9 +31,9 @@ class SlackTeamIdentity(models.Model):
     slack_id = models.CharField(max_length=100)
     cached_name = models.CharField(max_length=100, null=True, default=None)
     cached_app_id = models.CharField(max_length=100, null=True, default=None)
-    access_token = models.CharField(max_length=100, null=True, default=None)
+    access_token = models.CharField(max_length=255, null=True, default=None)
     bot_user_id = models.CharField(max_length=100, null=True, default=None)
-    bot_access_token = models.CharField(max_length=100, null=True, default=None)
+    bot_access_token = models.CharField(max_length=255, null=True, default=None)
     oauth_scope = models.TextField(max_length=30000, null=True, default=None)
     detected_token_revoked = models.DateTimeField(null=True, default=None, verbose_name="Deleted At")
     is_profile_populated = models.BooleanField(default=False)

engine/apps/slack/scenarios/distribute_alerts.py

@@ -880,34 +880,38 @@ class DeleteGroupStep(scenario_step.ScenarioStep):
     def process_signal(self, log_record: AlertGroupLogRecord) -> None:
         alert_group = log_record.alert_group
 
-        self.remove_resolution_note_reaction(alert_group)
-
-        bot_messages_ts: typing.List[str] = []
-        bot_messages_ts.extend(alert_group.slack_messages.values_list("slack_id", flat=True))
-        bot_messages_ts.extend(
-            alert_group.resolution_note_slack_messages.filter(posted_by_bot=True).values_list("ts", flat=True)
-        )
-        channel_id = alert_group.slack_message.channel_id
-
-        for message_ts in bot_messages_ts:
-            try:
-                self._slack_client.chat_delete(channel=channel_id, ts=message_ts)
-            except (
-                SlackAPITokenError,
-                SlackAPIChannelNotFoundError,
-                SlackAPIMessageNotFoundError,
-                SlackAPIChannelArchivedError,
-            ):
-                pass
-
-    def remove_resolution_note_reaction(self, alert_group: AlertGroup) -> None:
+        # Remove "memo" emoji from resolution note messages
         for message in alert_group.resolution_note_slack_messages.filter(added_to_resolution_note=True):
-            message.added_to_resolution_note = False
-            message.save(update_fields=["added_to_resolution_note"])
             try:
                 self._slack_client.reactions_remove(channel=message.slack_channel_id, name="memo", timestamp=message.ts)
+            except SlackAPIRatelimitError:
+                # retries on ratelimit are handled in apps.alerts.tasks.delete_alert_group.delete_alert_group
+                raise
             except SlackAPIError:
                 pass
+            message.delete()
+
+        # Remove resolution note messages posted by OnCall bot
+        for message in alert_group.resolution_note_slack_messages.filter(posted_by_bot=True):
+            try:
+                self._slack_client.chat_delete(channel=message.slack_channel_id, ts=message.ts)
+            except SlackAPIRatelimitError:
+                # retries on ratelimit are handled in apps.alerts.tasks.delete_alert_group.delete_alert_group
+                raise
+            except SlackAPIError:
+                pass
+            message.delete()
+
+        # Remove alert group Slack messages
+        for message in alert_group.slack_messages.all():
+            try:
+                self._slack_client.chat_delete(channel=message.channel_id, ts=message.slack_id)
+            except SlackAPIRatelimitError:
+                # retries on ratelimit are handled in apps.alerts.tasks.delete_alert_group.delete_alert_group
+                raise
+            except SlackAPIError:
+                pass
+            message.delete()
 
 
 class UpdateLogReportMessageStep(scenario_step.ScenarioStep):

engine/apps/telegram/models/connectors/personal.py

@@ -4,11 +4,12 @@ from telegram import error
 from apps.alerts.models import AlertGroup
 from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord
 from apps.telegram.client import TelegramClient
+from apps.telegram.decorators import ignore_reply_to_message_deleted
 from apps.telegram.models import TelegramMessage, TelegramToOrganizationConnector
 from apps.telegram.tasks import send_link_to_channel_message_or_fallback_to_full_alert_group
 from apps.user_management.models import User
 
-ONE_MORE_NOTIFICATION = "One more notification about this ☝"
+ONE_MORE_NOTIFICATION = "One more notification about this 👆"
 ALERT_CANT_BE_RENDERED = (
     "You have a new alert group, but Telegram can't render its content! Please check it out: {link}"
 )
@@ -123,11 +124,7 @@ class TelegramToUserConnector(models.Model):
                 else:
                     raise e
         else:
-            telegram_client.send_raw_message(
-                chat_id=old_alert_group_message.chat_id,
-                text=ONE_MORE_NOTIFICATION,
-                reply_to_message_id=old_alert_group_message.message_id,
-            )
+            self._nudge_about_alert_group_message(telegram_client, old_alert_group_message)
 
     # 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):
@@ -172,3 +169,11 @@ class TelegramToUserConnector(models.Model):
                 )
             else:
                 raise e
+
+    @staticmethod
+    @ignore_reply_to_message_deleted
+    def _nudge_about_alert_group_message(telegram_client: TelegramClient, message: TelegramMessage) -> None:
+        """Nudge the user about existing alert group message, without sending the full alert group content again."""
+        telegram_client.send_raw_message(
+            chat_id=message.chat_id, reply_to_message_id=message.message_id, text=ONE_MORE_NOTIFICATION
+        )

engine/apps/telegram/tests/test_personal_connector.py

@@ -0,0 +1,49 @@
+from unittest.mock import patch
+
+import pytest
+from telegram import error
+
+from apps.base.models import UserNotificationPolicy
+from apps.telegram.client import TelegramClient
+from apps.telegram.models import TelegramMessage
+
+
+@patch.object(TelegramClient, "send_raw_message", side_effect=error.BadRequest("Replied message not found"))
+@pytest.mark.django_db
+def test_personal_connector_replied_message_not_found(
+    mock_send_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()
+    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 user's DM
+    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)
+    telegram_message = make_telegram_message(
+        alert_group=alert_group,
+        message_type=TelegramMessage.PERSONAL_MESSAGE,
+        chat_id=str(user.telegram_connection.telegram_chat_id),
+    )
+
+    # make sure no exception is raised when replying to the message that has been deleted
+    user.telegram_connection.notify(alert_group=alert_group, notification_policy=notification_policy)
+    mock_send_message.assert_called_once_with(
+        chat_id=telegram_message.chat_id,
+        text="One more notification about this 👆",
+        reply_to_message_id=telegram_message.message_id,
+    )