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