From 1b4cca9d03cf56f2f40280695bde0354578e35df Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 22 Nov 2024 11:45:03 +0800 Subject: [PATCH 1/6] Remove AM status from title (#5261) --- engine/config_integrations/alertmanager.py | 2 +- engine/config_integrations/grafana_alerting.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/config_integrations/alertmanager.py b/engine/config_integrations/alertmanager.py index 5b8535b275..9d1b3d6f05 100644 --- a/engine/config_integrations/alertmanager.py +++ b/engine/config_integrations/alertmanager.py @@ -31,7 +31,7 @@ {% set alertname = groupLabels.pop("alertname", "") -%} {% endif -%} -[{{ payload.status }}{% if payload.status == 'firing' and payload.numFiring %}:{{ payload.numFiring }}{% endif %}] {{ alertname }} {% if groupLabels | length > 0 %}({{ groupLabels.values()|join(", ") }}){% endif %} +{{ alertname }} {% if groupLabels | length > 0 %}({{ groupLabels.values()|join(", ") }}){% endif %} """ # noqa web_message = """\ diff --git a/engine/config_integrations/grafana_alerting.py b/engine/config_integrations/grafana_alerting.py index a53ddccffa..c808e0142d 100644 --- a/engine/config_integrations/grafana_alerting.py +++ b/engine/config_integrations/grafana_alerting.py @@ -33,7 +33,7 @@ {% set alertname = groupLabels.pop("alertname", "") -%} {% endif -%} -[{{ payload.status }}{% if payload.status == 'firing' and payload.numFiring %}:{{ payload.numFiring }}{% endif %}] {{ alertname }} {% if groupLabels | length > 0 %}({{ groupLabels.values()|join(", ") }}){% endif %} +{{ alertname }} {% if groupLabels | length > 0 %}({{ groupLabels.values()|join(", ") }}){% endif %} """ # noqa web_message = """\ From 03ff4c542672c35c44b85d812e5c9e4c7f428612 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 22 Nov 2024 12:34:40 +0800 Subject: [PATCH 2/6] Listen for alert group actions during whole notification call (#5282) 1. Wrap whole message in twiml - that's an actual fix 2. Use twilio helper lib to build twiml queries 3. URLencode twimlquery only once before making a call to reduce code duplication. --------- Co-authored-by: Joey Orlando --- engine/apps/twilioapp/gather.py | 14 +++++-- engine/apps/twilioapp/phone_provider.py | 42 +++++++++++-------- .../apps/twilioapp/tests/test_phone_calls.py | 6 +-- .../twilioapp/tests/test_twilio_provider.py | 12 ++++-- 4 files changed, 46 insertions(+), 28 deletions(-) diff --git a/engine/apps/twilioapp/gather.py b/engine/apps/twilioapp/gather.py index 8b4f3036b0..8afe4ec65e 100644 --- a/engine/apps/twilioapp/gather.py +++ b/engine/apps/twilioapp/gather.py @@ -24,16 +24,22 @@ def process_gather_data(call_sid: str, digit: str) -> VoiceResponse: response = VoiceResponse() + success_messages = { + "1": "Acknowledged", + "2": "Resolved", + "3": "Silenced", + } if digit in ["1", "2", "3"]: # Success case - response.say(f"You have pressed digit {digit}") + msg = success_messages.get(digit, f"You have pressed digit {digit}") + response.say(msg) process_digit(call_sid, digit) else: # Error wrong digit pressing gather = Gather(action=get_gather_url(), method="POST", num_digits=1) response.say("Wrong digit") - gather.say(get_gather_message()) + gather.say(get_alert_group_gather_instructions()) response.append(gather) @@ -85,5 +91,5 @@ def get_gather_url(): return create_engine_url(reverse("twilioapp:gather")) -def get_gather_message(): - return "Press 1 to acknowledge, 2 to resolve, 3 to silence to 30 minutes" +def get_alert_group_gather_instructions(): + return "Press 1 to acknowledge, 2 to resolve, 3 to silence for 30 minutes" diff --git a/engine/apps/twilioapp/phone_provider.py b/engine/apps/twilioapp/phone_provider.py index 988746f1c8..89811bad3e 100644 --- a/engine/apps/twilioapp/phone_provider.py +++ b/engine/apps/twilioapp/phone_provider.py @@ -6,6 +6,7 @@ from phonenumbers import COUNTRY_CODE_TO_REGION_CODE from twilio.base.exceptions import TwilioRestException from twilio.rest import Client +from twilio.twiml.voice_response import Gather, Say, VoiceResponse from apps.base.models import LiveSetting from apps.base.utils import live_settings @@ -16,7 +17,7 @@ FailedToStartVerification, ) from apps.phone_notifications.phone_provider import PhoneProvider, ProviderFlags -from apps.twilioapp.gather import get_gather_message, get_gather_url +from apps.twilioapp.gather import get_alert_group_gather_instructions, get_gather_url from apps.twilioapp.models import ( TwilioCallStatuses, TwilioPhoneCall, @@ -34,13 +35,13 @@ class TwilioPhoneProvider(PhoneProvider): def make_notification_call(self, number: str, message: str) -> TwilioPhoneCall | None: message = self._escape_call_message(message) - twiml_query = self._message_to_twiml(message, with_gather=True) + twiml = self._message_to_twiml_gather(message) response = None try_without_callback = False try: - response = self._call_create(twiml_query, number, with_callback=True) + response = self._call_create(twiml, number, with_callback=True) except TwilioRestException as e: # If status callback is not valid and not accessible from public url then trying to send message without it # https://www.twilio.com/docs/api/errors/21609 @@ -53,7 +54,7 @@ def make_notification_call(self, number: str, message: str) -> TwilioPhoneCall | if try_without_callback: try: - response = self._call_create(twiml_query, number, with_callback=False) + response = self._call_create(twiml, number, with_callback=False) except TwilioRestException as e: logger.error(f"TwilioPhoneProvider.make_notification_call: failed {e}") raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number)) @@ -146,9 +147,9 @@ def _get_graceful_msg(self, e, number): return None def make_call(self, number: str, message: str): - twiml_query = self._message_to_twiml(message, with_gather=False) + twiml = self._message_to_twiml_say(message) try: - self._call_create(twiml_query, number, with_callback=False) + self._call_create(twiml, number, with_callback=False) except TwilioRestException as e: logger.error(f"TwilioPhoneProvider.make_call: failed {e}") raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number)) @@ -160,18 +161,25 @@ def send_sms(self, number: str, message: str): logger.error(f"TwilioPhoneProvider.send_sms: failed {e}") raise FailedToSendSMS(graceful_msg=self._get_graceful_msg(e, number)) - def _message_to_twiml(self, message: str, with_gather=False): - q = f"{message}" - if with_gather: - gather_subquery = f'{get_gather_message()}' - q = f"{message}{gather_subquery}" - return urllib.parse.quote( - q, - safe="", - ) - - def _call_create(self, twiml_query: str, to: str, with_callback: bool): + def _message_to_twiml_say(self, message: str) -> VoiceResponse: + response = VoiceResponse() + say = Say(message) + response.append(say) + return response + + def _message_to_twiml_gather(self, message: str) -> VoiceResponse: + response = VoiceResponse() + gather = Gather(action=get_gather_url(), method="POST", num_digits=1) + gather.say(message) + gather.pause(length=1) + gather.say(get_alert_group_gather_instructions()) + response.append(gather) + return response + + def _call_create(self, twiml: VoiceResponse, to: str, with_callback: bool): client, from_ = self._phone_sender(to) + # encode twiml VoiceResponse to use in url + twiml_query = urllib.parse.quote(str(twiml), safe="") url = "http://twimlets.com/echo?Twiml=" + twiml_query if with_callback: status_callback = get_call_status_callback_url() diff --git a/engine/apps/twilioapp/tests/test_phone_calls.py b/engine/apps/twilioapp/tests/test_phone_calls.py index c66977e1f0..5a8d196631 100644 --- a/engine/apps/twilioapp/tests/test_phone_calls.py +++ b/engine/apps/twilioapp/tests/test_phone_calls.py @@ -138,7 +138,7 @@ def test_acknowledge_by_phone(mock_has_permission, mock_get_gather_url, make_twi content = response.content.decode("utf-8") assert response.status_code == 200 - assert "You have pressed digit 1" in content + assert "Acknowledged" in content alert_group.refresh_from_db() assert alert_group.acknowledged is True @@ -173,7 +173,7 @@ def test_resolve_by_phone(mock_has_permission, mock_get_gather_url, make_twilio_ content = BeautifulSoup(content, features="xml").findAll(string=True) assert response.status_code == 200 - assert "You have pressed digit 2" in content + assert "Resolved" in content alert_group.refresh_from_db() assert alert_group.resolved is True @@ -207,7 +207,7 @@ def test_silence_by_phone(mock_has_permission, mock_get_gather_url, make_twilio_ content = response.content.decode("utf-8") assert response.status_code == 200 - assert "You have pressed digit 3" in content + assert "Silenced" in content alert_group.refresh_from_db() assert alert_group.silenced_until is not None diff --git a/engine/apps/twilioapp/tests/test_twilio_provider.py b/engine/apps/twilioapp/tests/test_twilio_provider.py index 1d7384b635..5054c37559 100644 --- a/engine/apps/twilioapp/tests/test_twilio_provider.py +++ b/engine/apps/twilioapp/tests/test_twilio_provider.py @@ -19,13 +19,15 @@ class MockTwilioMessageInstance: @pytest.mark.django_db @mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._call_create", return_value=MockTwilioCallInstance()) -@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml", return_value="mocked_twiml") +@mock.patch( + "apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml_gather", return_value="twiml_gather_response" +) def test_make_notification_call(mock_twiml, mock_call_create): number = "+1234567890" message = "Hello" provider = TwilioPhoneProvider() provider_call = provider.make_notification_call(number, message) - mock_call_create.assert_called_once_with("mocked_twiml", number, with_callback=True) + mock_call_create.assert_called_once_with("twiml_gather_response", number, with_callback=True) assert provider_call is not None assert provider_call.sid == MockTwilioCallInstance.sid assert provider_call.id is None # test that provider_call is returned by notification call and NOT saved @@ -33,14 +35,16 @@ def test_make_notification_call(mock_twiml, mock_call_create): @pytest.mark.django_db @mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._call_create", return_value=MockTwilioCallInstance()) -@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml", return_value="mocked_twiml") +@mock.patch( + "apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml_say", return_value="twiml_say_response" +) def test_make_call(mock_twiml, mock_call_create): number = "+1234567890" message = "Hello" provider = TwilioPhoneProvider() provider_call = provider.make_call(number, message) assert provider_call is None # test that provider_call is not returned from make_call - mock_call_create.assert_called_once_with("mocked_twiml", number, with_callback=False) + mock_call_create.assert_called_once_with("twiml_say_response", number, with_callback=False) class MockTwilioSMSInstance: From 469dccc63b3e1e24bd2a4c3ebb03eff855282fd6 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Mon, 25 Nov 2024 09:32:53 +0000 Subject: [PATCH 3/6] Inbound email improvements (continued) (#5294) # What this PR does Minor inbound email improvements: * Adds SNS certificate caching (the [original JS SDK](https://github.com/aws/aws-js-sns-message-validator/blob/a6ba4d646dc60912653357660301f3b25f94d686/index.js#L101-L104) does that as well) * Makes sure we see a 500 when OnCall can't fetch the certificate ## Which issue(s) this PR closes Related to https://github.com/grafana/oncall-private/issues/2905 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/email/tests/test_inbound_email.py | 72 +++++++++++++++++++ .../apps/email/validate_amazon_sns_message.py | 23 ++++-- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/engine/apps/email/tests/test_inbound_email.py b/engine/apps/email/tests/test_inbound_email.py index 252b529208..808fbdfac4 100644 --- a/engine/apps/email/tests/test_inbound_email.py +++ b/engine/apps/email/tests/test_inbound_email.py @@ -14,6 +14,7 @@ from cryptography.x509 import CertificateBuilder, NameOID from django.conf import settings from django.urls import reverse +from requests import RequestException from rest_framework import status from rest_framework.test import APIClient @@ -604,6 +605,77 @@ def test_amazon_ses_validated_fail_wrong_signature( mock_requests_get.assert_called_once_with(SIGNING_CERT_URL, timeout=5) +@patch("requests.get", side_effect=RequestException) +@pytest.mark.django_db +def test_amazon_ses_validated_fail_cant_download_certificate( + _, settings, make_organization, make_alert_receive_channel +): + settings.INBOUND_EMAIL_ESP = "amazon_ses_validated,mailgun" + settings.INBOUND_EMAIL_DOMAIN = "inbound.example.com" + settings.INBOUND_EMAIL_WEBHOOK_SECRET = "secret" + settings.INBOUND_EMAIL_AMAZON_SNS_TOPIC_ARN = AMAZON_SNS_TOPIC_ARN + + organization = make_organization() + make_alert_receive_channel( + organization, + integration=AlertReceiveChannel.INTEGRATION_INBOUND_EMAIL, + token="test-token", + ) + + sns_payload, sns_headers = _sns_inbound_email_payload_and_headers( + sender_email=SENDER_EMAIL, + to_email=TO_EMAIL, + subject=SUBJECT, + message=MESSAGE, + ) + + client = APIClient() + with pytest.raises(RequestException): + client.post( + reverse("integrations:inbound_email_webhook"), + data=sns_payload, + headers=sns_headers, + format="json", + ) + + +@patch("requests.get", return_value=Mock(content=CERTIFICATE)) +@pytest.mark.django_db +def test_amazon_ses_validated_caches_certificate( + mock_requests_get, settings, make_organization, make_alert_receive_channel +): + settings.INBOUND_EMAIL_ESP = "amazon_ses_validated,mailgun" + settings.INBOUND_EMAIL_DOMAIN = "inbound.example.com" + settings.INBOUND_EMAIL_WEBHOOK_SECRET = "secret" + settings.INBOUND_EMAIL_AMAZON_SNS_TOPIC_ARN = AMAZON_SNS_TOPIC_ARN + + organization = make_organization() + make_alert_receive_channel( + organization, + integration=AlertReceiveChannel.INTEGRATION_INBOUND_EMAIL, + token="test-token", + ) + + sns_payload, sns_headers = _sns_inbound_email_payload_and_headers( + sender_email=SENDER_EMAIL, + to_email=TO_EMAIL, + subject=SUBJECT, + message=MESSAGE, + ) + + client = APIClient() + for _ in range(2): + response = client.post( + reverse("integrations:inbound_email_webhook"), + data=sns_payload, + headers=sns_headers, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + + mock_requests_get.assert_called_once_with(SIGNING_CERT_URL, timeout=5) + + @patch.object(create_alert, "delay") @pytest.mark.django_db def test_mailgun_pass(create_alert_mock, settings, make_organization, make_alert_receive_channel): diff --git a/engine/apps/email/validate_amazon_sns_message.py b/engine/apps/email/validate_amazon_sns_message.py index f3d2aec482..e08256525f 100644 --- a/engine/apps/email/validate_amazon_sns_message.py +++ b/engine/apps/email/validate_amazon_sns_message.py @@ -9,6 +9,7 @@ from cryptography.hazmat.primitives.hashes import SHA1, SHA256 from cryptography.x509 import NameOID, load_pem_x509_certificate from django.conf import settings +from django.core.cache import cache logger = logging.getLogger(__name__) @@ -67,13 +68,7 @@ def validate_amazon_sns_message(message: dict) -> bool: return False # Fetch the certificate - try: - response = requests.get(signing_cert_url, timeout=5) - response.raise_for_status() - certificate_bytes = response.content - except requests.RequestException as e: - logger.warning("Failed to fetch the certificate from %s: %s", signing_cert_url, e) - return False + certificate_bytes = fetch_certificate(signing_cert_url) # Verify the certificate issuer certificate = load_pem_x509_certificate(certificate_bytes) @@ -97,3 +92,17 @@ def validate_amazon_sns_message(message: dict) -> bool: return False return True + + +def fetch_certificate(certificate_url: str) -> bytes: + cache_key = f"aws_sns_cert_{certificate_url}" + cached_certificate = cache.get(cache_key) + if cached_certificate: + return cached_certificate + + response = requests.get(certificate_url, timeout=5) + response.raise_for_status() + certificate = response.content + + cache.set(cache_key, certificate, timeout=60 * 60) # Cache for 1 hour + return certificate From a29e35c25ade9f044cb6066574fb9820e512aeeb Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 26 Nov 2024 06:03:38 -0500 Subject: [PATCH 4/6] refactor `SlackMessage.channel_id` (`CHAR` field) to `SlackMessage.channel` (foreign key relationship) (#5292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What this PR does Related to https://github.com/grafana/oncall-private/issues/2947 **NOTE** This PR introduces steps 1 and 2 of the 3 part migration proposed [here](https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099). Step 3, swapping reads to be from the new-column and dropping dual-writes, will be done in a future PR/release. --- I’m tackling this work now because _ultimately_ I want to move `AlertReceiveChannel.rate_limited_in_slack_at` to `SlackChannel.rate_limited_at` , but first I sorta need to refactor `SlackMessage.channel_id` from a `CHAR` field to a foreign key relationship (because in the spots where we touch Slack rate limiting, like [here](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/alert_group_slack_service.py#L42-L50) for example, we only have `slack_message.channel_id`, which means I need to do extra queries to fetch the appropriate `SlackChannel` to then be able to get/set `SlackChannel.rate_limited_at` Other minor stuffs: - it also prepares us to drop `SlackMessage._slack_team_identity`. We already have a `@property` of `SlackMessage.slack_team_identity` (which [previously had some hacky logic](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/models/slack_message.py#L74-L84)). I've refactored `SlackMessage.slack_team_identity` to simply point to `self.organization.slack_team_identity` + updated our code to _stop_ setting `SlackMessage._slack_team_identity` (will drop this column in future release) ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/alerts/models/alert_group.py | 2 +- engine/apps/alerts/tasks/notify_user.py | 1 + engine/apps/alerts/tests/test_alert_group.py | 18 +- engine/apps/alerts/tests/test_notify_user.py | 40 ++-- .../public_api/tests/test_alert_groups.py | 4 +- .../schedules/models/shift_swap_request.py | 12 +- .../tasks/shift_swaps/test_slack_followups.py | 6 +- .../tasks/shift_swaps/test_slack_messages.py | 7 +- .../0007_migrate_slackmessage_channel_id.py | 69 +++++++ .../apps/slack/alert_group_slack_service.py | 14 +- ...el_id_slackmessage__channel_id_and_more.py | 61 ++++++ engine/apps/slack/models/slack_channel.py | 10 + engine/apps/slack/models/slack_message.py | 81 +++++--- .../apps/slack/models/slack_user_identity.py | 7 +- .../slack/scenarios/alertgroup_appearance.py | 16 +- .../apps/slack/scenarios/distribute_alerts.py | 189 ++++++++++-------- .../slack/scenarios/notification_delivery.py | 38 ++-- .../apps/slack/scenarios/resolution_note.py | 20 +- .../slack/scenarios/shift_swap_requests.py | 19 +- .../scenarios/slack_channel_integration.py | 7 +- engine/apps/slack/scenarios/step_mixins.py | 31 ++- engine/apps/slack/test_slack_message.py | 78 -------- engine/apps/slack/tests/factories.py | 1 - .../test_alert_group_actions.py | 72 ++++--- .../scenario_steps/test_distribute_alerts.py | 14 +- .../scenario_steps/test_manage_responders.py | 2 +- .../scenario_steps/test_resolution_note.py | 28 +-- .../test_shift_swap_requests.py | 24 ++- .../test_slack_channel_integration.py | 36 ++-- engine/apps/slack/tests/test_slack_message.py | 80 +++++++- engine/apps/slack/views.py | 7 +- .../tests/test_organization.py | 4 +- engine/conftest.py | 13 +- .../batch_migrate_slack_message_channel.py | 99 +++++++++ 34 files changed, 741 insertions(+), 369 deletions(-) create mode 100644 engine/apps/slack/0007_migrate_slackmessage_channel_id.py create mode 100644 engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py delete mode 100644 engine/apps/slack/test_slack_message.py create mode 100644 engine/engine/management/commands/batch_migrate_slack_message_channel.py diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index f1e5a66d73..54a5b5e204 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -1983,7 +1983,7 @@ def slack_channel_id(self) -> str | None: if not self.channel.organization.slack_team_identity: return None elif self.slack_message: - return self.slack_message.channel_id + return self.slack_message.channel.slack_id elif self.channel_filter: return self.channel_filter.slack_channel_id_or_org_default_id return None diff --git a/engine/apps/alerts/tasks/notify_user.py b/engine/apps/alerts/tasks/notify_user.py index 2cc8b89e91..97e3787268 100644 --- a/engine/apps/alerts/tasks/notify_user.py +++ b/engine/apps/alerts/tasks/notify_user.py @@ -541,6 +541,7 @@ def perform_notification(log_record_pk, use_default_notification_policy_fallback if alert_group.slack_message: alert_group.slack_message.send_slack_notification(user, alert_group, notification_policy) task_logger.debug(f"Finished send_slack_notification for alert_group {alert_group.pk}.") + # check how much time has passed since log record was created # to prevent eternal loop of restarting perform_notification task elif timezone.now() < log_record.created_at + timezone.timedelta(hours=RETRY_TIMEOUT_HOURS): diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index 676c955d25..5bcab73b88 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -14,7 +14,6 @@ ) 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 @@ -24,14 +23,15 @@ def test_render_for_phone_call( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, + make_slack_message, ): - organization, _ = make_organization_with_slack_team_identity() + organization, slack_team_identity = make_organization_with_slack_team_identity() 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) - - alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel) make_alert( alert_group, @@ -105,7 +105,7 @@ def test_delete( make_alert(alert_group, raw_request_data={}) # Create Slack messages - slack_message = make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel1) resolution_note_1 = make_resolution_note_slack_message( alert_group=alert_group, user=user, @@ -154,7 +154,7 @@ def test_delete( 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) + assert mock_chat_delete.call_args_list[1] == call(channel=slack_message.channel.slack_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 ) @@ -188,7 +188,7 @@ def test_delete_slack_ratelimit( 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_slack_message(alert_group=alert_group, channel=slack_channel1) make_resolution_note_slack_message( alert_group=alert_group, user=user, @@ -259,7 +259,7 @@ def test_delete_slack_api_error_other_than_ratelimit( 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_slack_message(alert_group=alert_group, channel=slack_channel1) make_resolution_note_slack_message( alert_group=alert_group, user=user, diff --git a/engine/apps/alerts/tests/test_notify_user.py b/engine/apps/alerts/tests/test_notify_user.py index 2b885cedbe..d40ac5e812 100644 --- a/engine/apps/alerts/tests/test_notify_user.py +++ b/engine/apps/alerts/tests/test_notify_user.py @@ -232,19 +232,17 @@ def test_notify_user_perform_notification_skip_if_resolved( def test_perform_notification_reason_to_skip_escalation_in_slack( reason_to_skip_escalation, error_code, - make_organization, - make_slack_team_identity, + make_organization_with_slack_team_identity, make_user, make_user_notification_policy, make_alert_receive_channel, make_alert_group, make_user_notification_policy_log_record, + make_slack_channel, make_slack_message, ): - organization = make_organization() - slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() + organization, slack_team_identity = make_organization_with_slack_team_identity() + user = make_user(organization=organization) user_notification_policy = make_user_notification_policy( user=user, @@ -252,19 +250,26 @@ def test_perform_notification_reason_to_skip_escalation_in_slack( notify_by=UserNotificationPolicy.NotificationChannel.SLACK, ) alert_receive_channel = make_alert_receive_channel(organization=organization) - alert_group = make_alert_group(alert_receive_channel=alert_receive_channel) - alert_group.reason_to_skip_escalation = reason_to_skip_escalation - alert_group.save() + + alert_group = make_alert_group( + alert_receive_channel=alert_receive_channel, + reason_to_skip_escalation=reason_to_skip_escalation, + ) + log_record = make_user_notification_policy_log_record( author=user, alert_group=alert_group, notification_policy=user_notification_policy, type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED, ) + if not error_code: - make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + slack_channel = make_slack_channel(slack_team_identity=slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel) + with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification: perform_notification(log_record.pk, False) + last_log_record = UserNotificationPolicyLogRecord.objects.last() if error_code: @@ -280,25 +285,24 @@ def test_perform_notification_reason_to_skip_escalation_in_slack( @pytest.mark.django_db def test_perform_notification_slack_prevent_posting( - make_organization, - make_slack_team_identity, + make_organization_with_slack_team_identity, make_user, make_user_notification_policy, make_alert_receive_channel, make_alert_group, make_user_notification_policy_log_record, + make_slack_channel, make_slack_message, ): - organization = make_organization() - slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() + organization, slack_team_identity = make_organization_with_slack_team_identity() + user = make_user(organization=organization) user_notification_policy = make_user_notification_policy( user=user, step=UserNotificationPolicy.Step.NOTIFY, notify_by=UserNotificationPolicy.NotificationChannel.SLACK, ) + alert_receive_channel = make_alert_receive_channel(organization=organization) alert_group = make_alert_group(alert_receive_channel=alert_receive_channel) log_record = make_user_notification_policy_log_record( @@ -308,7 +312,9 @@ def test_perform_notification_slack_prevent_posting( type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED, slack_prevent_posting=True, ) - make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + + slack_channel = make_slack_channel(slack_team_identity=slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel) with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification: perform_notification(log_record.pk, False) diff --git a/engine/apps/public_api/tests/test_alert_groups.py b/engine/apps/public_api/tests/test_alert_groups.py index e3cc872e3a..acc6b823e9 100644 --- a/engine/apps/public_api/tests/test_alert_groups.py +++ b/engine/apps/public_api/tests/test_alert_groups.py @@ -162,9 +162,7 @@ def test_get_alert_group_slack_links( organization.slack_team_identity = slack_team_identity organization.save() slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message( - alert_group=alert_group, channel_id=slack_channel.slack_id, cached_permalink="the-link" - ) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel, cached_permalink="the-link") url = reverse("api-public:alert_groups-detail", kwargs={"pk": expected_response["id"]}) response = client.get(url, format="json", HTTP_AUTHORIZATION=token) diff --git a/engine/apps/schedules/models/shift_swap_request.py b/engine/apps/schedules/models/shift_swap_request.py index b305b26742..b32a4f4802 100644 --- a/engine/apps/schedules/models/shift_swap_request.py +++ b/engine/apps/schedules/models/shift_swap_request.py @@ -17,7 +17,7 @@ if typing.TYPE_CHECKING: from apps.schedules.models import OnCallSchedule from apps.schedules.models.on_call_schedule import ScheduleEvents - from apps.slack.models import SlackMessage + from apps.slack.models import SlackChannel, SlackMessage from apps.user_management.models import Organization, User @@ -164,7 +164,15 @@ def status(self) -> str: return self.Statuses.OPEN @property - def slack_channel_id(self) -> str | None: + def slack_channel(self) -> typing.Optional["SlackChannel"]: + """ + This is only set if the schedule associated with the shift swap request + has a Slack channel configured for it. + """ + return self.schedule.slack_channel + + @property + def slack_channel_id(self) -> typing.Optional[str]: """ This is only set if the schedule associated with the shift swap request has a Slack channel configured for it. diff --git a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py index 53c864b523..d0be1dc063 100644 --- a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py +++ b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py @@ -29,7 +29,9 @@ def _shift_swap_request_test_setup(swap_start=None, swap_end=None, **kwargs): user = make_user(organization=organization) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=None, organization=organization, slack_id="12345") + slack_message = make_slack_message( + alert_group=None, channel=slack_channel, organization=organization, slack_id="12345" + ) schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, slack_channel=slack_channel) @@ -161,7 +163,7 @@ def test_send_shift_swap_request_followup(mock_slack_chat_post_message, shift_sw send_shift_swap_request_slack_followup(shift_swap_request.pk) mock_slack_chat_post_message.assert_called_once_with( - channel=shift_swap_request.slack_message.channel_id, + channel=shift_swap_request.slack_message.channel.slack_id, thread_ts=shift_swap_request.slack_message.slack_id, reply_broadcast=True, blocks=ANY, diff --git a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py index fc1eb8932a..da57d29124 100644 --- a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py +++ b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py @@ -42,12 +42,15 @@ def test_create_shift_swap_request_message_post_message_to_channel_called( schedule = ssr.schedule organization = schedule.organization - slack_message = make_slack_message(alert_group=None, organization=organization, slack_id="12345") slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message( + alert_group=None, channel=slack_channel, organization=organization, slack_id="12345" + ) MockBaseShiftSwapRequestStep.return_value.create_message.return_value = slack_message - schedule.slack_channel = make_slack_channel(slack_team_identity) + schedule.slack_channel = slack_channel schedule.save() organization.slack_team_identity = slack_team_identity diff --git a/engine/apps/slack/0007_migrate_slackmessage_channel_id.py b/engine/apps/slack/0007_migrate_slackmessage_channel_id.py new file mode 100644 index 0000000000..35b7ed81ba --- /dev/null +++ b/engine/apps/slack/0007_migrate_slackmessage_channel_id.py @@ -0,0 +1,69 @@ +# NOTE: I'm temporarily leaving this migration file here, it will very soon be moved to the migrations folder +# see this conversation for context +# https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 + +# Generated by Django 4.2.16 on 2024-11-01 10:58 +import logging + +from django.db import migrations + +logger = logging.getLogger(__name__) + + +def populate_slack_channel(apps, schema_editor): + SlackMessage = apps.get_model("slack", "SlackMessage") + SlackChannel = apps.get_model("slack", "SlackChannel") + + logger.info("Starting migration to populate the 'channel' field.") + + # Filter SlackMessages that need to be updated + slack_messages = SlackMessage.objects.filter( + _channel_id__isnull=False, # Old column + organization_id__isnull=False, + channel_id__isnull=True, # New column to be populated + ) + + total_messages = slack_messages.count() + if total_messages == 0: + logger.info("No SlackMessages need updating.") + return + + logger.info(f"Found {total_messages} SlackMessages to update.") + + updated_count = 0 + + # Use iterator() to avoid loading all records into memory at once + for message in slack_messages.iterator(): + try: + slack_team_identity = message.organization.slack_team_identity + + channel = SlackChannel.objects.filter( + slack_id=message._channel_id, + slack_team_identity=slack_team_identity, + ).first() + + if channel: + message.channel = channel + message.save(update_fields=["channel"]) + updated_count += 1 + else: + logger.warning( + f"No SlackChannel found for SlackMessage id {message.id} " + f"with slack_id {message._channel_id} and " + f"slack_team_identity_id {slack_team_identity.id}." + ) + except Exception as e: + logger.error(f"Error updating SlackMessage id {message.id}: {e}") + + logger.info(f"Updated {updated_count} SlackMessages.") + logger.info("Finished migration to populate the 'channel' field.") + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0006_rename_channel_id_slackmessage__channel_id_and_more"), + ] + + operations = [ + migrations.RunPython(populate_slack_channel, migrations.RunPython.noop), + ] diff --git a/engine/apps/slack/alert_group_slack_service.py b/engine/apps/slack/alert_group_slack_service.py index ed614305f8..12c1a1de60 100644 --- a/engine/apps/slack/alert_group_slack_service.py +++ b/engine/apps/slack/alert_group_slack_service.py @@ -39,7 +39,7 @@ def update_alert_group_slack_message(self, alert_group: "AlertGroup") -> None: try: self._slack_client.chat_update( - channel=alert_group.slack_message.channel_id, + channel=alert_group.slack_message.channel.slack_id, ts=alert_group.slack_message.slack_id, attachments=alert_group.render_slack_attachments(), blocks=alert_group.render_slack_blocks(), @@ -71,15 +71,17 @@ def publish_message_to_alert_group_thread( if alert_group.channel.is_rate_limited_in_slack: return + slack_message = alert_group.slack_message + if attachments is None: attachments = [] try: result = self._slack_client.chat_postMessage( - channel=alert_group.slack_message.channel_id, + channel=slack_message.channel.slack_id, text=text, attachments=attachments, - thread_ts=alert_group.slack_message.slack_id, + thread_ts=slack_message.slack_id, mrkdwn=mrkdwn, unfurl_links=unfurl_links, ) @@ -91,9 +93,11 @@ def publish_message_to_alert_group_thread( ): return + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], organization=alert_group.channel.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=alert_group.slack_message.channel_id, + _channel_id=slack_message.channel.slack_id, + channel=slack_message.channel, ) diff --git a/engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py b/engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py new file mode 100644 index 0000000000..da39c76ba3 --- /dev/null +++ b/engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py @@ -0,0 +1,61 @@ +# NOTE: the foreign key checks are disabled and re-enabled in this migration +# see the following resources as to why +# +# https://arc.net/l/quote/hgqztayk +# https://dba.stackexchange.com/a/316683 +# +# tldr; +# The INPLACE algorithm is supported when foreign_key_checks is disabled. Otherwise, only the COPY algorithm is supported +# +# which means that creating this foreign key constraint, without this, on a large table would take foreverz + +# Generated by Django 4.2.16 on 2024-11-22 12:36 + +import logging + +from django.db import migrations, models +import django.db.models.deletion +import django_migration_linter as linter + +logger = logging.getLogger(__name__) + + +def disable_foreign_key_checks(apps, schema_editor): + if schema_editor.connection.vendor == 'mysql': + with schema_editor.connection.cursor() as cursor: + cursor.execute('SET SESSION foreign_key_checks = OFF;') + logger.info("Disabled foreign key checks.") + else: + logger.info("Skipping disabling foreign key checks for non-MySQL database.") + + +def enable_foreign_key_checks(apps, schema_editor): + if schema_editor.connection.vendor == 'mysql': + with schema_editor.connection.cursor() as cursor: + cursor.execute('SET SESSION foreign_key_checks = ON;') + logger.info("Re-enabled foreign key checks.") + else: + logger.info("Skipping enabling foreign key checks for non-MySQL database.") + + +class Migration(migrations.Migration): + + dependencies = [ + ('slack', '0005_slackteamidentity__unified_slack_app_installed'), + ] + + operations = [ + linter.IgnoreMigration(), + migrations.RenameField( + model_name='slackmessage', + old_name='channel_id', + new_name='_channel_id', + ), + migrations.RunPython(disable_foreign_key_checks, reverse_code=migrations.RunPython.noop), + migrations.AddField( + model_name='slackmessage', + name='channel', + field=models.ForeignKey(default=None, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='slack_messages', to='slack.slackchannel'), + ), + migrations.RunPython(enable_foreign_key_checks, reverse_code=migrations.RunPython.noop), + ] diff --git a/engine/apps/slack/models/slack_channel.py b/engine/apps/slack/models/slack_channel.py index 30de0d841b..c8f119ad13 100644 --- a/engine/apps/slack/models/slack_channel.py +++ b/engine/apps/slack/models/slack_channel.py @@ -1,9 +1,16 @@ +import typing + from django.conf import settings from django.core.validators import MinLengthValidator from django.db import models from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length +if typing.TYPE_CHECKING: + from django.db.models.manager import RelatedManager + + from apps.slack.models import SlackMessage, SlackTeamIdentity + def generate_public_primary_key_for_slack_channel(): prefix = "H" @@ -20,6 +27,9 @@ def generate_public_primary_key_for_slack_channel(): class SlackChannel(models.Model): + slack_team_identity: "SlackTeamIdentity" + slack_messages: "RelatedManager['SlackMessage']" + public_primary_key = models.CharField( max_length=20, validators=[MinLengthValidator(settings.PUBLIC_PRIMARY_KEY_MIN_LENGTH + 1)], diff --git a/engine/apps/slack/models/slack_message.py b/engine/apps/slack/models/slack_message.py index a1a4a969fe..83eaafca7e 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -18,6 +18,9 @@ if typing.TYPE_CHECKING: from apps.alerts.models import AlertGroup + from apps.base.models import UserNotificationPolicy + from apps.slack.models import SlackChannel + from apps.user_management.models import User logger = logging.getLogger(__name__) logger.setLevel(logging.DEBUG) @@ -25,17 +28,32 @@ class SlackMessage(models.Model): alert_group: typing.Optional["AlertGroup"] + channel: "SlackChannel" id = models.CharField(primary_key=True, default=uuid.uuid4, editable=False, max_length=36) - slack_id = models.CharField(max_length=100) - # TODO: convert this to a foreign key field to SlackChannel - channel_id = models.CharField(max_length=100, null=True, default=None) + _channel_id = models.CharField(max_length=100, null=True, default=None) + """ + DEPRECATED/TODO: this is no longer being referenced/set, drop in a separate PR/release + """ + + channel = models.ForeignKey( + "slack.SlackChannel", on_delete=models.CASCADE, null=True, default=None, related_name="slack_messages" + ) + """ + TODO: once we've migrated the data in `_channel_id` to this field, set `null=False` + as we should always have a `channel` associated with a message + """ organization = models.ForeignKey( - "user_management.Organization", on_delete=models.CASCADE, null=True, default=None, related_name="slack_message" + "user_management.Organization", + on_delete=models.CASCADE, + null=True, + default=None, + related_name="slack_message", ) + _slack_team_identity = models.ForeignKey( "slack.SlackTeamIdentity", on_delete=models.PROTECT, @@ -44,6 +62,11 @@ class SlackMessage(models.Model): related_name="slack_message", db_column="slack_team_identity", ) + """ + DEPRECATED/TODO: drop this field in a separate PR/release + + Instead of using this column, we can simply do self.organization.slack_team_identity + """ ack_reminder_message_ts = models.CharField(max_length=100, null=True, default=None) @@ -72,26 +95,17 @@ class Meta: @property def slack_team_identity(self): - if self._slack_team_identity is None: - if self.organization is None: # strange case when organization is None - logger.warning( - f"SlackMessage (pk: {self.pk}) fields _slack_team_identity and organization is None. " - f"It is strange!" - ) - return None - self._slack_team_identity = self.organization.slack_team_identity - self.save() - return self._slack_team_identity + return self.organization.slack_team_identity @property def permalink(self) -> typing.Optional[str]: - # Don't send request for permalink if there is no slack_team_identity or slack token has been revoked - if self.cached_permalink or not self.slack_team_identity or self.slack_team_identity.detected_token_revoked: + # Don't send request for permalink if slack token has been revoked + if self.cached_permalink or self.slack_team_identity.detected_token_revoked: return self.cached_permalink try: result = SlackClient(self.slack_team_identity).chat_getPermalink( - channel=self.channel_id, message_ts=self.slack_id + channel=self.channel.slack_id, message_ts=self.slack_id ) except SlackAPIError: return None @@ -103,12 +117,28 @@ def permalink(self) -> typing.Optional[str]: @property def deep_link(self) -> str: - return f"https://slack.com/app_redirect?channel={self.channel_id}&team={self.slack_team_identity.slack_id}&message={self.slack_id}" + return f"https://slack.com/app_redirect?channel={self.channel.slack_id}&team={self.slack_team_identity.slack_id}&message={self.slack_id}" + + @classmethod + def send_slack_notification( + cls, user: "User", alert_group: "AlertGroup", notification_policy: "UserNotificationPolicy" + ) -> None: + """ + NOTE: the reason why we pass in `alert_group` as an argument here, as opposed to just doing + `self.alert_group`, is that it "looks like" we may have a race condition occuring between two celery tasks: + - one which sends out the initial slack message + - one which notifies the user (this method) inside of the above slack message's thread + + Still some more investigation needed to confirm this, but for now, we'll pass in the `alert_group` as an argument + """ - def send_slack_notification(self, user, alert_group, notification_policy): from apps.base.models import UserNotificationPolicyLogRecord slack_message = alert_group.slack_message + slack_channel = slack_message.channel + organization = alert_group.channel.organization + channel_id = slack_channel.slack_id + user_verbal = user.get_username_with_slack_verbal(mention=True) slack_user_identity = user.slack_user_identity @@ -142,8 +172,8 @@ def send_slack_notification(self, user, alert_group, notification_policy): }, } ] - sc = SlackClient(self.slack_team_identity, enable_ratelimit_retry=True) - channel_id = slack_message.channel_id + + sc = SlackClient(organization.slack_team_identity, enable_ratelimit_retry=True) try: result = sc.chat_postMessage( @@ -190,12 +220,15 @@ def send_slack_notification(self, user, alert_group, notification_policy): ).save() return else: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], - organization=self.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, + organization=organization, + _channel_id=slack_channel.slack_id, + channel=slack_channel, ) + # create success record UserNotificationPolicyLogRecord.objects.create( author=user, diff --git a/engine/apps/slack/models/slack_user_identity.py b/engine/apps/slack/models/slack_user_identity.py index 9f16ca5cea..5788253634 100644 --- a/engine/apps/slack/models/slack_user_identity.py +++ b/engine/apps/slack/models/slack_user_identity.py @@ -18,6 +18,8 @@ if typing.TYPE_CHECKING: from django.db.models.manager import RelatedManager + from apps.slack.models import SlackMessage + logger = logging.getLogger(__name__) @@ -103,7 +105,7 @@ class Meta: def __str__(self): return self.slack_login - def send_link_to_slack_message(self, slack_message): + def send_link_to_slack_message(self, slack_message: "SlackMessage"): blocks = [ { "type": "section", @@ -131,7 +133,8 @@ def send_link_to_slack_message(self, slack_message): { "type": "mrkdwn", "text": ( - f"You received this message because you're not a member of <#{slack_message.channel_id}>.\n" + f"You received this message because you're not a member of " + f"<#{slack_message.channel.slack_id}>.\n" "Please join the channel to get notified right in the alert group thread." ), } diff --git a/engine/apps/slack/scenarios/alertgroup_appearance.py b/engine/apps/slack/scenarios/alertgroup_appearance.py index b07bd85b5e..2f7a0113fe 100644 --- a/engine/apps/slack/scenarios/alertgroup_appearance.py +++ b/engine/apps/slack/scenarios/alertgroup_appearance.py @@ -83,18 +83,14 @@ def process_scenario( from apps.alerts.models import AlertGroup private_metadata = json.loads(payload["view"]["private_metadata"]) - alert_group_pk = private_metadata["alert_group_pk"] - - alert_group = AlertGroup.objects.get(pk=alert_group_pk) - - attachments = alert_group.render_slack_attachments() - blocks = alert_group.render_slack_blocks() + alert_group = AlertGroup.objects.get(pk=private_metadata["alert_group_pk"]) + slack_message = alert_group.slack_message self._slack_client.chat_update( - channel=alert_group.slack_message.channel_id, - ts=alert_group.slack_message.slack_id, - attachments=attachments, - blocks=blocks, + channel=slack_message.channel.slack_id, + ts=slack_message.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), ) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index 3d3c1a60a8..8e11fafc67 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -22,6 +22,7 @@ SlackAPIRestrictedActionError, SlackAPITokenError, ) +from apps.slack.models import SlackChannel, SlackTeamIdentity, SlackUserIdentity from apps.slack.scenarios import scenario_step from apps.slack.slack_formatter import SlackFormatter from apps.slack.tasks import send_message_to_thread_if_bot_not_in_channel, update_incident_slack_message @@ -41,7 +42,6 @@ from .step_mixins import AlertGroupActionsMixin if typing.TYPE_CHECKING: - from apps.slack.models import SlackTeamIdentity, SlackUserIdentity from apps.user_management.models import Organization ATTACH_TO_ALERT_GROUPS_LIMIT = 20 @@ -67,19 +67,19 @@ def process_signal(self, alert: Alert) -> None: if num_updated_rows == 1: try: - channel_id = ( - alert.group.channel_filter.slack_channel_id_or_org_default_id + slack_channel = ( + alert.group.channel_filter.slack_channel if alert.group.channel_filter # if channel filter is deleted mid escalation, use default Slack channel - else alert.group.channel.organization.default_slack_channel_slack_id + else alert.group.channel.organization.default_slack_channel ) - self._send_first_alert(alert, channel_id) + self._send_first_alert(alert, slack_channel) except (SlackAPIError, TimeoutError): AlertGroup.objects.filter(pk=alert.group.pk).update(slack_message_sent=False) raise if alert.group.channel.maintenance_mode == AlertReceiveChannel.DEBUG_MAINTENANCE: - self._send_debug_mode_notice(alert.group, channel_id) + self._send_debug_mode_notice(alert.group, slack_channel) if alert.group.is_maintenance_incident: # not sending log report message for maintenance incident @@ -87,7 +87,7 @@ def process_signal(self, alert: Alert) -> None: else: # check if alert group was posted to slack before posting message to thread if not alert.group.skip_escalation_in_slack: - self._send_message_to_thread_if_bot_not_in_channel(alert.group, channel_id) + self._send_message_to_thread_if_bot_not_in_channel(alert.group, slack_channel) else: # check if alert group was posted to slack before updating its message if not alert.group.skip_escalation_in_slack: @@ -103,42 +103,50 @@ def process_signal(self, alert: Alert) -> None: else: logger.info("Skip updating alert_group in Slack due to rate limit") - def _send_first_alert(self, alert: Alert, channel_id: str) -> None: - attachments = alert.group.render_slack_attachments() - blocks = alert.group.render_slack_blocks() + def _send_first_alert(self, alert: Alert, slack_channel: typing.Optional[SlackChannel]) -> None: self._post_alert_group_to_slack( slack_team_identity=self.slack_team_identity, alert_group=alert.group, alert=alert, - attachments=attachments, - channel_id=channel_id, - blocks=blocks, + attachments=alert.group.render_slack_attachments(), + slack_channel=slack_channel, + blocks=alert.group.render_slack_blocks(), ) def _post_alert_group_to_slack( self, - slack_team_identity: "SlackTeamIdentity", + slack_team_identity: SlackTeamIdentity, alert_group: AlertGroup, alert: Alert, attachments, - channel_id: str, + slack_channel: typing.Optional[SlackChannel], blocks: Block.AnyBlocks, ) -> None: - # channel_id can be None if general log channel for slack_team_identity is not set - if channel_id is None: - logger.info(f"Failed to post message to Slack for alert_group {alert_group.pk} because channel_id is None") + # slack_channel can be None if org default slack channel for slack_team_identity is not set + if slack_channel is None: + logger.info( + f"Failed to post message to Slack for alert_group {alert_group.pk} because slack_channel is None" + ) + alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_NOT_SPECIFIED alert_group.save(update_fields=["reason_to_skip_escalation"]) + return try: - result = self._slack_client.chat_postMessage(channel=channel_id, attachments=attachments, blocks=blocks) + result = self._slack_client.chat_postMessage( + channel=slack_channel.slack_id, + attachments=attachments, + blocks=blocks, + ) + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], organization=alert_group.channel.organization, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + _channel_id=slack_channel.slack_id, + channel=slack_channel, ) alert.delivered = True @@ -174,12 +182,14 @@ def _post_alert_group_to_slack( finally: alert.save() - def _send_debug_mode_notice(self, alert_group: AlertGroup, channel_id: str) -> None: + def _send_debug_mode_notice(self, alert_group: AlertGroup, slack_channel: SlackChannel) -> None: blocks: Block.AnyBlocks = [] + text = "Escalations are silenced due to Debug mode" blocks.append({"type": "section", "text": {"type": "mrkdwn", "text": text}}) + self._slack_client.chat_postMessage( - channel=channel_id, + channel=slack_channel.slack_id, text=text, attachments=[], thread_ts=alert_group.slack_message.slack_id, @@ -187,16 +197,20 @@ def _send_debug_mode_notice(self, alert_group: AlertGroup, channel_id: str) -> N blocks=blocks, ) - def _send_message_to_thread_if_bot_not_in_channel(self, alert_group: AlertGroup, channel_id: str) -> None: + def _send_message_to_thread_if_bot_not_in_channel( + self, + alert_group: AlertGroup, + slack_channel: SlackChannel, + ) -> None: send_message_to_thread_if_bot_not_in_channel.apply_async( - (alert_group.pk, self.slack_team_identity.pk, channel_id), + (alert_group.pk, self.slack_team_identity.pk, slack_channel.slack_id), countdown=1, # delay for message so that the log report is published first ) def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -213,8 +227,8 @@ class InviteOtherPersonToIncident(AlertGroupActionsMixin, scenario_step.Scenario def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -250,8 +264,8 @@ class SilenceGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -280,8 +294,8 @@ class UnSilenceGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -301,8 +315,8 @@ class SelectAttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -459,7 +473,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: if slack_user_identity: self._slack_client.chat_postEphemeral( user=slack_user_identity.slack_id, - channel=alert_group.slack_message.channel_id, + channel=alert_group.slack_message.channel.slack_id, text="{}{}".format(ephemeral_text[:1].upper(), ephemeral_text[1:]), unfurl_links=True, ) @@ -468,8 +482,8 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -521,8 +535,8 @@ class UnAttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -547,8 +561,8 @@ class StopInvitationProcess(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -575,8 +589,8 @@ class ResolveGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -617,8 +631,8 @@ class UnResolveGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -638,8 +652,8 @@ class AcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -659,8 +673,8 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -675,10 +689,11 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: from apps.alerts.models import AlertGroupLogRecord alert_group = log_record.alert_group + slack_message = alert_group.slack_message + logger.debug(f"Started process_signal in UnAcknowledgeGroupStep for alert_group {alert_group.pk}") if log_record.type == AlertGroupLogRecord.TYPE_AUTO_UN_ACK: - channel_id = alert_group.slack_message.channel_id if log_record.author is not None: user_verbal = log_record.author.get_username_with_slack_verbal(mention=True) else: @@ -695,11 +710,12 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: f"{user_verbal} hasn't responded to an acknowledge timeout reminder." f" Alert Group is unacknowledged automatically." ) - if alert_group.slack_message.ack_reminder_message_ts: + + if slack_message.ack_reminder_message_ts: try: self._slack_client.chat_update( - channel=channel_id, - ts=alert_group.slack_message.ack_reminder_message_ts, + channel=slack_message.channel.slack_id, + ts=slack_message.ack_reminder_message_ts, text=text, attachments=message_attachments, ) @@ -714,6 +730,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: self.alert_group_slack_service.publish_message_to_alert_group_thread( alert_group, attachments=message_attachments, text=text ) + self.alert_group_slack_service.update_alert_group_slack_message(alert_group) logger.debug(f"Finished process_signal in UnAcknowledgeGroupStep for alert_group {alert_group.pk}") @@ -721,8 +738,8 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: class AcknowledgeConfirmationStep(AcknowledgeGroupStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -771,48 +788,52 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: from apps.user_management.models import Organization alert_group = log_record.alert_group - channel_id = alert_group.slack_message.channel_id + slack_channel = alert_group.slack_message.channel + user_verbal = log_record.author.get_username_with_slack_verbal(mention=True) text = f"{user_verbal}, please confirm that you're still working on this Alert Group." if alert_group.channel.organization.unacknowledge_timeout != Organization.UNACKNOWLEDGE_TIMEOUT_NEVER: - attachments = [ - { - "fallback": "Are you still working on this Alert Group?", - "text": text, - "callback_id": "alert", - "attachment_type": "default", - "footer": "This is a reminder that the Alert Group is still acknowledged" - " and not resolved. It will be unacknowledged automatically and escalation will" - " start again soon.", - "actions": [ - { - "name": scenario_step.ScenarioStep.get_step( - "distribute_alerts", "AcknowledgeConfirmationStep" - ).routing_uid(), - "text": "Confirm", - "type": "button", - "style": "primary", - "value": make_value({"alert_group_pk": alert_group.pk}, alert_group.channel.organization), - }, - ], - } - ] try: response = self._slack_client.chat_postMessage( - channel=channel_id, + channel=slack_channel.slack_id, text=text, - attachments=attachments, + attachments=[ + { + "fallback": "Are you still working on this Alert Group?", + "text": text, + "callback_id": "alert", + "attachment_type": "default", + "footer": "This is a reminder that the Alert Group is still acknowledged" + " and not resolved. It will be unacknowledged automatically and escalation will" + " start again soon.", + "actions": [ + { + "name": scenario_step.ScenarioStep.get_step( + "distribute_alerts", "AcknowledgeConfirmationStep" + ).routing_uid(), + "text": "Confirm", + "type": "button", + "style": "primary", + "value": make_value( + {"alert_group_pk": alert_group.pk}, alert_group.channel.organization + ), + }, + ], + } + ], thread_ts=alert_group.slack_message.slack_id, ) except (SlackAPITokenError, SlackAPIChannelArchivedError, SlackAPIChannelNotFoundError): pass else: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=response["ts"], organization=alert_group.channel.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, + _channel_id=slack_channel.slack_id, + channel=slack_channel, ) alert_group.slack_message.ack_reminder_message_ts = response["ts"] @@ -860,7 +881,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: # 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) + self._slack_client.chat_delete(channel=message.channel.slack_id, ts=message.slack_id) except SlackAPIRatelimitError: # retries on ratelimit are handled in apps.alerts.tasks.delete_alert_group.delete_alert_group raise diff --git a/engine/apps/slack/scenarios/notification_delivery.py b/engine/apps/slack/scenarios/notification_delivery.py index d1815b3879..d77286ad62 100644 --- a/engine/apps/slack/scenarios/notification_delivery.py +++ b/engine/apps/slack/scenarios/notification_delivery.py @@ -7,7 +7,6 @@ SlackAPITokenError, ) from apps.slack.scenarios import scenario_step -from apps.slack.types import Block if typing.TYPE_CHECKING: from apps.alerts.models import AlertGroupLogRecord @@ -19,6 +18,7 @@ def process_signal(self, log_record: "AlertGroupLogRecord") -> None: user = log_record.author alert_group = log_record.alert_group + slack_channel_id = alert_group.slack_message.channel.slack_id user_verbal_with_mention = user.get_username_with_slack_verbal(mention=True) @@ -31,7 +31,7 @@ def process_signal(self, log_record: "AlertGroupLogRecord") -> None: ): self._post_message_to_channel( f"Attempt to send an SMS to {user_verbal_with_mention} has been failed due to a plan limit", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif ( log_record.notification_error_code @@ -39,7 +39,7 @@ def process_signal(self, log_record: "AlertGroupLogRecord") -> None: ): self._post_message_to_channel( f"Attempt to call to {user_verbal_with_mention} has been failed due to a plan limit", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif ( log_record.notification_error_code @@ -47,7 +47,7 @@ def process_signal(self, log_record: "AlertGroupLogRecord") -> None: ): self._post_message_to_channel( f"Failed to send email to {user_verbal_with_mention}. Exceeded limit for mails", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif ( log_record.notification_error_code @@ -56,31 +56,29 @@ def process_signal(self, log_record: "AlertGroupLogRecord") -> None: if log_record.notification_channel == UserNotificationPolicy.NotificationChannel.SMS: self._post_message_to_channel( f"Failed to send an SMS to {user_verbal_with_mention}. Phone number is not verified", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif log_record.notification_channel == UserNotificationPolicy.NotificationChannel.PHONE_CALL: self._post_message_to_channel( f"Failed to call to {user_verbal_with_mention}. Phone number is not verified", - alert_group.slack_message.channel_id, + slack_channel_id, ) - def _post_message_to_channel(self, text: str, channel: str) -> None: - blocks: Block.AnyBlocks = [ - { - "type": "section", - "block_id": "alert", - "text": { - "type": "mrkdwn", - "text": text, - }, - }, - ] - + def _post_message_to_channel(self, text: str, channel_id: str) -> None: try: self._slack_client.chat_postMessage( - channel=channel, + channel=channel_id, text=text, - blocks=blocks, + blocks=[ + { + "type": "section", + "block_id": "alert", + "text": { + "type": "mrkdwn", + "text": text, + }, + }, + ], unfurl_links=True, ) except ( diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index 8d8a852a80..046967b175 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -92,16 +92,20 @@ def process_scenario( return try: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=payload["message"]["thread_ts"], - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: if settings.UNIFIED_SLACK_APP_ENABLED: # Message shortcut events are broadcasted to multiple regions by chatops-proxy # Don't open a warning window as this event could be handled by another region return + self.open_warning_window(payload, warning_text) return @@ -160,10 +164,14 @@ def process_scenario( slack_channel = SlackChannel.objects.get( slack_id=channel_id, slack_team_identity=slack_team_identity ) + + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=thread_ts, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + # channel__slack_id=channel_id, + _channel_id=channel_id, ) alert_group = slack_message.alert_group @@ -255,7 +263,7 @@ def post_or_update_resolution_note_in_thread(self, resolution_note: "ResolutionN resolution_note_slack_message = resolution_note.resolution_note_slack_message alert_group = resolution_note.alert_group alert_group_slack_message = alert_group.slack_message - slack_channel_id = alert_group_slack_message.channel_id + slack_channel_id = alert_group_slack_message.channel.slack_id blocks = self.get_resolution_note_blocks(resolution_note) slack_channel = SlackChannel.objects.get( @@ -298,7 +306,7 @@ def post_or_update_resolution_note_in_thread(self, resolution_note: "ResolutionN resolution_note_text = Truncator(resolution_note_slack_message.text) try: self._slack_client.chat_update( - channel=alert_group_slack_message.channel_id, + channel=slack_channel_id, ts=resolution_note_slack_message.ts, text=resolution_note_text.chars(BLOCK_SECTION_TEXT_MAX_SIZE), blocks=blocks, diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index 0ef6e3166b..33c29ea920 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -156,17 +156,18 @@ def _generate_blocks(self, shift_swap_request: "ShiftSwapRequest") -> Block.AnyB return blocks def create_message(self, shift_swap_request: "ShiftSwapRequest") -> SlackMessage: - channel_id = shift_swap_request.slack_channel_id - organization = self.organization - - blocks = self._generate_blocks(shift_swap_request) - result = self._slack_client.chat_postMessage(channel=channel_id, blocks=blocks) + result = self._slack_client.chat_postMessage( + channel=shift_swap_request.slack_channel_id, + blocks=self._generate_blocks(shift_swap_request), + ) + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 return SlackMessage.objects.create( slack_id=result["ts"], - organization=organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, + organization=self.organization, + _channel_id=shift_swap_request.slack_channel_id, + channel=shift_swap_request.slack_channel, ) def update_message(self, shift_swap_request: "ShiftSwapRequest") -> None: @@ -186,7 +187,7 @@ def post_message_to_thread( return self._slack_client.chat_postMessage( - channel=shift_swap_request.slack_message.channel_id, + channel=shift_swap_request.slack_message.channel.slack_id, thread_ts=shift_swap_request.slack_message.slack_id, reply_broadcast=reply_broadcast, blocks=blocks, diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index c5295b3dc4..4d9b6be314 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -66,10 +66,13 @@ def save_thread_message_for_resolution_note( message_ts = payload["event"]["ts"] try: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=thread_ts, - channel_id=channel_id, - _slack_team_identity=self.slack_team_identity, + organization__slack_team_identity=self.slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: return diff --git a/engine/apps/slack/scenarios/step_mixins.py b/engine/apps/slack/scenarios/step_mixins.py index 65d4e82577..8813ac32ed 100644 --- a/engine/apps/slack/scenarios/step_mixins.py +++ b/engine/apps/slack/scenarios/step_mixins.py @@ -3,7 +3,7 @@ from apps.alerts.models import AlertGroup from apps.api.permissions import user_is_authorized -from apps.slack.models import SlackMessage, SlackTeamIdentity +from apps.slack.models import SlackChannel, SlackMessage, SlackTeamIdentity from apps.slack.types import EventPayload logger = logging.getLogger(__name__) @@ -44,24 +44,32 @@ def is_authorized(self, alert_group: AlertGroup) -> bool: ) def _repair_alert_group( - self, slack_team_identity: SlackTeamIdentity, alert_group: AlertGroup, payload: EventPayload + self, + slack_team_identity: SlackTeamIdentity, + alert_group: AlertGroup, + payload: EventPayload, ) -> None: """ - There's a possibility that OnCall failed to create a SlackMessage instance for an AlertGroup, but the message - was sent to Slack. This method creates SlackMessage instance for such orphaned messages. + There's a possibility that OnCall failed to create a `SlackMessage` instance for an `AlertGroup`, + but the message was sent to Slack. This method creates `SlackMessage` instance for such orphaned messages. """ - - channel_id = payload["channel"]["id"] try: message_id = payload["message"]["ts"] except KeyError: message_id = payload["original_message"]["ts"] + slack_channel = SlackChannel.objects.get( + slack_id=payload["channel"]["id"], + slack_team_identity=slack_team_identity, + ) + + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 SlackMessage.objects.create( slack_id=message_id, organization=alert_group.channel.organization, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + _channel_id=slack_channel.slack_id, + channel=slack_channel, alert_group=alert_group, ) @@ -171,9 +179,12 @@ def _get_alert_group_from_slack_message_in_db( logger.warning(f"alert_group_pk not found in payload, fetching SlackMessage from DB. message_ts: {message_ts}") # Get SlackMessage from DB + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=message_ts, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) return slack_message.alert_group diff --git a/engine/apps/slack/test_slack_message.py b/engine/apps/slack/test_slack_message.py deleted file mode 100644 index cd68aca486..0000000000 --- a/engine/apps/slack/test_slack_message.py +++ /dev/null @@ -1,78 +0,0 @@ -from unittest.mock import patch - -import pytest -from django.utils import timezone - -from apps.slack.client import SlackClient -from apps.slack.errors import SlackAPIError -from apps.slack.tests.conftest import build_slack_response - - -@pytest.fixture -def slack_message_setup( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, make_slack_message -): - def _slack_message_setup(cached_permalink): - ( - organization, - user, - slack_team_identity, - slack_user_identity, - ) = make_organization_and_user_with_slack_identities() - integration = make_alert_receive_channel(organization) - alert_group = make_alert_group(integration) - - return make_slack_message(alert_group, cached_permalink=cached_permalink) - - return _slack_message_setup - - -@patch.object( - SlackClient, - "chat_getPermalink", - return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), -) -@pytest.mark.django_db -def test_slack_message_permalink(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink=None) - assert slack_message.permalink == "test_permalink" - mock_slack_api_call.assert_called_once() - - -@patch.object( - SlackClient, - "chat_getPermalink", - side_effect=SlackAPIError(response=build_slack_response({"ok": False, "error": "message_not_found"})), -) -@pytest.mark.django_db -def test_slack_message_permalink_error(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink=None) - assert slack_message.permalink is None - mock_slack_api_call.assert_called_once() - - -@patch.object( - SlackClient, - "chat_getPermalink", - return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), -) -@pytest.mark.django_db -def test_slack_message_permalink_cache(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink="cached_permalink") - assert slack_message.permalink == "cached_permalink" - mock_slack_api_call.assert_not_called() - - -@patch.object( - SlackClient, - "chat_getPermalink", - return_value=build_slack_response({"ok": False, "error": "account_inactive"}), -) -@pytest.mark.django_db -def test_slack_message_permalink_token_revoked(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink=None) - slack_message._slack_team_identity.detected_token_revoked = timezone.now() - slack_message._slack_team_identity.save() - assert slack_message._slack_team_identity is not None - assert slack_message.permalink is None - mock_slack_api_call.assert_not_called() diff --git a/engine/apps/slack/tests/factories.py b/engine/apps/slack/tests/factories.py index b99b5105c8..5946a690c8 100644 --- a/engine/apps/slack/tests/factories.py +++ b/engine/apps/slack/tests/factories.py @@ -41,7 +41,6 @@ class Meta: class SlackMessageFactory(factory.DjangoModelFactory): slack_id = UniqueFaker("sentence", nb_words=3) - channel_id = factory.Faker("word") class Meta: model = SlackMessage diff --git a/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py b/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py index 64473c9f34..6898428911 100644 --- a/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py +++ b/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py @@ -86,7 +86,12 @@ def _get_payload(action_type="button", **kwargs): @pytest.mark.parametrize("role", (LegacyAccessControlRole.VIEWER, LegacyAccessControlRole.NONE)) @pytest.mark.django_db def test_alert_group_actions_unauthorized( - step_class, make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, role + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, + step_class, + role, ): organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities( role=role @@ -95,6 +100,8 @@ def test_alert_group_actions_unauthorized( alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -102,7 +109,7 @@ def test_alert_group_actions_unauthorized( "value": json.dumps({"organization_id": organization.pk, "alert_group_pk": alert_group.pk}), } ], - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, "message": {"ts": "RANDOM_MESSAGE_TS"}, "trigger_id": "RANDOM_TRIGGER_ID", } @@ -117,13 +124,18 @@ def test_alert_group_actions_unauthorized( @pytest.mark.django_db def test_get_alert_group_button( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, ): organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -131,7 +143,7 @@ def test_get_alert_group_button( "value": json.dumps({"organization_id": organization.pk, "alert_group_pk": alert_group.pk}), } ], - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, "message": {"ts": "RANDOM_MESSAGE_TS"}, } @@ -145,13 +157,18 @@ def test_get_alert_group_button( @pytest.mark.django_db def test_get_alert_group_static_select( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, ): organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -161,7 +178,7 @@ def test_get_alert_group_static_select( }, } ], - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, "message": {"ts": "RANDOM_MESSAGE_TS"}, } @@ -175,13 +192,18 @@ def test_get_alert_group_static_select( @pytest.mark.django_db def test_get_alert_group_from_message( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, ): organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -207,7 +229,7 @@ def test_get_alert_group_from_message( } ], }, - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, } step = TestScenario(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -232,7 +254,7 @@ def test_get_alert_group_from_slack_message_in_db( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) payload = { "message_ts": slack_message.slack_id, @@ -257,7 +279,7 @@ def test_get_alert_group_from_slack_message_in_db_no_alert_group( organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=None, organization=organization, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=None, organization=organization, channel=slack_channel) payload = { "message_ts": slack_message.slack_id, @@ -307,7 +329,7 @@ def test_step_acknowledge( alert_group = make_alert_group(alert_receive_channel, acknowledged=False, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "AcknowledgeGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -353,7 +375,7 @@ def test_step_unacknowledge( alert_group = make_alert_group(alert_receive_channel, acknowledged=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnAcknowledgeGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -399,7 +421,7 @@ def test_step_resolve( alert_group = make_alert_group(alert_receive_channel, resolved=False, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "ResolveGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -445,7 +467,7 @@ def test_step_unresolve( alert_group = make_alert_group(alert_receive_channel, resolved=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnResolveGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -497,7 +519,7 @@ def test_step_invite( alert_group = make_alert_group(alert_receive_channel, resolved=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "InviteOtherPersonToIncident") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -555,7 +577,7 @@ def test_step_stop_invite( alert_group = make_alert_group(alert_receive_channel, resolved=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) invitation = make_invitation(alert_group, user, second_user, pk=INVITATION_ID) @@ -608,7 +630,7 @@ def test_step_silence( alert_group = make_alert_group(alert_receive_channel, silenced=False, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "SilenceGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -659,7 +681,7 @@ def test_step_unsilence( alert_group = make_alert_group(alert_receive_channel, silenced=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnSilenceGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -699,7 +721,7 @@ def test_step_select_attach( alert_group = make_alert_group(alert_receive_channel, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "SelectAttachGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -753,7 +775,7 @@ def test_step_unattach( alert_group = make_alert_group(alert_receive_channel, root_alert_group=root_alert_group, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnAttachGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -796,8 +818,6 @@ def test_step_format_alert( ): slack_team_identity = make_slack_team_identity() slack_user_identity = make_slack_user_identity(slack_team_identity=slack_team_identity) - slack_channel = make_slack_channel(slack_team_identity, slack_id=SLACK_CHANNEL_ID) - organization = make_organization(pk=ORGANIZATION_ID, slack_team_identity=slack_team_identity) user = make_user(organization=organization, slack_user_identity=slack_user_identity) @@ -805,7 +825,8 @@ def test_step_format_alert( alert_group = make_alert_group(alert_receive_channel, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + slack_channel = make_slack_channel(slack_team_identity, slack_id=SLACK_CHANNEL_ID) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("alertgroup_appearance", "OpenAlertAppearanceDialogStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -824,6 +845,7 @@ def test_step_resolution_note( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, ): organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() @@ -831,7 +853,9 @@ def test_step_resolution_note( alert_group = make_alert_group(alert_receive_channel) make_alert(alert_group, raw_request_data={}) - channel_id = "RANDOM_CHANNEL_ID" + slack_channel = make_slack_channel(slack_team_identity) + channel_id = slack_channel.slack_id + payload = { "trigger_id": "RANDOM_TRIGGER_ID", "actions": [ diff --git a/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py index 9da50f7bb8..4844121027 100644 --- a/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py +++ b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py @@ -24,6 +24,7 @@ def test_skip_escalations_error( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, reason, slack_error, ): @@ -33,16 +34,20 @@ def test_skip_escalations_error( alert_group = make_alert_group(alert_receive_channel) alert = make_alert(alert_group, raw_request_data="{}") + slack_channel = make_slack_channel(slack_team_identity) + step = SlackAlertShootingStep(slack_team_identity) with patch.object(step._slack_client, "api_call") as mock_slack_api_call: error_response = build_slack_response({"error": slack_error}) error_class = get_error_class(error_response) mock_slack_api_call.side_effect = error_class(error_response) - channel_id = "channel-id" + + channel = slack_channel if reason == AlertGroup.CHANNEL_NOT_SPECIFIED: - channel_id = None - step._post_alert_group_to_slack(slack_team_identity, alert_group, alert, None, channel_id, []) + channel = None + + step._post_alert_group_to_slack(slack_team_identity, alert_group, alert, None, channel, []) alert_group.refresh_from_db() alert.refresh_from_db() @@ -107,5 +112,4 @@ def test_alert_shooting_no_channel_filter( step = AlertShootingStep(slack_team_identity, organization) step.process_signal(alert) - mock_post_alert_group_to_slack.assert_called_once() - assert mock_post_alert_group_to_slack.call_args[1]["channel_id"] == "DEFAULT_CHANNEL_ID" + assert mock_post_alert_group_to_slack.call_args[1]["slack_channel"] == slack_channel diff --git a/engine/apps/slack/tests/scenario_steps/test_manage_responders.py b/engine/apps/slack/tests/scenario_steps/test_manage_responders.py index 9802e6c6f8..922a666406 100644 --- a/engine/apps/slack/tests/scenario_steps/test_manage_responders.py +++ b/engine/apps/slack/tests/scenario_steps/test_manage_responders.py @@ -64,7 +64,7 @@ def manage_responders_setup( make_alert(alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity, slack_id=CHANNEL_ID) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=MESSAGE_TS) return organization, user, slack_team_identity, slack_user_identity diff --git a/engine/apps/slack/tests/scenario_steps/test_resolution_note.py b/engine/apps/slack/tests/scenario_steps/test_resolution_note.py index 60cbd2a03b..6fc62816ae 100644 --- a/engine/apps/slack/tests/scenario_steps/test_resolution_note.py +++ b/engine/apps/slack/tests/scenario_steps/test_resolution_note.py @@ -153,9 +153,7 @@ def test_post_or_update_resolution_note_in_thread_truncate_message_text( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group=alert_group, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, channel=slack_channel) resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000) @@ -191,9 +189,7 @@ def test_post_or_update_resolution_note_in_thread_update_truncate_message_text( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group=alert_group, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, channel=slack_channel) resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000) make_resolution_note_slack_message( @@ -312,6 +308,7 @@ def test_resolution_notes_modal_closed_before_update( make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, + make_slack_channel, make_slack_message, ): ResolutionNoteModalStep = ScenarioStep.get_step("resolution_note", "ResolutionNoteModalStep") @@ -320,7 +317,9 @@ def test_resolution_notes_modal_closed_before_update( alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) - make_slack_message(alert_group=alert_group, channel_id="RANDOM_CHANNEL_ID", slack_id="RANDOM_MESSAGE_ID") + + slack_channel = make_slack_channel(slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id="RANDOM_MESSAGE_ID") payload = { "trigger_id": "TEST", @@ -366,12 +365,10 @@ def test_add_to_resolution_note( make_alert(alert_group=alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) payload = { - "channel": {"id": slack_channel_id}, + "channel": {"id": slack_channel.slack_id}, "message_ts": "random_ts", "message": { "type": "message", @@ -421,6 +418,7 @@ def test_add_to_resolution_note_deleted_org( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, make_slack_message, make_organization, make_user_for_organization, @@ -428,18 +426,20 @@ def test_add_to_resolution_note_deleted_org( ): settings.UNIFIED_SLACK_APP_ENABLED = True - organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() + organization, _, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() 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={}) - slack_message = make_slack_message(alert_group=alert_group) + + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) organization.delete() other_organization = make_organization(slack_team_identity=slack_team_identity) other_user = make_user_for_organization(organization=other_organization, slack_user_identity=slack_user_identity) payload = { - "channel": {"id": slack_message.channel_id}, + "channel": {"id": slack_message.channel.slack_id}, "message_ts": "random_ts", "message": { "type": "message", diff --git a/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py b/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py index b6a7fd13a2..906b856fa9 100644 --- a/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py +++ b/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py @@ -11,11 +11,14 @@ @pytest.fixture -def setup(make_organization_and_user_with_slack_identities, shift_swap_request_setup): +def setup(make_organization_and_user_with_slack_identities, make_slack_channel, shift_swap_request_setup): def _setup(**kwargs): organization, _, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() ssr, beneficiary, benefactor = shift_swap_request_setup(**kwargs) + ssr.schedule.slack_channel = make_slack_channel(slack_team_identity) + ssr.schedule.save() + organization = ssr.organization organization.slack_team_identity = slack_team_identity organization.save() @@ -158,19 +161,22 @@ def test_create_message(self, mock_generate_blocks, setup) -> None: assert slack_message.slack_id == ts assert slack_message.organization == organization - assert slack_message.channel_id == ssr.slack_channel_id - assert slack_message._slack_team_identity == slack_team_identity + assert slack_message.channel.slack_id == ssr.slack_channel_id + assert slack_message.slack_team_identity == slack_team_identity @patch("apps.slack.scenarios.shift_swap_requests.BaseShiftSwapRequestStep._generate_blocks") @pytest.mark.django_db - def test_update_message(self, mock_generate_blocks, setup, make_slack_message) -> None: + def test_update_message(self, mock_generate_blocks, setup, make_slack_channel, make_slack_message) -> None: ts = "12345.67" ssr, _, _, _ = setup() organization = ssr.organization slack_team_identity = organization.slack_team_identity - slack_message = make_slack_message(alert_group=None, organization=organization, slack_id=ts) + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message( + alert_group=None, organization=organization, channel=slack_channel, slack_id=ts + ) ssr.slack_message = slack_message ssr.save() @@ -198,17 +204,17 @@ def test_update_message_no_slack_message(self, mock_generate_blocks, setup, make mock_slack_client.chat_update.assert_not_called() @pytest.mark.django_db - def test_post_message_to_thread(self, setup, make_slack_message) -> None: + def test_post_message_to_thread(self, setup, make_slack_channel, make_slack_message) -> None: ts = "12345.67" blocks = [{"foo": "bar"}] ssr, _, _, _ = setup() - channel_id = "asdfadf" organization = ssr.organization slack_team_identity = organization.slack_team_identity + slack_channel = make_slack_channel(slack_team_identity) slack_message = make_slack_message( - alert_group=None, organization=organization, slack_id=ts, channel_id=channel_id + alert_group=None, organization=organization, slack_id=ts, channel=slack_channel ) step = scenarios.BaseShiftSwapRequestStep(slack_team_identity, organization) @@ -226,7 +232,7 @@ def test_post_message_to_thread(self, setup, make_slack_message) -> None: step.post_message_to_thread(ssr, blocks, True) mock_slack_client.chat_postMessage.assert_called_once_with( - channel=channel_id, + channel=slack_channel.slack_id, thread_ts=ts, reply_broadcast=True, blocks=blocks, diff --git a/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py b/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py index fbce8203cd..45384f5316 100644 --- a/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py +++ b/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py @@ -234,9 +234,7 @@ def test_save_thread_message_for_resolution_note_really_long_text( thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group, slack_id=thread_ts, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) mock_permalink = "http://example.com" @@ -246,7 +244,7 @@ def test_save_thread_message_for_resolution_note_really_long_text( payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "h" * 2901, @@ -290,9 +288,7 @@ def test_save_thread_message_for_resolution_note_api_errors( thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group, slack_id=thread_ts, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) step = SlackChannelMessageEventStep(slack_team_identity, organization, user) step._slack_client = Mock() @@ -302,7 +298,7 @@ def test_save_thread_message_for_resolution_note_api_errors( payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "h" * 2901, @@ -343,9 +339,7 @@ def test_save_thread_message_for_resolution_note( thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group, slack_id=thread_ts, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) resolution_note_slack_message = None if resolution_note_slack_message_already_exists: @@ -361,7 +355,7 @@ def test_save_thread_message_for_resolution_note( payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": new_text, @@ -489,9 +483,13 @@ def test_delete_thread_message_from_resolution_note( def test_slack_message_has_no_alert_group( self, make_organization_and_user_with_slack_identities, + make_slack_channel, make_slack_message, ) -> None: """Thread messages for SlackMessage instances without alert_group set (e.g., SSR Slack messages)""" + ts = 88945.4849 + thread_ts = 16789.123 + ( organization, user, @@ -499,21 +497,23 @@ def test_slack_message_has_no_alert_group( slack_user_identity, ) = make_organization_and_user_with_slack_identities() - channel = "potato" - ts = 88945.4849 - thread_ts = 16789.123 + slack_channel = make_slack_channel(slack_team_identity) + make_slack_message( + alert_group=None, + organization=organization, + slack_id=thread_ts, + channel=slack_channel, + ) payload = { "event": { - "channel": channel, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "hello", }, } - make_slack_message(alert_group=None, organization=organization, slack_id=thread_ts, channel_id=channel) - step = SlackChannelMessageEventStep(slack_team_identity, organization, user) step.process_scenario(slack_user_identity, slack_team_identity, payload) diff --git a/engine/apps/slack/tests/test_slack_message.py b/engine/apps/slack/tests/test_slack_message.py index 3f1d8f748d..61419a1b27 100644 --- a/engine/apps/slack/tests/test_slack_message.py +++ b/engine/apps/slack/tests/test_slack_message.py @@ -1,8 +1,31 @@ from unittest.mock import patch import pytest +from django.utils import timezone from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIError +from apps.slack.tests.conftest import build_slack_response + + +@pytest.fixture +def slack_message_setup( + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, + make_slack_message, +): + def _slack_message_setup(cached_permalink): + organization, _, slack_team_identity, _ = make_organization_and_user_with_slack_identities() + integration = make_alert_receive_channel(organization) + alert_group = make_alert_group(integration) + slack_channel = make_slack_channel(slack_team_identity) + + return make_slack_message(alert_group=alert_group, channel=slack_channel, cached_permalink=cached_permalink) + + return _slack_message_setup @pytest.mark.django_db @@ -28,7 +51,7 @@ def test_send_slack_notification( make_alert(alert_group=alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) with patch("apps.slack.client.SlackClient.conversations_members") as mock_members: mock_members.return_value = {"members": [slack_user_identity.slack_id]} @@ -54,10 +77,63 @@ def test_slack_message_deep_link( make_alert(alert_group=alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) expected = ( f"https://slack.com/app_redirect?channel={slack_channel.slack_id}" f"&team={slack_team_identity.slack_id}&message={slack_message.slack_id}" ) assert slack_message.deep_link == expected + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), +) +@pytest.mark.django_db +def test_slack_message_permalink(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + assert slack_message.permalink == "test_permalink" + mock_slack_api_call.assert_called_once() + + +@patch.object( + SlackClient, + "chat_getPermalink", + side_effect=SlackAPIError(response=build_slack_response({"ok": False, "error": "message_not_found"})), +) +@pytest.mark.django_db +def test_slack_message_permalink_error(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + assert slack_message.permalink is None + mock_slack_api_call.assert_called_once() + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), +) +@pytest.mark.django_db +def test_slack_message_permalink_cache(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink="cached_permalink") + assert slack_message.permalink == "cached_permalink" + mock_slack_api_call.assert_not_called() + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": False, "error": "account_inactive"}), +) +@pytest.mark.django_db +def test_slack_message_permalink_token_revoked(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + slack_message.slack_team_identity.detected_token_revoked = timezone.now() + slack_message.slack_team_identity.save() + + assert slack_message.slack_team_identity is not None + assert slack_message.permalink is None + + mock_slack_api_call.assert_not_called() diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index 4eb0eb47ee..e64ea15720 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -516,10 +516,13 @@ def _message_ts() -> str | None: return None try: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( - _slack_team_identity=slack_team_identity, slack_id=message_ts, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: return None diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index 1f4607e5fb..cdc552d884 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -50,6 +50,7 @@ def test_organization_hard_delete( make_team, make_slack_team_identity, make_slack_user_identity, + make_slack_channel, make_slack_message, make_schedule, make_custom_action, @@ -141,7 +142,8 @@ def test_organization_hard_delete( ) telegram_message = make_telegram_message(alert_group=alert_group, message_type=TelegramMessage.ALERT_GROUP_MESSAGE) - slack_message = make_slack_message(alert_group=alert_group) + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) plugin_token, _ = make_token_for_organization(organization) public_api_token, _ = make_public_api_token(user_1, organization) diff --git a/engine/conftest.py b/engine/conftest.py index 0b66e3adea..7ef2555835 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -528,15 +528,16 @@ def _make_slack_user_identity(**kwargs): @pytest.fixture def make_slack_message(): - def _make_slack_message(alert_group=None, organization=None, **kwargs): - organization = organization or alert_group.channel.organization - slack_message = SlackMessageFactory( + def _make_slack_message(channel, alert_group=None, organization=None, **kwargs): + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 + return SlackMessageFactory( alert_group=alert_group, - organization=organization, - _slack_team_identity=organization.slack_team_identity, + organization=organization or alert_group.channel.organization, + _channel_id=channel.slack_id, + channel=channel, **kwargs, ) - return slack_message return _make_slack_message diff --git a/engine/engine/management/commands/batch_migrate_slack_message_channel.py b/engine/engine/management/commands/batch_migrate_slack_message_channel.py new file mode 100644 index 0000000000..2c302b9473 --- /dev/null +++ b/engine/engine/management/commands/batch_migrate_slack_message_channel.py @@ -0,0 +1,99 @@ +import time + +from django.core.management.base import BaseCommand +from django.db import connection, transaction + +from apps.slack.models import SlackChannel, SlackMessage +from apps.user_management.models import Organization + + +class Command(BaseCommand): + help = "Batch updates SlackMessage.channel_id in chunks to avoid locking the table." + + def handle(self, *args, **options): + start_time = time.time() + self.stdout.write("Starting batch update of SlackMessage.channel_id...") + + # Step 1: Determine the queryset to update + # qs is ordered by id to ensure consistent batching + # since id is indexed, this ordering operation "should" be more efficient (as opposed to say created_at + # which we don't have an index on) + qs = SlackMessage.objects.filter( + _channel_id__isnull=False, # old column + organization__isnull=False, + channel_id__isnull=True, # new column + ).order_by("id") + + total_records = qs.count() + if total_records == 0: + self.stdout.write("No records to update.") + return + + self.stdout.write(f"Total records to update: {total_records}") + + # some considerations here.. + # + # Large IN clauses can be inefficient. Keep BATCH_SIZE reasonable (e.g., 1000) + # Fetching large batches of IDs consumes memory. With a BATCH_SIZE of 1000, this "should" be manageable + # + # references + # https://stackoverflow.com/a/5919165 + BATCH_SIZE = 1000 + + total_batches = (total_records + BATCH_SIZE - 1) // BATCH_SIZE + self.stdout.write(f"Batch size: {BATCH_SIZE}") + self.stdout.write(f"Total batches: {total_batches}") + + records_updated = 0 + batch_number = 1 + + # Process updates in batches + while True: + # Get the next batch of IDs + batch_qs = qs[:BATCH_SIZE] + + # collect the IDs to be updated + batch_ids = list(batch_qs.values_list("id", flat=True)) + + if not batch_ids: + break # No more records to process + + placeholders = ", ".join(["%s"] * len(batch_ids)) + update_query = f""" + UPDATE + {SlackMessage._meta.db_table} AS sm + INNER JOIN {Organization._meta.db_table} AS org + ON org.id = sm.organization_id + INNER JOIN {SlackChannel._meta.db_table} AS sc + ON sc.slack_id = sm._channel_id + AND sc.slack_team_identity_id = org.slack_team_identity_id + SET + sm.channel_id = sc.id + WHERE + sm.id IN ({placeholders}) + """ + params = batch_ids + + try: + # Execute the update + with transaction.atomic(): + with connection.cursor() as cursor: + cursor.execute(update_query, params) + batch_records_updated = cursor.rowcount + records_updated += batch_records_updated + + self.stdout.write(f"Batch {batch_number}/{total_batches}: Updated {batch_records_updated} records") + except Exception as e: + self.stderr.write(f"Error updating batch {batch_number}: {e}") + # Optionally, decide whether to continue or abort + continue + + # Remove processed records from queryset for next batch + qs = qs.exclude(id__in=batch_ids) + + batch_number += 1 + + end_time = time.time() + total_time = end_time - start_time + self.stdout.write(f"Batch update completed successfully. Total records updated: {records_updated}") + self.stdout.write(f"Total time taken: {total_time:.2f} seconds") From 98f6b1bfc4a9ab9f1ce59ae8056789b10fa80dd0 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 26 Nov 2024 14:04:39 -0500 Subject: [PATCH 5/6] fix: patch data migration scripts for non-MySQL databases (#5297) # What this PR does ## Which issue(s) this PR closes Fixes https://github.com/grafana/oncall/issues/5244#issuecomment-2493688544 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- ..._migrate_channelfilter_slack_channel_id.py | 82 +++++++++++++---- ...lutionnoteslackmessage_slack_channel_id.py | 87 +++++++++++++++---- .../migrations/0019_auto_20241021_1735.py | 77 ++++++++++++---- .../migrations/0026_auto_20241017_1919.py | 76 ++++++++++++---- 4 files changed, 256 insertions(+), 66 deletions(-) diff --git a/engine/apps/alerts/migrations/0063_migrate_channelfilter_slack_channel_id.py b/engine/apps/alerts/migrations/0063_migrate_channelfilter_slack_channel_id.py index dab5a45943..e0935fe899 100644 --- a/engine/apps/alerts/migrations/0063_migrate_channelfilter_slack_channel_id.py +++ b/engine/apps/alerts/migrations/0063_migrate_channelfilter_slack_channel_id.py @@ -14,23 +14,71 @@ def populate_slack_channel(apps, schema_editor): logger.info("Starting migration to populate slack_channel field.") - sql = f""" - UPDATE {ChannelFilter._meta.db_table} AS cf - JOIN {AlertReceiveChannel._meta.db_table} AS arc ON arc.id = cf.alert_receive_channel_id - JOIN {Organization._meta.db_table} AS org ON org.id = arc.organization_id - JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = cf._slack_channel_id - AND sc.slack_team_identity_id = org.slack_team_identity_id - SET cf.slack_channel_id = sc.id - WHERE cf._slack_channel_id IS NOT NULL - AND org.slack_team_identity_id IS NOT NULL; - """ - - with schema_editor.connection.cursor() as cursor: - cursor.execute(sql) - updated_rows = cursor.rowcount # Number of rows updated - - logger.info(f"Bulk updated {updated_rows} ChannelFilters with their Slack channel.") - logger.info("Finished migration to populate slack_channel field.") + # NOTE: the following raw SQL only works on mysql, fall back to the less-efficient (but working) ORM method + # for non-mysql databases + # + # see the following references for more information: + # https://github.com/grafana/oncall/issues/5244#issuecomment-2493688544 + # https://github.com/grafana/oncall/pull/5233/files#diff-d03cd69968936ddd363cb81aee15a643e4058d1e34bb191a407a0b8fe5fe0a77 + if schema_editor.connection.vendor == "mysql": + sql = f""" + UPDATE {ChannelFilter._meta.db_table} AS cf + JOIN {AlertReceiveChannel._meta.db_table} AS arc ON arc.id = cf.alert_receive_channel_id + JOIN {Organization._meta.db_table} AS org ON org.id = arc.organization_id + JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = cf._slack_channel_id + AND sc.slack_team_identity_id = org.slack_team_identity_id + SET cf.slack_channel_id = sc.id + WHERE cf._slack_channel_id IS NOT NULL + AND org.slack_team_identity_id IS NOT NULL; + """ + + with schema_editor.connection.cursor() as cursor: + cursor.execute(sql) + updated_rows = cursor.rowcount # Number of rows updated + + logger.info(f"Bulk updated {updated_rows} ChannelFilters with their Slack channel.") + logger.info("Finished migration to populate slack_channel field.") + else: + queryset = ChannelFilter.objects.filter( + _slack_channel_id__isnull=False, + alert_receive_channel__organization__slack_team_identity__isnull=False, + ) + total_channel_filters = queryset.count() + updated_channel_filters = 0 + missing_channel_filters = 0 + channel_filters_to_update = [] + + logger.info(f"Total channel filters to process: {total_channel_filters}") + + for channel_filter in queryset: + slack_id = channel_filter._slack_channel_id + slack_team_identity = channel_filter.alert_receive_channel.organization.slack_team_identity + + try: + slack_channel = SlackChannel.objects.get(slack_id=slack_id, slack_team_identity=slack_team_identity) + channel_filter.slack_channel = slack_channel + channel_filters_to_update.append(channel_filter) + + updated_channel_filters += 1 + logger.info( + f"ChannelFilter {channel_filter.id} updated with SlackChannel {slack_channel.id} " + f"(slack_id: {slack_id})." + ) + except SlackChannel.DoesNotExist: + missing_channel_filters += 1 + logger.warning( + f"SlackChannel with slack_id {slack_id} and slack_team_identity {slack_team_identity} " + f"does not exist for ChannelFilter {channel_filter.id}." + ) + + if channel_filters_to_update: + ChannelFilter.objects.bulk_update(channel_filters_to_update, ["slack_channel"]) + logger.info(f"Bulk updated {len(channel_filters_to_update)} ChannelFilters with their Slack channel.") + + logger.info( + f"Finished migration. Total channel filters processed: {total_channel_filters}. " + f"Channel filters updated: {updated_channel_filters}. Missing SlackChannels: {missing_channel_filters}." + ) class Migration(migrations.Migration): diff --git a/engine/apps/alerts/migrations/0064_migrate_resolutionnoteslackmessage_slack_channel_id.py b/engine/apps/alerts/migrations/0064_migrate_resolutionnoteslackmessage_slack_channel_id.py index 4f492e31c4..e542e36c78 100644 --- a/engine/apps/alerts/migrations/0064_migrate_resolutionnoteslackmessage_slack_channel_id.py +++ b/engine/apps/alerts/migrations/0064_migrate_resolutionnoteslackmessage_slack_channel_id.py @@ -16,24 +16,75 @@ def populate_slack_channel(apps, schema_editor): logger.info("Starting migration to populate slack_channel field.") - sql = f""" - UPDATE {ResolutionNoteSlackMessage._meta.db_table} AS rsm - JOIN {AlertGroup._meta.db_table} AS ag ON ag.id = rsm.alert_group_id - JOIN {AlertReceiveChannel._meta.db_table} AS arc ON arc.id = ag.channel_id - JOIN {Organization._meta.db_table} AS org ON org.id = arc.organization_id - JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = rsm._slack_channel_id - AND sc.slack_team_identity_id = org.slack_team_identity_id - SET rsm.slack_channel_id = sc.id - WHERE rsm._slack_channel_id IS NOT NULL - AND org.slack_team_identity_id IS NOT NULL; - """ - - with schema_editor.connection.cursor() as cursor: - cursor.execute(sql) - updated_rows = cursor.rowcount # Number of rows updated - - logger.info(f"Bulk updated {updated_rows} ResolutionNoteSlackMessage records with their Slack channel.") - logger.info("Finished migration to populate slack_channel field.") + # NOTE: the following raw SQL only works on mysql, fall back to the less-efficient (but working) ORM method + # for non-mysql databases + # + # see the following references for more information: + # https://github.com/grafana/oncall/issues/5244#issuecomment-2493688544 + # https://github.com/grafana/oncall/pull/5233/files#diff-4ee42d7e773e6116d7c1d0280d2dbb053422ea55bfa5802a1f26ffbf23a28867 + if schema_editor.connection.vendor == "mysql": + sql = f""" + UPDATE {ResolutionNoteSlackMessage._meta.db_table} AS rsm + JOIN {AlertGroup._meta.db_table} AS ag ON ag.id = rsm.alert_group_id + JOIN {AlertReceiveChannel._meta.db_table} AS arc ON arc.id = ag.channel_id + JOIN {Organization._meta.db_table} AS org ON org.id = arc.organization_id + JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = rsm._slack_channel_id + AND sc.slack_team_identity_id = org.slack_team_identity_id + SET rsm.slack_channel_id = sc.id + WHERE rsm._slack_channel_id IS NOT NULL + AND org.slack_team_identity_id IS NOT NULL; + """ + + with schema_editor.connection.cursor() as cursor: + cursor.execute(sql) + updated_rows = cursor.rowcount # Number of rows updated + + logger.info(f"Bulk updated {updated_rows} ResolutionNoteSlackMessage records with their Slack channel.") + logger.info("Finished migration to populate slack_channel field.") + else: + queryset = ResolutionNoteSlackMessage.objects.filter( + _slack_channel_id__isnull=False, + alert_group__channel__organization__slack_team_identity__isnull=False, + ) + total_resolution_notes = queryset.count() + updated_resolution_notes = 0 + missing_resolution_notes = 0 + resolution_notes_to_update = [] + + logger.info(f"Total resolution note slack messages to process: {total_resolution_notes}") + + for resolution_note in queryset: + slack_id = resolution_note._slack_channel_id + slack_team_identity = resolution_note.alert_group.channel.organization.slack_team_identity + + try: + slack_channel = SlackChannel.objects.get(slack_id=slack_id, slack_team_identity=slack_team_identity) + resolution_note.slack_channel = slack_channel + resolution_notes_to_update.append(resolution_note) + + updated_resolution_notes += 1 + logger.info( + f"ResolutionNoteSlackMessage {resolution_note.id} updated with SlackChannel {slack_channel.id} " + f"(slack_id: {slack_id})." + ) + except SlackChannel.DoesNotExist: + missing_resolution_notes += 1 + logger.warning( + f"SlackChannel with slack_id {slack_id} and slack_team_identity {slack_team_identity} " + f"does not exist for ResolutionNoteSlackMessage {resolution_note.id}." + ) + + if resolution_notes_to_update: + ResolutionNoteSlackMessage.objects.bulk_update(resolution_notes_to_update, ["slack_channel"]) + logger.info( + f"Bulk updated {len(resolution_notes_to_update)} ResolutionNoteSlackMessage with their Slack channel." + ) + + logger.info( + f"Finished migration. Total resolution note slack messages processed: {total_resolution_notes}. " + f"Resolution note slack messages updated: {updated_resolution_notes}. " + f"Missing SlackChannels: {missing_resolution_notes}." + ) class Migration(migrations.Migration): diff --git a/engine/apps/schedules/migrations/0019_auto_20241021_1735.py b/engine/apps/schedules/migrations/0019_auto_20241021_1735.py index edc89366b3..5a7264a245 100644 --- a/engine/apps/schedules/migrations/0019_auto_20241021_1735.py +++ b/engine/apps/schedules/migrations/0019_auto_20241021_1735.py @@ -13,22 +13,67 @@ def populate_slack_channel(apps, schema_editor): logger.info("Starting migration to populate slack_channel field.") - sql = f""" - UPDATE {OnCallSchedule._meta.db_table} AS ocs - JOIN {Organization._meta.db_table} AS org ON org.id = ocs.organization_id - JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = ocs.channel - AND sc.slack_team_identity_id = org.slack_team_identity_id - SET ocs.slack_channel_id = sc.id - WHERE ocs.channel IS NOT NULL - AND org.slack_team_identity_id IS NOT NULL; - """ - - with schema_editor.connection.cursor() as cursor: - cursor.execute(sql) - updated_rows = cursor.rowcount # Number of rows updated - - logger.info(f"Bulk updated {updated_rows} OnCallSchedules with their Slack channel.") - logger.info("Finished migration to populate slack_channel field.") + # NOTE: the following raw SQL only works on mysql, fall back to the less-efficient (but working) ORM method + # for non-mysql databases + # + # see the following references for more information: + # https://github.com/grafana/oncall/issues/5244#issuecomment-2493688544 + # https://github.com/grafana/oncall/pull/5233/files#diff-d287631475456a42d005595383fb0b829cafb25aa15ed09b8e898b34803e59ef + if schema_editor.connection.vendor == "mysql": + sql = f""" + UPDATE {OnCallSchedule._meta.db_table} AS ocs + JOIN {Organization._meta.db_table} AS org ON org.id = ocs.organization_id + JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = ocs.channel + AND sc.slack_team_identity_id = org.slack_team_identity_id + SET ocs.slack_channel_id = sc.id + WHERE ocs.channel IS NOT NULL + AND org.slack_team_identity_id IS NOT NULL; + """ + + with schema_editor.connection.cursor() as cursor: + cursor.execute(sql) + updated_rows = cursor.rowcount # Number of rows updated + + logger.info(f"Bulk updated {updated_rows} OnCallSchedules with their Slack channel.") + logger.info("Finished migration to populate slack_channel field.") + else: + queryset = OnCallSchedule.objects.filter(channel__isnull=False, organization__slack_team_identity__isnull=False) + total_schedules = queryset.count() + updated_schedules = 0 + missing_channels = 0 + schedules_to_update = [] + + logger.info(f"Total schedules to process: {total_schedules}") + + for schedule in queryset: + slack_id = schedule.channel + slack_team_identity = schedule.organization.slack_team_identity + + try: + slack_channel = SlackChannel.objects.get(slack_id=slack_id, slack_team_identity=slack_team_identity) + + schedule.slack_channel = slack_channel + schedules_to_update.append(schedule) + + updated_schedules += 1 + logger.info( + f"Schedule {schedule.id} updated with SlackChannel {slack_channel.id} (slack_id: {slack_id})." + ) + except SlackChannel.DoesNotExist: + missing_channels += 1 + logger.warning( + f"SlackChannel with slack_id {slack_id} and slack_team_identity {slack_team_identity} " + f"does not exist for Schedule {schedule.id}." + ) + + if schedules_to_update: + OnCallSchedule.objects.bulk_update(schedules_to_update, ["slack_channel"]) + logger.info(f"Bulk updated {len(schedules_to_update)} OnCallSchedules with their Slack channel.") + + logger.info( + f"Finished migration. Total schedules processed: {total_schedules}. " + f"Schedules updated: {updated_schedules}. Missing SlackChannels: {missing_channels}." + ) class Migration(migrations.Migration): diff --git a/engine/apps/user_management/migrations/0026_auto_20241017_1919.py b/engine/apps/user_management/migrations/0026_auto_20241017_1919.py index df28b02603..7d1fa1c64f 100644 --- a/engine/apps/user_management/migrations/0026_auto_20241017_1919.py +++ b/engine/apps/user_management/migrations/0026_auto_20241017_1919.py @@ -13,21 +13,67 @@ def populate_default_slack_channel(apps, schema_editor): logger.info("Starting migration to populate default_slack_channel field.") - sql = f""" - UPDATE {Organization._meta.db_table} AS org - JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = org.general_log_channel_id - AND sc.slack_team_identity_id = org.slack_team_identity_id - SET org.default_slack_channel_id = sc.id - WHERE org.general_log_channel_id IS NOT NULL - AND org.slack_team_identity_id IS NOT NULL; - """ - - with schema_editor.connection.cursor() as cursor: - cursor.execute(sql) - updated_rows = cursor.rowcount # Number of rows updated - - logger.info(f"Bulk updated {updated_rows} organizations with their default Slack channel.") - logger.info("Finished migration to populate default_slack_channel field.") + # NOTE: the following raw SQL only works on mysql, fall back to the less-efficient (but working) ORM method + # for non-mysql databases + # + # see the following references for more information: + # https://github.com/grafana/oncall/issues/5244#issuecomment-2493688544 + # https://github.com/grafana/oncall/pull/5233/files#diff-e69e0d7ecf51300be2ca5f4239c5f08b4c6e41de9856788f85a522001595a192 + if schema_editor.connection.vendor == "mysql": + sql = f""" + UPDATE {Organization._meta.db_table} AS org + JOIN {SlackChannel._meta.db_table} AS sc ON sc.slack_id = org.general_log_channel_id + AND sc.slack_team_identity_id = org.slack_team_identity_id + SET org.default_slack_channel_id = sc.id + WHERE org.general_log_channel_id IS NOT NULL + AND org.slack_team_identity_id IS NOT NULL; + """ + + with schema_editor.connection.cursor() as cursor: + cursor.execute(sql) + updated_rows = cursor.rowcount # Number of rows updated + + logger.info(f"Bulk updated {updated_rows} organizations with their default Slack channel.") + logger.info("Finished migration to populate default_slack_channel field.") + else: + queryset = Organization.objects.filter(general_log_channel_id__isnull=False, slack_team_identity__isnull=False) + total_orgs = queryset.count() + updated_orgs = 0 + missing_channels = 0 + organizations_to_update = [] + + logger.info(f"Total organizations to process: {total_orgs}") + + for org in queryset: + slack_id = org.general_log_channel_id + slack_team_identity = org.slack_team_identity + + try: + slack_channel = SlackChannel.objects.get(slack_id=slack_id, slack_team_identity=slack_team_identity) + + org.default_slack_channel = slack_channel + organizations_to_update.append(org) + + updated_orgs += 1 + logger.info( + f"Organization {org.id} updated with SlackChannel {slack_channel.id} (slack_id: {slack_id})." + ) + except SlackChannel.DoesNotExist: + missing_channels += 1 + logger.warning( + f"SlackChannel with slack_id {slack_id} and slack_team_identity {slack_team_identity} " + f"does not exist for Organization {org.id}." + ) + + if organizations_to_update: + Organization.objects.bulk_update(organizations_to_update, ["default_slack_channel"]) + logger.info(f"Bulk updated {len(organizations_to_update)} organizations with their default Slack channel.") + + logger.info( + f"Finished migration. Total organizations processed: {total_orgs}. " + f"Organizations updated: {updated_orgs}. Missing SlackChannels: {missing_channels}." + ) + class Migration(migrations.Migration): From fb0ede6656a161e230937013c47b9800dbe1c8ac Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:06:39 -0500 Subject: [PATCH 6/6] chore(deps): bump tornado from 6.4.1 to 6.4.2 in /engine (#5295) Bumps [tornado](https://github.com/tornadoweb/tornado) from 6.4.1 to 6.4.2.
Changelog

Sourced from tornado's changelog.

Release notes

.. toctree:: :maxdepth: 2

releases/v6.4.2 releases/v6.4.1 releases/v6.4.0 releases/v6.3.3 releases/v6.3.2 releases/v6.3.1 releases/v6.3.0 releases/v6.2.0 releases/v6.1.0 releases/v6.0.4 releases/v6.0.3 releases/v6.0.2 releases/v6.0.1 releases/v6.0.0 releases/v5.1.1 releases/v5.1.0 releases/v5.0.2 releases/v5.0.1 releases/v5.0.0 releases/v4.5.3 releases/v4.5.2 releases/v4.5.1 releases/v4.5.0 releases/v4.4.3 releases/v4.4.2 releases/v4.4.1 releases/v4.4.0 releases/v4.3.0 releases/v4.2.1 releases/v4.2.0 releases/v4.1.0 releases/v4.0.2 releases/v4.0.1 releases/v4.0.0 releases/v3.2.2 releases/v3.2.1 releases/v3.2.0 releases/v3.1.1 releases/v3.1.0 releases/v3.0.2 releases/v3.0.1 releases/v3.0.0 releases/v2.4.1 releases/v2.4.0

... (truncated)

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=tornado&package-manager=pip&previous-version=6.4.1&new-version=6.4.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/grafana/oncall/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- engine/requirements.txt | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/engine/requirements.txt b/engine/requirements.txt index fc1376da5b..5c60801e2e 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -34,7 +34,7 @@ cachetools==4.2.2 # via # google-auth # python-telegram-bot -celery==5.3.1 +celery[redis]==5.3.1 # via -r requirements.in certifi==2024.7.4 # via @@ -98,7 +98,7 @@ django==4.2.16 # social-auth-app-django django-add-default-value==0.10.0 # via -r requirements.in -django-anymail==12.0 +django-anymail[amazon-ses]==12.0 # via -r requirements.in django-cors-headers==3.7.0 # via -r requirements.in @@ -155,7 +155,7 @@ firebase-admin==5.4.0 # via fcm-django flask==3.0.2 # via slack-export-viewer -google-api-core==2.17.0 +google-api-core[grpc]==2.17.0 # via # firebase-admin # google-api-python-client @@ -415,10 +415,6 @@ rsa==4.9 # via google-auth s3transfer==0.10.0 # via boto3 -setuptools==75.3.0 - # via - # apscheduler - # opentelemetry-instrumentation six==1.16.0 # via # apscheduler @@ -442,7 +438,7 @@ sqlparse==0.5.0 # django-silk toml==0.10.2 # via django-migration-linter -tornado==6.4.1 +tornado==6.4.2 # via python-telegram-bot tqdm==4.66.3 # via django-mirage-field