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 5b8535b2..9d1b3d6f 100644 --- a/engine/config_integrations/alertmanager.py +++ b/engine/config_integrations/alertmanager.py @@ -31,7 +31,7 @@ web_title = """\ {% 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 a53ddccf..c808e014 100644 --- a/engine/config_integrations/grafana_alerting.py +++ b/engine/config_integrations/grafana_alerting.py @@ -33,7 +33,7 @@ web_title = """\ {% 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 | 40 +++++++++++-------- .../apps/twilioapp/tests/test_phone_calls.py | 6 +-- .../twilioapp/tests/test_twilio_provider.py | 12 ++++-- 4 files changed, 45 insertions(+), 27 deletions(-) diff --git a/engine/apps/twilioapp/gather.py b/engine/apps/twilioapp/gather.py index 8b4f3036..8afe4ec6 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 988746f1..89811bad 100644 --- a/engine/apps/twilioapp/phone_provider.py +++ b/engine/apps/twilioapp/phone_provider.py @@ -6,6 +6,7 @@ from django.db.models import F, Q 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 @@ from apps.phone_notifications.exceptions import ( 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 @@ class TwilioPhoneProvider(PhoneProvider): 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 @@ class TwilioPhoneProvider(PhoneProvider): 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 @@ class TwilioPhoneProvider(PhoneProvider): 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 _message_to_twiml_say(self, message: str) -> VoiceResponse: + response = VoiceResponse() + say = Say(message) + response.append(say) + return response - def _call_create(self, twiml_query: str, to: str, with_callback: bool): + 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 c66977e1..5a8d1966 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 1d7384b6..5054c375 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 252b5292..808fbdfa 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.hazmat.primitives.asymmetric import padding, rsa 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 f3d2aec4..e0825652 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.asymmetric.padding import PKCS1v15 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 f1e5a66d..54a5b5e2 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -1983,7 +1983,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. 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 2cc8b89e..97e37872 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 676c955d..5bcab73b 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.alerts.tasks.delete_alert_group import ( ) 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 2b885ced..d40ac5e8 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 e3cc872e..acc6b823 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 b305b267..b32a4f48 100644 --- a/engine/apps/schedules/models/shift_swap_request.py +++ b/engine/apps/schedules/models/shift_swap_request.py @@ -17,7 +17,7 @@ from common.public_primary_keys import generate_public_primary_key, increase_pub 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 @@ class ShiftSwapRequest(models.Model): 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 53c864b5..d0be1dc0 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( 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 fc1eb893..da57d291 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 00000000..35b7ed81 --- /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 ed614305..12c1a1de 100644 --- a/engine/apps/slack/alert_group_slack_service.py +++ b/engine/apps/slack/alert_group_slack_service.py @@ -39,7 +39,7 @@ class AlertGroupSlackService: 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 @@ class AlertGroupSlackService: 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 @@ class AlertGroupSlackService: ): 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 00000000..da39c76b --- /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 30de0d84..c8f119ad 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 a1a4a969..83eaafca 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -18,6 +18,9 @@ from apps.slack.errors import ( 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 @@ logger.setLevel(logging.DEBUG) 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 SlackMessage(models.Model): @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 @@ class SlackMessage(models.Model): @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 @@ class SlackMessage(models.Model): }, } ] - 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 @@ class SlackMessage(models.Model): ).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 9f16ca5c..57882536 100644 --- a/engine/apps/slack/models/slack_user_identity.py +++ b/engine/apps/slack/models/slack_user_identity.py @@ -18,6 +18,8 @@ from apps.user_management.models import Organization, User 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 SlackUserIdentity(models.Model): 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 @@ class SlackUserIdentity(models.Model): { "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 b07bd85b..2f7a0113 100644 --- a/engine/apps/slack/scenarios/alertgroup_appearance.py +++ b/engine/apps/slack/scenarios/alertgroup_appearance.py @@ -83,18 +83,14 @@ class UpdateAppearanceStep(scenario_step.ScenarioStep): 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 3d3c1a60..8e11fafc 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -22,6 +22,7 @@ from apps.slack.errors import ( 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 common.utils import clean_markup, is_string_with_visible_characters 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 @@ class AlertShootingStep(scenario_step.ScenarioStep): 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 @@ class AlertShootingStep(scenario_step.ScenarioStep): 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 @@ class AlertShootingStep(scenario_step.ScenarioStep): 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 @@ class AlertShootingStep(scenario_step.ScenarioStep): 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 @@ class AlertShootingStep(scenario_step.ScenarioStep): 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 @@ class AttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): 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 @@ class AttachGroupStep(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: @@ -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 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) 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 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) 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 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) 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 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) 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 @@ class AcknowledgeConfirmationStep(AcknowledgeGroupStep): 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 @@ class DeleteGroupStep(scenario_step.ScenarioStep): # 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 d1815b38..d77286ad 100644 --- a/engine/apps/slack/scenarios/notification_delivery.py +++ b/engine/apps/slack/scenarios/notification_delivery.py @@ -7,7 +7,6 @@ from apps.slack.errors import ( 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 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): 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 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): ): 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 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): ): 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 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): ): 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 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): 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 8d8a852a..046967b1 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -92,16 +92,20 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): 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 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): 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 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): 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 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): 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 0ef6e316..33c29ea9 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -156,17 +156,18 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): 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 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): 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 c5295b3d..4d9b6be3 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -66,10 +66,13 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): 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 65d4e825..8813ac32 100644 --- a/engine/apps/slack/scenarios/step_mixins.py +++ b/engine/apps/slack/scenarios/step_mixins.py @@ -3,7 +3,7 @@ import logging 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 @@ class AlertGroupActionsMixin: ) 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 @@ class AlertGroupActionsMixin: 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 cd68aca4..00000000 --- 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 b99b5105..5946a690 100644 --- a/engine/apps/slack/tests/factories.py +++ b/engine/apps/slack/tests/factories.py @@ -41,7 +41,6 @@ class SlackChannelFactory(factory.DjangoModelFactory): 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 64473c9f..68984289 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 9da50f7b..48441210 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 9802e6c6..922a6664 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 60cbd2a0..6fc62816 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 b6a7fd13..906b856f 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 @@ from apps.slack.scenarios import shift_swap_requests as scenarios @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 @@ class TestBaseShiftSwapRequestStep: 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 @@ class TestBaseShiftSwapRequestStep: 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 @@ class TestBaseShiftSwapRequestStep: 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 fbce8203..45384f53 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 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "h" * 2901, @@ -290,9 +288,7 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "h" * 2901, @@ -343,9 +339,7 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": new_text, @@ -489,9 +483,13 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: 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 3f1d8f74..61419a1b 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 4eb0eb47..e64ea157 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -516,10 +516,13 @@ class SlackEventApiEndpointView(APIView): 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 1f4607e5..cdc552d8 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 0b66e3ad..7ef25558 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -528,15 +528,16 @@ def make_slack_user_identity(): @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 00000000..2c302b94 --- /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 | 78 +++++++++++++---- ...lutionnoteslackmessage_slack_channel_id.py | 83 +++++++++++++++---- .../migrations/0019_auto_20241021_1735.py | 73 ++++++++++++---- .../migrations/0026_auto_20241017_1919.py | 72 +++++++++++++--- 4 files changed, 248 insertions(+), 58 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 dab5a459..e0935fe8 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; - """ + # 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 + 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.") + 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 4f492e31..e542e36c 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; - """ + # 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 + 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.") + 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 edc89366..5a7264a2 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; - """ + # 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 + 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.") + 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 df28b026..7d1fa1c6 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; - """ + # 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 + 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}." + ) - logger.info(f"Bulk updated {updated_rows} organizations with their default Slack channel.") - logger.info("Finished migration to populate default_slack_channel field.") 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 fc1376da..5c60801e 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