diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index f1e5a66d..54a5b5e2 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -1983,7 +1983,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. if not self.channel.organization.slack_team_identity: return None elif self.slack_message: - return self.slack_message.channel_id + return self.slack_message.channel.slack_id elif self.channel_filter: return self.channel_filter.slack_channel_id_or_org_default_id return None diff --git a/engine/apps/alerts/tasks/notify_user.py b/engine/apps/alerts/tasks/notify_user.py index 2cc8b89e..97e37872 100644 --- a/engine/apps/alerts/tasks/notify_user.py +++ b/engine/apps/alerts/tasks/notify_user.py @@ -541,6 +541,7 @@ def perform_notification(log_record_pk, use_default_notification_policy_fallback if alert_group.slack_message: alert_group.slack_message.send_slack_notification(user, alert_group, notification_policy) task_logger.debug(f"Finished send_slack_notification for alert_group {alert_group.pk}.") + # check how much time has passed since log record was created # to prevent eternal loop of restarting perform_notification task elif timezone.now() < log_record.created_at + timezone.timedelta(hours=RETRY_TIMEOUT_HOURS): diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index 676c955d..5bcab73b 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -14,7 +14,6 @@ from apps.alerts.tasks.delete_alert_group import ( ) from apps.slack.client import SlackClient from apps.slack.errors import SlackAPIMessageNotFoundError, SlackAPIRatelimitError -from apps.slack.models import SlackMessage from apps.slack.tests.conftest import build_slack_response @@ -24,14 +23,15 @@ def test_render_for_phone_call( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, + make_slack_message, ): - organization, _ = make_organization_with_slack_team_identity() + organization, slack_team_identity = make_organization_with_slack_team_identity() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) - SlackMessage.objects.create(channel_id="CWER1ASD", alert_group=alert_group) - - alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel) make_alert( alert_group, @@ -105,7 +105,7 @@ def test_delete( make_alert(alert_group, raw_request_data={}) # Create Slack messages - slack_message = make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel1) resolution_note_1 = make_resolution_note_slack_message( alert_group=alert_group, user=user, @@ -154,7 +154,7 @@ def test_delete( assert mock_chat_delete.call_args_list[0] == call( channel=resolution_note_1.slack_channel_id, ts=resolution_note_1.ts ) - assert mock_chat_delete.call_args_list[1] == call(channel=slack_message.channel_id, ts=slack_message.slack_id) + assert mock_chat_delete.call_args_list[1] == call(channel=slack_message.channel.slack_id, ts=slack_message.slack_id) mock_reactions_remove.assert_called_once_with( channel=resolution_note_2.slack_channel_id, name="memo", timestamp=resolution_note_2.ts ) @@ -188,7 +188,7 @@ def test_delete_slack_ratelimit( make_alert(alert_group, raw_request_data={}) # Create Slack messages - make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + make_slack_message(alert_group=alert_group, channel=slack_channel1) make_resolution_note_slack_message( alert_group=alert_group, user=user, @@ -259,7 +259,7 @@ def test_delete_slack_api_error_other_than_ratelimit( make_alert(alert_group, raw_request_data={}) # Create Slack messages - make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + make_slack_message(alert_group=alert_group, channel=slack_channel1) make_resolution_note_slack_message( alert_group=alert_group, user=user, diff --git a/engine/apps/alerts/tests/test_notify_user.py b/engine/apps/alerts/tests/test_notify_user.py index 2b885ced..d40ac5e8 100644 --- a/engine/apps/alerts/tests/test_notify_user.py +++ b/engine/apps/alerts/tests/test_notify_user.py @@ -232,19 +232,17 @@ def test_notify_user_perform_notification_skip_if_resolved( def test_perform_notification_reason_to_skip_escalation_in_slack( reason_to_skip_escalation, error_code, - make_organization, - make_slack_team_identity, + make_organization_with_slack_team_identity, make_user, make_user_notification_policy, make_alert_receive_channel, make_alert_group, make_user_notification_policy_log_record, + make_slack_channel, make_slack_message, ): - organization = make_organization() - slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() + organization, slack_team_identity = make_organization_with_slack_team_identity() + user = make_user(organization=organization) user_notification_policy = make_user_notification_policy( user=user, @@ -252,19 +250,26 @@ def test_perform_notification_reason_to_skip_escalation_in_slack( notify_by=UserNotificationPolicy.NotificationChannel.SLACK, ) alert_receive_channel = make_alert_receive_channel(organization=organization) - alert_group = make_alert_group(alert_receive_channel=alert_receive_channel) - alert_group.reason_to_skip_escalation = reason_to_skip_escalation - alert_group.save() + + alert_group = make_alert_group( + alert_receive_channel=alert_receive_channel, + reason_to_skip_escalation=reason_to_skip_escalation, + ) + log_record = make_user_notification_policy_log_record( author=user, alert_group=alert_group, notification_policy=user_notification_policy, type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED, ) + if not error_code: - make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + slack_channel = make_slack_channel(slack_team_identity=slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel) + with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification: perform_notification(log_record.pk, False) + last_log_record = UserNotificationPolicyLogRecord.objects.last() if error_code: @@ -280,25 +285,24 @@ def test_perform_notification_reason_to_skip_escalation_in_slack( @pytest.mark.django_db def test_perform_notification_slack_prevent_posting( - make_organization, - make_slack_team_identity, + make_organization_with_slack_team_identity, make_user, make_user_notification_policy, make_alert_receive_channel, make_alert_group, make_user_notification_policy_log_record, + make_slack_channel, make_slack_message, ): - organization = make_organization() - slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() + organization, slack_team_identity = make_organization_with_slack_team_identity() + user = make_user(organization=organization) user_notification_policy = make_user_notification_policy( user=user, step=UserNotificationPolicy.Step.NOTIFY, notify_by=UserNotificationPolicy.NotificationChannel.SLACK, ) + alert_receive_channel = make_alert_receive_channel(organization=organization) alert_group = make_alert_group(alert_receive_channel=alert_receive_channel) log_record = make_user_notification_policy_log_record( @@ -308,7 +312,9 @@ def test_perform_notification_slack_prevent_posting( type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED, slack_prevent_posting=True, ) - make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id") + + slack_channel = make_slack_channel(slack_team_identity=slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel) with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification: perform_notification(log_record.pk, False) diff --git a/engine/apps/public_api/tests/test_alert_groups.py b/engine/apps/public_api/tests/test_alert_groups.py index e3cc872e..acc6b823 100644 --- a/engine/apps/public_api/tests/test_alert_groups.py +++ b/engine/apps/public_api/tests/test_alert_groups.py @@ -162,9 +162,7 @@ def test_get_alert_group_slack_links( organization.slack_team_identity = slack_team_identity organization.save() slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message( - alert_group=alert_group, channel_id=slack_channel.slack_id, cached_permalink="the-link" - ) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel, cached_permalink="the-link") url = reverse("api-public:alert_groups-detail", kwargs={"pk": expected_response["id"]}) response = client.get(url, format="json", HTTP_AUTHORIZATION=token) diff --git a/engine/apps/schedules/models/shift_swap_request.py b/engine/apps/schedules/models/shift_swap_request.py index b305b267..b32a4f48 100644 --- a/engine/apps/schedules/models/shift_swap_request.py +++ b/engine/apps/schedules/models/shift_swap_request.py @@ -17,7 +17,7 @@ from common.public_primary_keys import generate_public_primary_key, increase_pub if typing.TYPE_CHECKING: from apps.schedules.models import OnCallSchedule from apps.schedules.models.on_call_schedule import ScheduleEvents - from apps.slack.models import SlackMessage + from apps.slack.models import SlackChannel, SlackMessage from apps.user_management.models import Organization, User @@ -164,7 +164,15 @@ class ShiftSwapRequest(models.Model): return self.Statuses.OPEN @property - def slack_channel_id(self) -> str | None: + def slack_channel(self) -> typing.Optional["SlackChannel"]: + """ + This is only set if the schedule associated with the shift swap request + has a Slack channel configured for it. + """ + return self.schedule.slack_channel + + @property + def slack_channel_id(self) -> typing.Optional[str]: """ This is only set if the schedule associated with the shift swap request has a Slack channel configured for it. diff --git a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py index 53c864b5..d0be1dc0 100644 --- a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py +++ b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py @@ -29,7 +29,9 @@ def shift_swap_request_test_setup( user = make_user(organization=organization) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=None, organization=organization, slack_id="12345") + slack_message = make_slack_message( + alert_group=None, channel=slack_channel, organization=organization, slack_id="12345" + ) schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, slack_channel=slack_channel) @@ -161,7 +163,7 @@ def test_send_shift_swap_request_followup(mock_slack_chat_post_message, shift_sw send_shift_swap_request_slack_followup(shift_swap_request.pk) mock_slack_chat_post_message.assert_called_once_with( - channel=shift_swap_request.slack_message.channel_id, + channel=shift_swap_request.slack_message.channel.slack_id, thread_ts=shift_swap_request.slack_message.slack_id, reply_broadcast=True, blocks=ANY, diff --git a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py index fc1eb893..da57d291 100644 --- a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py +++ b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py @@ -42,12 +42,15 @@ def test_create_shift_swap_request_message_post_message_to_channel_called( schedule = ssr.schedule organization = schedule.organization - slack_message = make_slack_message(alert_group=None, organization=organization, slack_id="12345") slack_team_identity = make_slack_team_identity() + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message( + alert_group=None, channel=slack_channel, organization=organization, slack_id="12345" + ) MockBaseShiftSwapRequestStep.return_value.create_message.return_value = slack_message - schedule.slack_channel = make_slack_channel(slack_team_identity) + schedule.slack_channel = slack_channel schedule.save() organization.slack_team_identity = slack_team_identity diff --git a/engine/apps/slack/0007_migrate_slackmessage_channel_id.py b/engine/apps/slack/0007_migrate_slackmessage_channel_id.py new file mode 100644 index 00000000..35b7ed81 --- /dev/null +++ b/engine/apps/slack/0007_migrate_slackmessage_channel_id.py @@ -0,0 +1,69 @@ +# NOTE: I'm temporarily leaving this migration file here, it will very soon be moved to the migrations folder +# see this conversation for context +# https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 + +# Generated by Django 4.2.16 on 2024-11-01 10:58 +import logging + +from django.db import migrations + +logger = logging.getLogger(__name__) + + +def populate_slack_channel(apps, schema_editor): + SlackMessage = apps.get_model("slack", "SlackMessage") + SlackChannel = apps.get_model("slack", "SlackChannel") + + logger.info("Starting migration to populate the 'channel' field.") + + # Filter SlackMessages that need to be updated + slack_messages = SlackMessage.objects.filter( + _channel_id__isnull=False, # Old column + organization_id__isnull=False, + channel_id__isnull=True, # New column to be populated + ) + + total_messages = slack_messages.count() + if total_messages == 0: + logger.info("No SlackMessages need updating.") + return + + logger.info(f"Found {total_messages} SlackMessages to update.") + + updated_count = 0 + + # Use iterator() to avoid loading all records into memory at once + for message in slack_messages.iterator(): + try: + slack_team_identity = message.organization.slack_team_identity + + channel = SlackChannel.objects.filter( + slack_id=message._channel_id, + slack_team_identity=slack_team_identity, + ).first() + + if channel: + message.channel = channel + message.save(update_fields=["channel"]) + updated_count += 1 + else: + logger.warning( + f"No SlackChannel found for SlackMessage id {message.id} " + f"with slack_id {message._channel_id} and " + f"slack_team_identity_id {slack_team_identity.id}." + ) + except Exception as e: + logger.error(f"Error updating SlackMessage id {message.id}: {e}") + + logger.info(f"Updated {updated_count} SlackMessages.") + logger.info("Finished migration to populate the 'channel' field.") + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0006_rename_channel_id_slackmessage__channel_id_and_more"), + ] + + operations = [ + migrations.RunPython(populate_slack_channel, migrations.RunPython.noop), + ] diff --git a/engine/apps/slack/alert_group_slack_service.py b/engine/apps/slack/alert_group_slack_service.py index ed614305..12c1a1de 100644 --- a/engine/apps/slack/alert_group_slack_service.py +++ b/engine/apps/slack/alert_group_slack_service.py @@ -39,7 +39,7 @@ class AlertGroupSlackService: try: self._slack_client.chat_update( - channel=alert_group.slack_message.channel_id, + channel=alert_group.slack_message.channel.slack_id, ts=alert_group.slack_message.slack_id, attachments=alert_group.render_slack_attachments(), blocks=alert_group.render_slack_blocks(), @@ -71,15 +71,17 @@ class AlertGroupSlackService: if alert_group.channel.is_rate_limited_in_slack: return + slack_message = alert_group.slack_message + if attachments is None: attachments = [] try: result = self._slack_client.chat_postMessage( - channel=alert_group.slack_message.channel_id, + channel=slack_message.channel.slack_id, text=text, attachments=attachments, - thread_ts=alert_group.slack_message.slack_id, + thread_ts=slack_message.slack_id, mrkdwn=mrkdwn, unfurl_links=unfurl_links, ) @@ -91,9 +93,11 @@ class AlertGroupSlackService: ): return + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], organization=alert_group.channel.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=alert_group.slack_message.channel_id, + _channel_id=slack_message.channel.slack_id, + channel=slack_message.channel, ) diff --git a/engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py b/engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py new file mode 100644 index 00000000..da39c76b --- /dev/null +++ b/engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py @@ -0,0 +1,61 @@ +# NOTE: the foreign key checks are disabled and re-enabled in this migration +# see the following resources as to why +# +# https://arc.net/l/quote/hgqztayk +# https://dba.stackexchange.com/a/316683 +# +# tldr; +# The INPLACE algorithm is supported when foreign_key_checks is disabled. Otherwise, only the COPY algorithm is supported +# +# which means that creating this foreign key constraint, without this, on a large table would take foreverz + +# Generated by Django 4.2.16 on 2024-11-22 12:36 + +import logging + +from django.db import migrations, models +import django.db.models.deletion +import django_migration_linter as linter + +logger = logging.getLogger(__name__) + + +def disable_foreign_key_checks(apps, schema_editor): + if schema_editor.connection.vendor == 'mysql': + with schema_editor.connection.cursor() as cursor: + cursor.execute('SET SESSION foreign_key_checks = OFF;') + logger.info("Disabled foreign key checks.") + else: + logger.info("Skipping disabling foreign key checks for non-MySQL database.") + + +def enable_foreign_key_checks(apps, schema_editor): + if schema_editor.connection.vendor == 'mysql': + with schema_editor.connection.cursor() as cursor: + cursor.execute('SET SESSION foreign_key_checks = ON;') + logger.info("Re-enabled foreign key checks.") + else: + logger.info("Skipping enabling foreign key checks for non-MySQL database.") + + +class Migration(migrations.Migration): + + dependencies = [ + ('slack', '0005_slackteamidentity__unified_slack_app_installed'), + ] + + operations = [ + linter.IgnoreMigration(), + migrations.RenameField( + model_name='slackmessage', + old_name='channel_id', + new_name='_channel_id', + ), + migrations.RunPython(disable_foreign_key_checks, reverse_code=migrations.RunPython.noop), + migrations.AddField( + model_name='slackmessage', + name='channel', + field=models.ForeignKey(default=None, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='slack_messages', to='slack.slackchannel'), + ), + migrations.RunPython(enable_foreign_key_checks, reverse_code=migrations.RunPython.noop), + ] diff --git a/engine/apps/slack/models/slack_channel.py b/engine/apps/slack/models/slack_channel.py index 30de0d84..c8f119ad 100644 --- a/engine/apps/slack/models/slack_channel.py +++ b/engine/apps/slack/models/slack_channel.py @@ -1,9 +1,16 @@ +import typing + from django.conf import settings from django.core.validators import MinLengthValidator from django.db import models from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length +if typing.TYPE_CHECKING: + from django.db.models.manager import RelatedManager + + from apps.slack.models import SlackMessage, SlackTeamIdentity + def generate_public_primary_key_for_slack_channel(): prefix = "H" @@ -20,6 +27,9 @@ def generate_public_primary_key_for_slack_channel(): class SlackChannel(models.Model): + slack_team_identity: "SlackTeamIdentity" + slack_messages: "RelatedManager['SlackMessage']" + public_primary_key = models.CharField( max_length=20, validators=[MinLengthValidator(settings.PUBLIC_PRIMARY_KEY_MIN_LENGTH + 1)], diff --git a/engine/apps/slack/models/slack_message.py b/engine/apps/slack/models/slack_message.py index a1a4a969..83eaafca 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -18,6 +18,9 @@ from apps.slack.errors import ( if typing.TYPE_CHECKING: from apps.alerts.models import AlertGroup + from apps.base.models import UserNotificationPolicy + from apps.slack.models import SlackChannel + from apps.user_management.models import User logger = logging.getLogger(__name__) logger.setLevel(logging.DEBUG) @@ -25,17 +28,32 @@ logger.setLevel(logging.DEBUG) class SlackMessage(models.Model): alert_group: typing.Optional["AlertGroup"] + channel: "SlackChannel" id = models.CharField(primary_key=True, default=uuid.uuid4, editable=False, max_length=36) - slack_id = models.CharField(max_length=100) - # TODO: convert this to a foreign key field to SlackChannel - channel_id = models.CharField(max_length=100, null=True, default=None) + _channel_id = models.CharField(max_length=100, null=True, default=None) + """ + DEPRECATED/TODO: this is no longer being referenced/set, drop in a separate PR/release + """ + + channel = models.ForeignKey( + "slack.SlackChannel", on_delete=models.CASCADE, null=True, default=None, related_name="slack_messages" + ) + """ + TODO: once we've migrated the data in `_channel_id` to this field, set `null=False` + as we should always have a `channel` associated with a message + """ organization = models.ForeignKey( - "user_management.Organization", on_delete=models.CASCADE, null=True, default=None, related_name="slack_message" + "user_management.Organization", + on_delete=models.CASCADE, + null=True, + default=None, + related_name="slack_message", ) + _slack_team_identity = models.ForeignKey( "slack.SlackTeamIdentity", on_delete=models.PROTECT, @@ -44,6 +62,11 @@ class SlackMessage(models.Model): related_name="slack_message", db_column="slack_team_identity", ) + """ + DEPRECATED/TODO: drop this field in a separate PR/release + + Instead of using this column, we can simply do self.organization.slack_team_identity + """ ack_reminder_message_ts = models.CharField(max_length=100, null=True, default=None) @@ -72,26 +95,17 @@ class SlackMessage(models.Model): @property def slack_team_identity(self): - if self._slack_team_identity is None: - if self.organization is None: # strange case when organization is None - logger.warning( - f"SlackMessage (pk: {self.pk}) fields _slack_team_identity and organization is None. " - f"It is strange!" - ) - return None - self._slack_team_identity = self.organization.slack_team_identity - self.save() - return self._slack_team_identity + return self.organization.slack_team_identity @property def permalink(self) -> typing.Optional[str]: - # Don't send request for permalink if there is no slack_team_identity or slack token has been revoked - if self.cached_permalink or not self.slack_team_identity or self.slack_team_identity.detected_token_revoked: + # Don't send request for permalink if slack token has been revoked + if self.cached_permalink or self.slack_team_identity.detected_token_revoked: return self.cached_permalink try: result = SlackClient(self.slack_team_identity).chat_getPermalink( - channel=self.channel_id, message_ts=self.slack_id + channel=self.channel.slack_id, message_ts=self.slack_id ) except SlackAPIError: return None @@ -103,12 +117,28 @@ class SlackMessage(models.Model): @property def deep_link(self) -> str: - return f"https://slack.com/app_redirect?channel={self.channel_id}&team={self.slack_team_identity.slack_id}&message={self.slack_id}" + return f"https://slack.com/app_redirect?channel={self.channel.slack_id}&team={self.slack_team_identity.slack_id}&message={self.slack_id}" + + @classmethod + def send_slack_notification( + cls, user: "User", alert_group: "AlertGroup", notification_policy: "UserNotificationPolicy" + ) -> None: + """ + NOTE: the reason why we pass in `alert_group` as an argument here, as opposed to just doing + `self.alert_group`, is that it "looks like" we may have a race condition occuring between two celery tasks: + - one which sends out the initial slack message + - one which notifies the user (this method) inside of the above slack message's thread + + Still some more investigation needed to confirm this, but for now, we'll pass in the `alert_group` as an argument + """ - def send_slack_notification(self, user, alert_group, notification_policy): from apps.base.models import UserNotificationPolicyLogRecord slack_message = alert_group.slack_message + slack_channel = slack_message.channel + organization = alert_group.channel.organization + channel_id = slack_channel.slack_id + user_verbal = user.get_username_with_slack_verbal(mention=True) slack_user_identity = user.slack_user_identity @@ -142,8 +172,8 @@ class SlackMessage(models.Model): }, } ] - sc = SlackClient(self.slack_team_identity, enable_ratelimit_retry=True) - channel_id = slack_message.channel_id + + sc = SlackClient(organization.slack_team_identity, enable_ratelimit_retry=True) try: result = sc.chat_postMessage( @@ -190,12 +220,15 @@ class SlackMessage(models.Model): ).save() return else: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], - organization=self.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, + organization=organization, + _channel_id=slack_channel.slack_id, + channel=slack_channel, ) + # create success record UserNotificationPolicyLogRecord.objects.create( author=user, diff --git a/engine/apps/slack/models/slack_user_identity.py b/engine/apps/slack/models/slack_user_identity.py index 9f16ca5c..57882536 100644 --- a/engine/apps/slack/models/slack_user_identity.py +++ b/engine/apps/slack/models/slack_user_identity.py @@ -18,6 +18,8 @@ from apps.user_management.models import Organization, User if typing.TYPE_CHECKING: from django.db.models.manager import RelatedManager + from apps.slack.models import SlackMessage + logger = logging.getLogger(__name__) @@ -103,7 +105,7 @@ class SlackUserIdentity(models.Model): def __str__(self): return self.slack_login - def send_link_to_slack_message(self, slack_message): + def send_link_to_slack_message(self, slack_message: "SlackMessage"): blocks = [ { "type": "section", @@ -131,7 +133,8 @@ class SlackUserIdentity(models.Model): { "type": "mrkdwn", "text": ( - f"You received this message because you're not a member of <#{slack_message.channel_id}>.\n" + f"You received this message because you're not a member of " + f"<#{slack_message.channel.slack_id}>.\n" "Please join the channel to get notified right in the alert group thread." ), } diff --git a/engine/apps/slack/scenarios/alertgroup_appearance.py b/engine/apps/slack/scenarios/alertgroup_appearance.py index b07bd85b..2f7a0113 100644 --- a/engine/apps/slack/scenarios/alertgroup_appearance.py +++ b/engine/apps/slack/scenarios/alertgroup_appearance.py @@ -83,18 +83,14 @@ class UpdateAppearanceStep(scenario_step.ScenarioStep): from apps.alerts.models import AlertGroup private_metadata = json.loads(payload["view"]["private_metadata"]) - alert_group_pk = private_metadata["alert_group_pk"] - - alert_group = AlertGroup.objects.get(pk=alert_group_pk) - - attachments = alert_group.render_slack_attachments() - blocks = alert_group.render_slack_blocks() + alert_group = AlertGroup.objects.get(pk=private_metadata["alert_group_pk"]) + slack_message = alert_group.slack_message self._slack_client.chat_update( - channel=alert_group.slack_message.channel_id, - ts=alert_group.slack_message.slack_id, - attachments=attachments, - blocks=blocks, + channel=slack_message.channel.slack_id, + ts=slack_message.slack_id, + attachments=alert_group.render_slack_attachments(), + blocks=alert_group.render_slack_blocks(), ) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index 3d3c1a60..8e11fafc 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -22,6 +22,7 @@ from apps.slack.errors import ( SlackAPIRestrictedActionError, SlackAPITokenError, ) +from apps.slack.models import SlackChannel, SlackTeamIdentity, SlackUserIdentity from apps.slack.scenarios import scenario_step from apps.slack.slack_formatter import SlackFormatter from apps.slack.tasks import send_message_to_thread_if_bot_not_in_channel, update_incident_slack_message @@ -41,7 +42,6 @@ from common.utils import clean_markup, is_string_with_visible_characters from .step_mixins import AlertGroupActionsMixin if typing.TYPE_CHECKING: - from apps.slack.models import SlackTeamIdentity, SlackUserIdentity from apps.user_management.models import Organization ATTACH_TO_ALERT_GROUPS_LIMIT = 20 @@ -67,19 +67,19 @@ class AlertShootingStep(scenario_step.ScenarioStep): if num_updated_rows == 1: try: - channel_id = ( - alert.group.channel_filter.slack_channel_id_or_org_default_id + slack_channel = ( + alert.group.channel_filter.slack_channel if alert.group.channel_filter # if channel filter is deleted mid escalation, use default Slack channel - else alert.group.channel.organization.default_slack_channel_slack_id + else alert.group.channel.organization.default_slack_channel ) - self._send_first_alert(alert, channel_id) + self._send_first_alert(alert, slack_channel) except (SlackAPIError, TimeoutError): AlertGroup.objects.filter(pk=alert.group.pk).update(slack_message_sent=False) raise if alert.group.channel.maintenance_mode == AlertReceiveChannel.DEBUG_MAINTENANCE: - self._send_debug_mode_notice(alert.group, channel_id) + self._send_debug_mode_notice(alert.group, slack_channel) if alert.group.is_maintenance_incident: # not sending log report message for maintenance incident @@ -87,7 +87,7 @@ class AlertShootingStep(scenario_step.ScenarioStep): else: # check if alert group was posted to slack before posting message to thread if not alert.group.skip_escalation_in_slack: - self._send_message_to_thread_if_bot_not_in_channel(alert.group, channel_id) + self._send_message_to_thread_if_bot_not_in_channel(alert.group, slack_channel) else: # check if alert group was posted to slack before updating its message if not alert.group.skip_escalation_in_slack: @@ -103,42 +103,50 @@ class AlertShootingStep(scenario_step.ScenarioStep): else: logger.info("Skip updating alert_group in Slack due to rate limit") - def _send_first_alert(self, alert: Alert, channel_id: str) -> None: - attachments = alert.group.render_slack_attachments() - blocks = alert.group.render_slack_blocks() + def _send_first_alert(self, alert: Alert, slack_channel: typing.Optional[SlackChannel]) -> None: self._post_alert_group_to_slack( slack_team_identity=self.slack_team_identity, alert_group=alert.group, alert=alert, - attachments=attachments, - channel_id=channel_id, - blocks=blocks, + attachments=alert.group.render_slack_attachments(), + slack_channel=slack_channel, + blocks=alert.group.render_slack_blocks(), ) def _post_alert_group_to_slack( self, - slack_team_identity: "SlackTeamIdentity", + slack_team_identity: SlackTeamIdentity, alert_group: AlertGroup, alert: Alert, attachments, - channel_id: str, + slack_channel: typing.Optional[SlackChannel], blocks: Block.AnyBlocks, ) -> None: - # channel_id can be None if general log channel for slack_team_identity is not set - if channel_id is None: - logger.info(f"Failed to post message to Slack for alert_group {alert_group.pk} because channel_id is None") + # slack_channel can be None if org default slack channel for slack_team_identity is not set + if slack_channel is None: + logger.info( + f"Failed to post message to Slack for alert_group {alert_group.pk} because slack_channel is None" + ) + alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_NOT_SPECIFIED alert_group.save(update_fields=["reason_to_skip_escalation"]) + return try: - result = self._slack_client.chat_postMessage(channel=channel_id, attachments=attachments, blocks=blocks) + result = self._slack_client.chat_postMessage( + channel=slack_channel.slack_id, + attachments=attachments, + blocks=blocks, + ) + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], organization=alert_group.channel.organization, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + _channel_id=slack_channel.slack_id, + channel=slack_channel, ) alert.delivered = True @@ -174,12 +182,14 @@ class AlertShootingStep(scenario_step.ScenarioStep): finally: alert.save() - def _send_debug_mode_notice(self, alert_group: AlertGroup, channel_id: str) -> None: + def _send_debug_mode_notice(self, alert_group: AlertGroup, slack_channel: SlackChannel) -> None: blocks: Block.AnyBlocks = [] + text = "Escalations are silenced due to Debug mode" blocks.append({"type": "section", "text": {"type": "mrkdwn", "text": text}}) + self._slack_client.chat_postMessage( - channel=channel_id, + channel=slack_channel.slack_id, text=text, attachments=[], thread_ts=alert_group.slack_message.slack_id, @@ -187,16 +197,20 @@ class AlertShootingStep(scenario_step.ScenarioStep): blocks=blocks, ) - def _send_message_to_thread_if_bot_not_in_channel(self, alert_group: AlertGroup, channel_id: str) -> None: + def _send_message_to_thread_if_bot_not_in_channel( + self, + alert_group: AlertGroup, + slack_channel: SlackChannel, + ) -> None: send_message_to_thread_if_bot_not_in_channel.apply_async( - (alert_group.pk, self.slack_team_identity.pk, channel_id), + (alert_group.pk, self.slack_team_identity.pk, slack_channel.slack_id), countdown=1, # delay for message so that the log report is published first ) def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -213,8 +227,8 @@ class InviteOtherPersonToIncident(AlertGroupActionsMixin, scenario_step.Scenario def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -250,8 +264,8 @@ class SilenceGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -280,8 +294,8 @@ class UnSilenceGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -301,8 +315,8 @@ class SelectAttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -459,7 +473,7 @@ class AttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): if slack_user_identity: self._slack_client.chat_postEphemeral( user=slack_user_identity.slack_id, - channel=alert_group.slack_message.channel_id, + channel=alert_group.slack_message.channel.slack_id, text="{}{}".format(ephemeral_text[:1].upper(), ephemeral_text[1:]), unfurl_links=True, ) @@ -468,8 +482,8 @@ class AttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -521,8 +535,8 @@ class UnAttachGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -547,8 +561,8 @@ class StopInvitationProcess(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -575,8 +589,8 @@ class ResolveGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -617,8 +631,8 @@ class UnResolveGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -638,8 +652,8 @@ class AcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -659,8 +673,8 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -675,10 +689,11 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) from apps.alerts.models import AlertGroupLogRecord alert_group = log_record.alert_group + slack_message = alert_group.slack_message + logger.debug(f"Started process_signal in UnAcknowledgeGroupStep for alert_group {alert_group.pk}") if log_record.type == AlertGroupLogRecord.TYPE_AUTO_UN_ACK: - channel_id = alert_group.slack_message.channel_id if log_record.author is not None: user_verbal = log_record.author.get_username_with_slack_verbal(mention=True) else: @@ -695,11 +710,12 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) f"{user_verbal} hasn't responded to an acknowledge timeout reminder." f" Alert Group is unacknowledged automatically." ) - if alert_group.slack_message.ack_reminder_message_ts: + + if slack_message.ack_reminder_message_ts: try: self._slack_client.chat_update( - channel=channel_id, - ts=alert_group.slack_message.ack_reminder_message_ts, + channel=slack_message.channel.slack_id, + ts=slack_message.ack_reminder_message_ts, text=text, attachments=message_attachments, ) @@ -714,6 +730,7 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) self.alert_group_slack_service.publish_message_to_alert_group_thread( alert_group, attachments=message_attachments, text=text ) + self.alert_group_slack_service.update_alert_group_slack_message(alert_group) logger.debug(f"Finished process_signal in UnAcknowledgeGroupStep for alert_group {alert_group.pk}") @@ -721,8 +738,8 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) class AcknowledgeConfirmationStep(AcknowledgeGroupStep): def process_scenario( self, - slack_user_identity: "SlackUserIdentity", - slack_team_identity: "SlackTeamIdentity", + slack_user_identity: SlackUserIdentity, + slack_team_identity: SlackTeamIdentity, payload: "EventPayload", predefined_org: typing.Optional["Organization"] = None, ) -> None: @@ -771,48 +788,52 @@ class AcknowledgeConfirmationStep(AcknowledgeGroupStep): from apps.user_management.models import Organization alert_group = log_record.alert_group - channel_id = alert_group.slack_message.channel_id + slack_channel = alert_group.slack_message.channel + user_verbal = log_record.author.get_username_with_slack_verbal(mention=True) text = f"{user_verbal}, please confirm that you're still working on this Alert Group." if alert_group.channel.organization.unacknowledge_timeout != Organization.UNACKNOWLEDGE_TIMEOUT_NEVER: - attachments = [ - { - "fallback": "Are you still working on this Alert Group?", - "text": text, - "callback_id": "alert", - "attachment_type": "default", - "footer": "This is a reminder that the Alert Group is still acknowledged" - " and not resolved. It will be unacknowledged automatically and escalation will" - " start again soon.", - "actions": [ - { - "name": scenario_step.ScenarioStep.get_step( - "distribute_alerts", "AcknowledgeConfirmationStep" - ).routing_uid(), - "text": "Confirm", - "type": "button", - "style": "primary", - "value": make_value({"alert_group_pk": alert_group.pk}, alert_group.channel.organization), - }, - ], - } - ] try: response = self._slack_client.chat_postMessage( - channel=channel_id, + channel=slack_channel.slack_id, text=text, - attachments=attachments, + attachments=[ + { + "fallback": "Are you still working on this Alert Group?", + "text": text, + "callback_id": "alert", + "attachment_type": "default", + "footer": "This is a reminder that the Alert Group is still acknowledged" + " and not resolved. It will be unacknowledged automatically and escalation will" + " start again soon.", + "actions": [ + { + "name": scenario_step.ScenarioStep.get_step( + "distribute_alerts", "AcknowledgeConfirmationStep" + ).routing_uid(), + "text": "Confirm", + "type": "button", + "style": "primary", + "value": make_value( + {"alert_group_pk": alert_group.pk}, alert_group.channel.organization + ), + }, + ], + } + ], thread_ts=alert_group.slack_message.slack_id, ) except (SlackAPITokenError, SlackAPIChannelArchivedError, SlackAPIChannelNotFoundError): pass else: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=response["ts"], organization=alert_group.channel.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, + _channel_id=slack_channel.slack_id, + channel=slack_channel, ) alert_group.slack_message.ack_reminder_message_ts = response["ts"] @@ -860,7 +881,7 @@ class DeleteGroupStep(scenario_step.ScenarioStep): # Remove alert group Slack messages for message in alert_group.slack_messages.all(): try: - self._slack_client.chat_delete(channel=message.channel_id, ts=message.slack_id) + self._slack_client.chat_delete(channel=message.channel.slack_id, ts=message.slack_id) except SlackAPIRatelimitError: # retries on ratelimit are handled in apps.alerts.tasks.delete_alert_group.delete_alert_group raise diff --git a/engine/apps/slack/scenarios/notification_delivery.py b/engine/apps/slack/scenarios/notification_delivery.py index d1815b38..d77286ad 100644 --- a/engine/apps/slack/scenarios/notification_delivery.py +++ b/engine/apps/slack/scenarios/notification_delivery.py @@ -7,7 +7,6 @@ from apps.slack.errors import ( SlackAPITokenError, ) from apps.slack.scenarios import scenario_step -from apps.slack.types import Block if typing.TYPE_CHECKING: from apps.alerts.models import AlertGroupLogRecord @@ -19,6 +18,7 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): user = log_record.author alert_group = log_record.alert_group + slack_channel_id = alert_group.slack_message.channel.slack_id user_verbal_with_mention = user.get_username_with_slack_verbal(mention=True) @@ -31,7 +31,7 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): ): self._post_message_to_channel( f"Attempt to send an SMS to {user_verbal_with_mention} has been failed due to a plan limit", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif ( log_record.notification_error_code @@ -39,7 +39,7 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): ): self._post_message_to_channel( f"Attempt to call to {user_verbal_with_mention} has been failed due to a plan limit", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif ( log_record.notification_error_code @@ -47,7 +47,7 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): ): self._post_message_to_channel( f"Failed to send email to {user_verbal_with_mention}. Exceeded limit for mails", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif ( log_record.notification_error_code @@ -56,31 +56,29 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): if log_record.notification_channel == UserNotificationPolicy.NotificationChannel.SMS: self._post_message_to_channel( f"Failed to send an SMS to {user_verbal_with_mention}. Phone number is not verified", - alert_group.slack_message.channel_id, + slack_channel_id, ) elif log_record.notification_channel == UserNotificationPolicy.NotificationChannel.PHONE_CALL: self._post_message_to_channel( f"Failed to call to {user_verbal_with_mention}. Phone number is not verified", - alert_group.slack_message.channel_id, + slack_channel_id, ) - def _post_message_to_channel(self, text: str, channel: str) -> None: - blocks: Block.AnyBlocks = [ - { - "type": "section", - "block_id": "alert", - "text": { - "type": "mrkdwn", - "text": text, - }, - }, - ] - + def _post_message_to_channel(self, text: str, channel_id: str) -> None: try: self._slack_client.chat_postMessage( - channel=channel, + channel=channel_id, text=text, - blocks=blocks, + blocks=[ + { + "type": "section", + "block_id": "alert", + "text": { + "type": "mrkdwn", + "text": text, + }, + }, + ], unfurl_links=True, ) except ( diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index 8d8a852a..046967b1 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -92,16 +92,20 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): return try: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=payload["message"]["thread_ts"], - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: if settings.UNIFIED_SLACK_APP_ENABLED: # Message shortcut events are broadcasted to multiple regions by chatops-proxy # Don't open a warning window as this event could be handled by another region return + self.open_warning_window(payload, warning_text) return @@ -160,10 +164,14 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): slack_channel = SlackChannel.objects.get( slack_id=channel_id, slack_team_identity=slack_team_identity ) + + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=thread_ts, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + # channel__slack_id=channel_id, + _channel_id=channel_id, ) alert_group = slack_message.alert_group @@ -255,7 +263,7 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): resolution_note_slack_message = resolution_note.resolution_note_slack_message alert_group = resolution_note.alert_group alert_group_slack_message = alert_group.slack_message - slack_channel_id = alert_group_slack_message.channel_id + slack_channel_id = alert_group_slack_message.channel.slack_id blocks = self.get_resolution_note_blocks(resolution_note) slack_channel = SlackChannel.objects.get( @@ -298,7 +306,7 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): resolution_note_text = Truncator(resolution_note_slack_message.text) try: self._slack_client.chat_update( - channel=alert_group_slack_message.channel_id, + channel=slack_channel_id, ts=resolution_note_slack_message.ts, text=resolution_note_text.chars(BLOCK_SECTION_TEXT_MAX_SIZE), blocks=blocks, diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index 0ef6e316..33c29ea9 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -156,17 +156,18 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): return blocks def create_message(self, shift_swap_request: "ShiftSwapRequest") -> SlackMessage: - channel_id = shift_swap_request.slack_channel_id - organization = self.organization - - blocks = self._generate_blocks(shift_swap_request) - result = self._slack_client.chat_postMessage(channel=channel_id, blocks=blocks) + result = self._slack_client.chat_postMessage( + channel=shift_swap_request.slack_channel_id, + blocks=self._generate_blocks(shift_swap_request), + ) + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 return SlackMessage.objects.create( slack_id=result["ts"], - organization=organization, - _slack_team_identity=self.slack_team_identity, - channel_id=channel_id, + organization=self.organization, + _channel_id=shift_swap_request.slack_channel_id, + channel=shift_swap_request.slack_channel, ) def update_message(self, shift_swap_request: "ShiftSwapRequest") -> None: @@ -186,7 +187,7 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): return self._slack_client.chat_postMessage( - channel=shift_swap_request.slack_message.channel_id, + channel=shift_swap_request.slack_message.channel.slack_id, thread_ts=shift_swap_request.slack_message.slack_id, reply_broadcast=reply_broadcast, blocks=blocks, diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index c5295b3d..4d9b6be3 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -66,10 +66,13 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): message_ts = payload["event"]["ts"] try: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=thread_ts, - channel_id=channel_id, - _slack_team_identity=self.slack_team_identity, + organization__slack_team_identity=self.slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: return diff --git a/engine/apps/slack/scenarios/step_mixins.py b/engine/apps/slack/scenarios/step_mixins.py index 65d4e825..8813ac32 100644 --- a/engine/apps/slack/scenarios/step_mixins.py +++ b/engine/apps/slack/scenarios/step_mixins.py @@ -3,7 +3,7 @@ import logging from apps.alerts.models import AlertGroup from apps.api.permissions import user_is_authorized -from apps.slack.models import SlackMessage, SlackTeamIdentity +from apps.slack.models import SlackChannel, SlackMessage, SlackTeamIdentity from apps.slack.types import EventPayload logger = logging.getLogger(__name__) @@ -44,24 +44,32 @@ class AlertGroupActionsMixin: ) def _repair_alert_group( - self, slack_team_identity: SlackTeamIdentity, alert_group: AlertGroup, payload: EventPayload + self, + slack_team_identity: SlackTeamIdentity, + alert_group: AlertGroup, + payload: EventPayload, ) -> None: """ - There's a possibility that OnCall failed to create a SlackMessage instance for an AlertGroup, but the message - was sent to Slack. This method creates SlackMessage instance for such orphaned messages. + There's a possibility that OnCall failed to create a `SlackMessage` instance for an `AlertGroup`, + but the message was sent to Slack. This method creates `SlackMessage` instance for such orphaned messages. """ - - channel_id = payload["channel"]["id"] try: message_id = payload["message"]["ts"] except KeyError: message_id = payload["original_message"]["ts"] + slack_channel = SlackChannel.objects.get( + slack_id=payload["channel"]["id"], + slack_team_identity=slack_team_identity, + ) + + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 SlackMessage.objects.create( slack_id=message_id, organization=alert_group.channel.organization, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + _channel_id=slack_channel.slack_id, + channel=slack_channel, alert_group=alert_group, ) @@ -171,9 +179,12 @@ class AlertGroupActionsMixin: logger.warning(f"alert_group_pk not found in payload, fetching SlackMessage from DB. message_ts: {message_ts}") # Get SlackMessage from DB + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=message_ts, - _slack_team_identity=slack_team_identity, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) return slack_message.alert_group diff --git a/engine/apps/slack/test_slack_message.py b/engine/apps/slack/test_slack_message.py deleted file mode 100644 index cd68aca4..00000000 --- a/engine/apps/slack/test_slack_message.py +++ /dev/null @@ -1,78 +0,0 @@ -from unittest.mock import patch - -import pytest -from django.utils import timezone - -from apps.slack.client import SlackClient -from apps.slack.errors import SlackAPIError -from apps.slack.tests.conftest import build_slack_response - - -@pytest.fixture -def slack_message_setup( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, make_slack_message -): - def _slack_message_setup(cached_permalink): - ( - organization, - user, - slack_team_identity, - slack_user_identity, - ) = make_organization_and_user_with_slack_identities() - integration = make_alert_receive_channel(organization) - alert_group = make_alert_group(integration) - - return make_slack_message(alert_group, cached_permalink=cached_permalink) - - return _slack_message_setup - - -@patch.object( - SlackClient, - "chat_getPermalink", - return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), -) -@pytest.mark.django_db -def test_slack_message_permalink(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink=None) - assert slack_message.permalink == "test_permalink" - mock_slack_api_call.assert_called_once() - - -@patch.object( - SlackClient, - "chat_getPermalink", - side_effect=SlackAPIError(response=build_slack_response({"ok": False, "error": "message_not_found"})), -) -@pytest.mark.django_db -def test_slack_message_permalink_error(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink=None) - assert slack_message.permalink is None - mock_slack_api_call.assert_called_once() - - -@patch.object( - SlackClient, - "chat_getPermalink", - return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), -) -@pytest.mark.django_db -def test_slack_message_permalink_cache(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink="cached_permalink") - assert slack_message.permalink == "cached_permalink" - mock_slack_api_call.assert_not_called() - - -@patch.object( - SlackClient, - "chat_getPermalink", - return_value=build_slack_response({"ok": False, "error": "account_inactive"}), -) -@pytest.mark.django_db -def test_slack_message_permalink_token_revoked(mock_slack_api_call, slack_message_setup): - slack_message = slack_message_setup(cached_permalink=None) - slack_message._slack_team_identity.detected_token_revoked = timezone.now() - slack_message._slack_team_identity.save() - assert slack_message._slack_team_identity is not None - assert slack_message.permalink is None - mock_slack_api_call.assert_not_called() diff --git a/engine/apps/slack/tests/factories.py b/engine/apps/slack/tests/factories.py index b99b5105..5946a690 100644 --- a/engine/apps/slack/tests/factories.py +++ b/engine/apps/slack/tests/factories.py @@ -41,7 +41,6 @@ class SlackChannelFactory(factory.DjangoModelFactory): class SlackMessageFactory(factory.DjangoModelFactory): slack_id = UniqueFaker("sentence", nb_words=3) - channel_id = factory.Faker("word") class Meta: model = SlackMessage diff --git a/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py b/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py index 64473c9f..68984289 100644 --- a/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py +++ b/engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py @@ -86,7 +86,12 @@ def _get_payload(action_type="button", **kwargs): @pytest.mark.parametrize("role", (LegacyAccessControlRole.VIEWER, LegacyAccessControlRole.NONE)) @pytest.mark.django_db def test_alert_group_actions_unauthorized( - step_class, make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, role + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, + step_class, + role, ): organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities( role=role @@ -95,6 +100,8 @@ def test_alert_group_actions_unauthorized( alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -102,7 +109,7 @@ def test_alert_group_actions_unauthorized( "value": json.dumps({"organization_id": organization.pk, "alert_group_pk": alert_group.pk}), } ], - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, "message": {"ts": "RANDOM_MESSAGE_TS"}, "trigger_id": "RANDOM_TRIGGER_ID", } @@ -117,13 +124,18 @@ def test_alert_group_actions_unauthorized( @pytest.mark.django_db def test_get_alert_group_button( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, ): organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -131,7 +143,7 @@ def test_get_alert_group_button( "value": json.dumps({"organization_id": organization.pk, "alert_group_pk": alert_group.pk}), } ], - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, "message": {"ts": "RANDOM_MESSAGE_TS"}, } @@ -145,13 +157,18 @@ def test_get_alert_group_button( @pytest.mark.django_db def test_get_alert_group_static_select( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, ): organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -161,7 +178,7 @@ def test_get_alert_group_static_select( }, } ], - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, "message": {"ts": "RANDOM_MESSAGE_TS"}, } @@ -175,13 +192,18 @@ def test_get_alert_group_static_select( @pytest.mark.django_db def test_get_alert_group_from_message( - make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, ): organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) + slack_channel = make_slack_channel(slack_team_identity) + payload = { "actions": [ { @@ -207,7 +229,7 @@ def test_get_alert_group_from_message( } ], }, - "channel": {"id": "RANDOM_CHANNEL_ID"}, + "channel": {"id": slack_channel.slack_id}, } step = TestScenario(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -232,7 +254,7 @@ def test_get_alert_group_from_slack_message_in_db( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) payload = { "message_ts": slack_message.slack_id, @@ -257,7 +279,7 @@ def test_get_alert_group_from_slack_message_in_db_no_alert_group( organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities() slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=None, organization=organization, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=None, organization=organization, channel=slack_channel) payload = { "message_ts": slack_message.slack_id, @@ -307,7 +329,7 @@ def test_step_acknowledge( alert_group = make_alert_group(alert_receive_channel, acknowledged=False, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "AcknowledgeGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -353,7 +375,7 @@ def test_step_unacknowledge( alert_group = make_alert_group(alert_receive_channel, acknowledged=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnAcknowledgeGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -399,7 +421,7 @@ def test_step_resolve( alert_group = make_alert_group(alert_receive_channel, resolved=False, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "ResolveGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -445,7 +467,7 @@ def test_step_unresolve( alert_group = make_alert_group(alert_receive_channel, resolved=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnResolveGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -497,7 +519,7 @@ def test_step_invite( alert_group = make_alert_group(alert_receive_channel, resolved=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "InviteOtherPersonToIncident") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -555,7 +577,7 @@ def test_step_stop_invite( alert_group = make_alert_group(alert_receive_channel, resolved=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) invitation = make_invitation(alert_group, user, second_user, pk=INVITATION_ID) @@ -608,7 +630,7 @@ def test_step_silence( alert_group = make_alert_group(alert_receive_channel, silenced=False, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "SilenceGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -659,7 +681,7 @@ def test_step_unsilence( alert_group = make_alert_group(alert_receive_channel, silenced=True, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnSilenceGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -699,7 +721,7 @@ def test_step_select_attach( alert_group = make_alert_group(alert_receive_channel, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "SelectAttachGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -753,7 +775,7 @@ def test_step_unattach( alert_group = make_alert_group(alert_receive_channel, root_alert_group=root_alert_group, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("distribute_alerts", "UnAttachGroupStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -796,8 +818,6 @@ def test_step_format_alert( ): slack_team_identity = make_slack_team_identity() slack_user_identity = make_slack_user_identity(slack_team_identity=slack_team_identity) - slack_channel = make_slack_channel(slack_team_identity, slack_id=SLACK_CHANNEL_ID) - organization = make_organization(pk=ORGANIZATION_ID, slack_team_identity=slack_team_identity) user = make_user(organization=organization, slack_user_identity=slack_user_identity) @@ -805,7 +825,8 @@ def test_step_format_alert( alert_group = make_alert_group(alert_receive_channel, pk=ALERT_GROUP_ID) make_alert(alert_group, raw_request_data={}) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=SLACK_MESSAGE_TS) + slack_channel = make_slack_channel(slack_team_identity, slack_id=SLACK_CHANNEL_ID) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=SLACK_MESSAGE_TS) step_class = ScenarioStep.get_step("alertgroup_appearance", "OpenAlertAppearanceDialogStep") step = step_class(organization=organization, user=user, slack_team_identity=slack_team_identity) @@ -824,6 +845,7 @@ def test_step_resolution_note( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, ): organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() @@ -831,7 +853,9 @@ def test_step_resolution_note( alert_group = make_alert_group(alert_receive_channel) make_alert(alert_group, raw_request_data={}) - channel_id = "RANDOM_CHANNEL_ID" + slack_channel = make_slack_channel(slack_team_identity) + channel_id = slack_channel.slack_id + payload = { "trigger_id": "RANDOM_TRIGGER_ID", "actions": [ diff --git a/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py index 9da50f7b..48441210 100644 --- a/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py +++ b/engine/apps/slack/tests/scenario_steps/test_distribute_alerts.py @@ -24,6 +24,7 @@ def test_skip_escalations_error( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, reason, slack_error, ): @@ -33,16 +34,20 @@ def test_skip_escalations_error( alert_group = make_alert_group(alert_receive_channel) alert = make_alert(alert_group, raw_request_data="{}") + slack_channel = make_slack_channel(slack_team_identity) + step = SlackAlertShootingStep(slack_team_identity) with patch.object(step._slack_client, "api_call") as mock_slack_api_call: error_response = build_slack_response({"error": slack_error}) error_class = get_error_class(error_response) mock_slack_api_call.side_effect = error_class(error_response) - channel_id = "channel-id" + + channel = slack_channel if reason == AlertGroup.CHANNEL_NOT_SPECIFIED: - channel_id = None - step._post_alert_group_to_slack(slack_team_identity, alert_group, alert, None, channel_id, []) + channel = None + + step._post_alert_group_to_slack(slack_team_identity, alert_group, alert, None, channel, []) alert_group.refresh_from_db() alert.refresh_from_db() @@ -107,5 +112,4 @@ def test_alert_shooting_no_channel_filter( step = AlertShootingStep(slack_team_identity, organization) step.process_signal(alert) - mock_post_alert_group_to_slack.assert_called_once() - assert mock_post_alert_group_to_slack.call_args[1]["channel_id"] == "DEFAULT_CHANNEL_ID" + assert mock_post_alert_group_to_slack.call_args[1]["slack_channel"] == slack_channel diff --git a/engine/apps/slack/tests/scenario_steps/test_manage_responders.py b/engine/apps/slack/tests/scenario_steps/test_manage_responders.py index 9802e6c6..922a6664 100644 --- a/engine/apps/slack/tests/scenario_steps/test_manage_responders.py +++ b/engine/apps/slack/tests/scenario_steps/test_manage_responders.py @@ -64,7 +64,7 @@ def manage_responders_setup( make_alert(alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity, slack_id=CHANNEL_ID) - make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id, slack_id=MESSAGE_TS) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id=MESSAGE_TS) return organization, user, slack_team_identity, slack_user_identity diff --git a/engine/apps/slack/tests/scenario_steps/test_resolution_note.py b/engine/apps/slack/tests/scenario_steps/test_resolution_note.py index 60cbd2a0..6fc62816 100644 --- a/engine/apps/slack/tests/scenario_steps/test_resolution_note.py +++ b/engine/apps/slack/tests/scenario_steps/test_resolution_note.py @@ -153,9 +153,7 @@ def test_post_or_update_resolution_note_in_thread_truncate_message_text( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group=alert_group, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, channel=slack_channel) resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000) @@ -191,9 +189,7 @@ def test_post_or_update_resolution_note_in_thread_update_truncate_message_text( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group=alert_group, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, channel=slack_channel) resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000) make_resolution_note_slack_message( @@ -312,6 +308,7 @@ def test_resolution_notes_modal_closed_before_update( make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, + make_slack_channel, make_slack_message, ): ResolutionNoteModalStep = ScenarioStep.get_step("resolution_note", "ResolutionNoteModalStep") @@ -320,7 +317,9 @@ def test_resolution_notes_modal_closed_before_update( alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) - make_slack_message(alert_group=alert_group, channel_id="RANDOM_CHANNEL_ID", slack_id="RANDOM_MESSAGE_ID") + + slack_channel = make_slack_channel(slack_team_identity) + make_slack_message(alert_group=alert_group, channel=slack_channel, slack_id="RANDOM_MESSAGE_ID") payload = { "trigger_id": "TEST", @@ -366,12 +365,10 @@ def test_add_to_resolution_note( make_alert(alert_group=alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) payload = { - "channel": {"id": slack_channel_id}, + "channel": {"id": slack_channel.slack_id}, "message_ts": "random_ts", "message": { "type": "message", @@ -421,6 +418,7 @@ def test_add_to_resolution_note_deleted_org( make_alert_receive_channel, make_alert_group, make_alert, + make_slack_channel, make_slack_message, make_organization, make_user_for_organization, @@ -428,18 +426,20 @@ def test_add_to_resolution_note_deleted_org( ): settings.UNIFIED_SLACK_APP_ENABLED = True - organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() + organization, _, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() alert_receive_channel = make_alert_receive_channel(organization) alert_group = make_alert_group(alert_receive_channel) make_alert(alert_group=alert_group, raw_request_data={}) - slack_message = make_slack_message(alert_group=alert_group) + + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) organization.delete() other_organization = make_organization(slack_team_identity=slack_team_identity) other_user = make_user_for_organization(organization=other_organization, slack_user_identity=slack_user_identity) payload = { - "channel": {"id": slack_message.channel_id}, + "channel": {"id": slack_message.channel.slack_id}, "message_ts": "random_ts", "message": { "type": "message", diff --git a/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py b/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py index b6a7fd13..906b856f 100644 --- a/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py +++ b/engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py @@ -11,11 +11,14 @@ from apps.slack.scenarios import shift_swap_requests as scenarios @pytest.fixture -def setup(make_organization_and_user_with_slack_identities, shift_swap_request_setup): +def setup(make_organization_and_user_with_slack_identities, make_slack_channel, shift_swap_request_setup): def _setup(**kwargs): organization, _, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() ssr, beneficiary, benefactor = shift_swap_request_setup(**kwargs) + ssr.schedule.slack_channel = make_slack_channel(slack_team_identity) + ssr.schedule.save() + organization = ssr.organization organization.slack_team_identity = slack_team_identity organization.save() @@ -158,19 +161,22 @@ class TestBaseShiftSwapRequestStep: assert slack_message.slack_id == ts assert slack_message.organization == organization - assert slack_message.channel_id == ssr.slack_channel_id - assert slack_message._slack_team_identity == slack_team_identity + assert slack_message.channel.slack_id == ssr.slack_channel_id + assert slack_message.slack_team_identity == slack_team_identity @patch("apps.slack.scenarios.shift_swap_requests.BaseShiftSwapRequestStep._generate_blocks") @pytest.mark.django_db - def test_update_message(self, mock_generate_blocks, setup, make_slack_message) -> None: + def test_update_message(self, mock_generate_blocks, setup, make_slack_channel, make_slack_message) -> None: ts = "12345.67" ssr, _, _, _ = setup() organization = ssr.organization slack_team_identity = organization.slack_team_identity - slack_message = make_slack_message(alert_group=None, organization=organization, slack_id=ts) + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message( + alert_group=None, organization=organization, channel=slack_channel, slack_id=ts + ) ssr.slack_message = slack_message ssr.save() @@ -198,17 +204,17 @@ class TestBaseShiftSwapRequestStep: mock_slack_client.chat_update.assert_not_called() @pytest.mark.django_db - def test_post_message_to_thread(self, setup, make_slack_message) -> None: + def test_post_message_to_thread(self, setup, make_slack_channel, make_slack_message) -> None: ts = "12345.67" blocks = [{"foo": "bar"}] ssr, _, _, _ = setup() - channel_id = "asdfadf" organization = ssr.organization slack_team_identity = organization.slack_team_identity + slack_channel = make_slack_channel(slack_team_identity) slack_message = make_slack_message( - alert_group=None, organization=organization, slack_id=ts, channel_id=channel_id + alert_group=None, organization=organization, slack_id=ts, channel=slack_channel ) step = scenarios.BaseShiftSwapRequestStep(slack_team_identity, organization) @@ -226,7 +232,7 @@ class TestBaseShiftSwapRequestStep: step.post_message_to_thread(ssr, blocks, True) mock_slack_client.chat_postMessage.assert_called_once_with( - channel=channel_id, + channel=slack_channel.slack_id, thread_ts=ts, reply_broadcast=True, blocks=blocks, diff --git a/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py b/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py index fbce8203..45384f53 100644 --- a/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py +++ b/engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py @@ -234,9 +234,7 @@ class TestSlackChannelMessageEventStep: thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group, slack_id=thread_ts, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) mock_permalink = "http://example.com" @@ -246,7 +244,7 @@ class TestSlackChannelMessageEventStep: payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "h" * 2901, @@ -290,9 +288,7 @@ class TestSlackChannelMessageEventStep: thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group, slack_id=thread_ts, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) step = SlackChannelMessageEventStep(slack_team_identity, organization, user) step._slack_client = Mock() @@ -302,7 +298,7 @@ class TestSlackChannelMessageEventStep: payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "h" * 2901, @@ -343,9 +339,7 @@ class TestSlackChannelMessageEventStep: thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - slack_channel_id = slack_channel.slack_id - - make_slack_message(alert_group, slack_id=thread_ts, channel_id=slack_channel_id) + make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) resolution_note_slack_message = None if resolution_note_slack_message_already_exists: @@ -361,7 +355,7 @@ class TestSlackChannelMessageEventStep: payload = { "event": { - "channel": slack_channel_id, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": new_text, @@ -489,9 +483,13 @@ class TestSlackChannelMessageEventStep: def test_slack_message_has_no_alert_group( self, make_organization_and_user_with_slack_identities, + make_slack_channel, make_slack_message, ) -> None: """Thread messages for SlackMessage instances without alert_group set (e.g., SSR Slack messages)""" + ts = 88945.4849 + thread_ts = 16789.123 + ( organization, user, @@ -499,21 +497,23 @@ class TestSlackChannelMessageEventStep: slack_user_identity, ) = make_organization_and_user_with_slack_identities() - channel = "potato" - ts = 88945.4849 - thread_ts = 16789.123 + slack_channel = make_slack_channel(slack_team_identity) + make_slack_message( + alert_group=None, + organization=organization, + slack_id=thread_ts, + channel=slack_channel, + ) payload = { "event": { - "channel": channel, + "channel": slack_channel.slack_id, "ts": ts, "thread_ts": thread_ts, "text": "hello", }, } - make_slack_message(alert_group=None, organization=organization, slack_id=thread_ts, channel_id=channel) - step = SlackChannelMessageEventStep(slack_team_identity, organization, user) step.process_scenario(slack_user_identity, slack_team_identity, payload) diff --git a/engine/apps/slack/tests/test_slack_message.py b/engine/apps/slack/tests/test_slack_message.py index 3f1d8f74..61419a1b 100644 --- a/engine/apps/slack/tests/test_slack_message.py +++ b/engine/apps/slack/tests/test_slack_message.py @@ -1,8 +1,31 @@ from unittest.mock import patch import pytest +from django.utils import timezone from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIError +from apps.slack.tests.conftest import build_slack_response + + +@pytest.fixture +def slack_message_setup( + make_organization_and_user_with_slack_identities, + make_alert_receive_channel, + make_alert_group, + make_slack_channel, + make_slack_message, +): + def _slack_message_setup(cached_permalink): + organization, _, slack_team_identity, _ = make_organization_and_user_with_slack_identities() + integration = make_alert_receive_channel(organization) + alert_group = make_alert_group(integration) + slack_channel = make_slack_channel(slack_team_identity) + + return make_slack_message(alert_group=alert_group, channel=slack_channel, cached_permalink=cached_permalink) + + return _slack_message_setup @pytest.mark.django_db @@ -28,7 +51,7 @@ def test_send_slack_notification( make_alert(alert_group=alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) with patch("apps.slack.client.SlackClient.conversations_members") as mock_members: mock_members.return_value = {"members": [slack_user_identity.slack_id]} @@ -54,10 +77,63 @@ def test_slack_message_deep_link( make_alert(alert_group=alert_group, raw_request_data={}) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel_id=slack_channel.slack_id) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) expected = ( f"https://slack.com/app_redirect?channel={slack_channel.slack_id}" f"&team={slack_team_identity.slack_id}&message={slack_message.slack_id}" ) assert slack_message.deep_link == expected + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), +) +@pytest.mark.django_db +def test_slack_message_permalink(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + assert slack_message.permalink == "test_permalink" + mock_slack_api_call.assert_called_once() + + +@patch.object( + SlackClient, + "chat_getPermalink", + side_effect=SlackAPIError(response=build_slack_response({"ok": False, "error": "message_not_found"})), +) +@pytest.mark.django_db +def test_slack_message_permalink_error(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + assert slack_message.permalink is None + mock_slack_api_call.assert_called_once() + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), +) +@pytest.mark.django_db +def test_slack_message_permalink_cache(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink="cached_permalink") + assert slack_message.permalink == "cached_permalink" + mock_slack_api_call.assert_not_called() + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": False, "error": "account_inactive"}), +) +@pytest.mark.django_db +def test_slack_message_permalink_token_revoked(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + slack_message.slack_team_identity.detected_token_revoked = timezone.now() + slack_message.slack_team_identity.save() + + assert slack_message.slack_team_identity is not None + assert slack_message.permalink is None + + mock_slack_api_call.assert_not_called() diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index 4eb0eb47..e64ea157 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -516,10 +516,13 @@ class SlackEventApiEndpointView(APIView): return None try: + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( - _slack_team_identity=slack_team_identity, slack_id=message_ts, - channel_id=channel_id, + organization__slack_team_identity=slack_team_identity, + _channel_id=channel_id, + # channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: return None diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index 1f4607e5..cdc552d8 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -50,6 +50,7 @@ def test_organization_hard_delete( make_team, make_slack_team_identity, make_slack_user_identity, + make_slack_channel, make_slack_message, make_schedule, make_custom_action, @@ -141,7 +142,8 @@ def test_organization_hard_delete( ) telegram_message = make_telegram_message(alert_group=alert_group, message_type=TelegramMessage.ALERT_GROUP_MESSAGE) - slack_message = make_slack_message(alert_group=alert_group) + slack_channel = make_slack_channel(slack_team_identity) + slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) plugin_token, _ = make_token_for_organization(organization) public_api_token, _ = make_public_api_token(user_1, organization) diff --git a/engine/conftest.py b/engine/conftest.py index 0b66e3ad..7ef25558 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -528,15 +528,16 @@ def make_slack_user_identity(): @pytest.fixture def make_slack_message(): - def _make_slack_message(alert_group=None, organization=None, **kwargs): - organization = organization or alert_group.channel.organization - slack_message = SlackMessageFactory( + def _make_slack_message(channel, alert_group=None, organization=None, **kwargs): + # TODO: once _channel_id has been fully migrated to channel, remove _channel_id + # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 + return SlackMessageFactory( alert_group=alert_group, - organization=organization, - _slack_team_identity=organization.slack_team_identity, + organization=organization or alert_group.channel.organization, + _channel_id=channel.slack_id, + channel=channel, **kwargs, ) - return slack_message return _make_slack_message diff --git a/engine/engine/management/commands/batch_migrate_slack_message_channel.py b/engine/engine/management/commands/batch_migrate_slack_message_channel.py new file mode 100644 index 00000000..2c302b94 --- /dev/null +++ b/engine/engine/management/commands/batch_migrate_slack_message_channel.py @@ -0,0 +1,99 @@ +import time + +from django.core.management.base import BaseCommand +from django.db import connection, transaction + +from apps.slack.models import SlackChannel, SlackMessage +from apps.user_management.models import Organization + + +class Command(BaseCommand): + help = "Batch updates SlackMessage.channel_id in chunks to avoid locking the table." + + def handle(self, *args, **options): + start_time = time.time() + self.stdout.write("Starting batch update of SlackMessage.channel_id...") + + # Step 1: Determine the queryset to update + # qs is ordered by id to ensure consistent batching + # since id is indexed, this ordering operation "should" be more efficient (as opposed to say created_at + # which we don't have an index on) + qs = SlackMessage.objects.filter( + _channel_id__isnull=False, # old column + organization__isnull=False, + channel_id__isnull=True, # new column + ).order_by("id") + + total_records = qs.count() + if total_records == 0: + self.stdout.write("No records to update.") + return + + self.stdout.write(f"Total records to update: {total_records}") + + # some considerations here.. + # + # Large IN clauses can be inefficient. Keep BATCH_SIZE reasonable (e.g., 1000) + # Fetching large batches of IDs consumes memory. With a BATCH_SIZE of 1000, this "should" be manageable + # + # references + # https://stackoverflow.com/a/5919165 + BATCH_SIZE = 1000 + + total_batches = (total_records + BATCH_SIZE - 1) // BATCH_SIZE + self.stdout.write(f"Batch size: {BATCH_SIZE}") + self.stdout.write(f"Total batches: {total_batches}") + + records_updated = 0 + batch_number = 1 + + # Process updates in batches + while True: + # Get the next batch of IDs + batch_qs = qs[:BATCH_SIZE] + + # collect the IDs to be updated + batch_ids = list(batch_qs.values_list("id", flat=True)) + + if not batch_ids: + break # No more records to process + + placeholders = ", ".join(["%s"] * len(batch_ids)) + update_query = f""" + UPDATE + {SlackMessage._meta.db_table} AS sm + INNER JOIN {Organization._meta.db_table} AS org + ON org.id = sm.organization_id + INNER JOIN {SlackChannel._meta.db_table} AS sc + ON sc.slack_id = sm._channel_id + AND sc.slack_team_identity_id = org.slack_team_identity_id + SET + sm.channel_id = sc.id + WHERE + sm.id IN ({placeholders}) + """ + params = batch_ids + + try: + # Execute the update + with transaction.atomic(): + with connection.cursor() as cursor: + cursor.execute(update_query, params) + batch_records_updated = cursor.rowcount + records_updated += batch_records_updated + + self.stdout.write(f"Batch {batch_number}/{total_batches}: Updated {batch_records_updated} records") + except Exception as e: + self.stderr.write(f"Error updating batch {batch_number}: {e}") + # Optionally, decide whether to continue or abort + continue + + # Remove processed records from queryset for next batch + qs = qs.exclude(id__in=batch_ids) + + batch_number += 1 + + end_time = time.time() + total_time = end_time - start_time + self.stdout.write(f"Batch update completed successfully. Total records updated: {records_updated}") + self.stdout.write(f"Total time taken: {total_time:.2f} seconds")