diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 3db1d9edfb..6bcb8d735f 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -417,6 +417,10 @@ def status(self) -> int: ) prevent_posting_alerts = models.BooleanField(default=False) + """ + TODO: this column is no longer used, drop it in a subsequent PR/release + """ + maintenance_uuid = models.CharField(max_length=100, unique=True, null=True, default=None) raw_escalation_snapshot = JSONField(null=True, default=None) @@ -1983,11 +1987,7 @@ def slack_channel_id(self) -> str | None: channel_filter = self.channel_filter if self.slack_message: - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # - # return self.slack_message.channel.slack_id - return self.slack_message._channel_id + return self.slack_message.channel.slack_id elif channel_filter and channel_filter.slack_channel_or_org_default: return channel_filter.slack_channel_or_org_default.slack_id return None diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index d4fdbf7710..a29ae9caa2 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -31,7 +31,7 @@ def test_render_for_phone_call( 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_slack_message(slack_channel, alert_group=alert_group) 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=slack_channel1) + slack_message = make_slack_message(slack_channel1, alert_group=alert_group) resolution_note_1 = make_resolution_note_slack_message( alert_group=alert_group, user=user, @@ -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=slack_channel1) + make_slack_message(slack_channel1, alert_group=alert_group) 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=slack_channel1) + make_slack_message(slack_channel1, alert_group=alert_group) make_resolution_note_slack_message( alert_group=alert_group, user=user, @@ -827,8 +827,8 @@ def test_slack_channel_id_with_slack_message( slack_channel = make_slack_channel(slack_team_identity) slack_message = make_slack_message(slack_channel, alert_group=alert_group) - # Assert that slack_channel_id returns the _channel_id from slack_message - assert alert_group.slack_channel_id == slack_message._channel_id + # Assert that slack_channel_id returns the channel.slack_id from slack_message + assert alert_group.slack_channel_id == slack_message.channel.slack_id @pytest.mark.django_db def test_slack_channel_id_with_channel_filter( diff --git a/engine/apps/alerts/tests/test_alert_receiver_channel.py b/engine/apps/alerts/tests/test_alert_receiver_channel.py index d690cd2fb9..930239826d 100644 --- a/engine/apps/alerts/tests/test_alert_receiver_channel.py +++ b/engine/apps/alerts/tests/test_alert_receiver_channel.py @@ -6,12 +6,10 @@ from django.conf import settings from django.db import IntegrityError from django.urls import reverse -from django.utils import timezone from apps.alerts.models import AlertReceiveChannel from common.api_helpers.utils import create_engine_url from common.exceptions import UnableToSendDemoAlert -from engine.management.commands import alertmanager_v2_migrate from settings.base import DatabaseTypes @@ -306,381 +304,3 @@ def test_create_duplicate_direct_paging_integrations(make_organization, make_tea integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, ) super(AlertReceiveChannel, arc).save() # bypass the custom save method, so that IntegrityError is raised - - -@pytest.mark.django_db -def test_alertmanager_v2_migrate_forward(make_organization, make_alert_receive_channel): - organization = make_organization() - - legacy_alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_LEGACY_ALERTMANAGER, - slack_title_template="slack_title_template", - web_title_template="web_title_template", - grouping_id_template="grouping_id_template", - resolve_condition_template="resolve_condition_template", - ) - - alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - slack_title_template="slack_title_template", - ) - legacy_grafana_alerting = make_alert_receive_channel( - organization, integration=AlertReceiveChannel.INTEGRATION_LEGACY_GRAFANA_ALERTING - ) - grafana_alerting = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - slack_title_template="slack_title_template", - ) - - alertmanager_v2_migrate.Command().handle(backward=False) - - legacy_alertmanager.refresh_from_db() - alertmanager.refresh_from_db() - legacy_grafana_alerting.refresh_from_db() - grafana_alerting.refresh_from_db() - - assert legacy_alertmanager.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert legacy_alertmanager.alertmanager_v2_migrated_at is not None - assert legacy_alertmanager.slack_title_template is None - assert legacy_alertmanager.web_title_template is None - assert legacy_alertmanager.grouping_id_template is None - assert legacy_alertmanager.resolve_condition_template is None - assert legacy_alertmanager.alertmanager_v2_backup_templates["slack_title_template"] == "slack_title_template" - assert legacy_alertmanager.alertmanager_v2_backup_templates["web_title_template"] == "web_title_template" - assert legacy_alertmanager.alertmanager_v2_backup_templates["grouping_id_template"] == "grouping_id_template" - assert ( - legacy_alertmanager.alertmanager_v2_backup_templates["resolve_condition_template"] - == "resolve_condition_template" - ) - assert legacy_alertmanager.alertmanager_v2_backup_templates["messaging_backends_templates"] is None - - assert legacy_grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert legacy_grafana_alerting.alertmanager_v2_migrated_at is not None - assert legacy_grafana_alerting.alertmanager_v2_backup_templates is None - - assert alertmanager.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert alertmanager.alertmanager_v2_migrated_at is None - assert alertmanager.slack_title_template == "slack_title_template" - assert alertmanager.alertmanager_v2_backup_templates is None - - assert grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert grafana_alerting.alertmanager_v2_migrated_at is None - assert grafana_alerting.slack_title_template == "slack_title_template" - assert grafana_alerting.alertmanager_v2_backup_templates is None - - -@pytest.mark.django_db -def test_alertmanager_v2_migrate_backward(make_organization, make_alert_receive_channel): - organization = make_organization() - - migrated_alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - alertmanager_v2_migrated_at=timezone.now(), - alertmanager_v2_backup_templates={ - "slack_title_template": "slack_title_template", - "web_title_template": "web_title_template", - "grouping_id_template": "grouping_id_template", - "resolve_condition_template": "resolve_condition_template", - }, - ) - - alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - slack_title_template="slack_title_template", - ) - migrated_grafana_alerting = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - alertmanager_v2_migrated_at=timezone.now(), - ) - grafana_alerting = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - slack_title_template="slack_title_template", - ) - - alertmanager_v2_migrate.Command().handle(backward=True) - - migrated_alertmanager.refresh_from_db() - alertmanager.refresh_from_db() - migrated_grafana_alerting.refresh_from_db() - grafana_alerting.refresh_from_db() - - assert migrated_alertmanager.integration == AlertReceiveChannel.INTEGRATION_LEGACY_ALERTMANAGER - assert migrated_alertmanager.alertmanager_v2_migrated_at is None - assert migrated_alertmanager.slack_title_template == "slack_title_template" - assert migrated_alertmanager.web_title_template == "web_title_template" - assert migrated_alertmanager.grouping_id_template == "grouping_id_template" - assert migrated_alertmanager.resolve_condition_template == "resolve_condition_template" - assert migrated_alertmanager.alertmanager_v2_backup_templates is None - - assert migrated_grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_LEGACY_GRAFANA_ALERTING - assert migrated_grafana_alerting.alertmanager_v2_migrated_at is None - assert migrated_grafana_alerting.slack_title_template is None - assert migrated_grafana_alerting.web_title_template is None - assert migrated_grafana_alerting.grouping_id_template is None - assert migrated_grafana_alerting.resolve_condition_template is None - assert migrated_grafana_alerting.alertmanager_v2_backup_templates is None - - assert alertmanager.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert alertmanager.alertmanager_v2_migrated_at is None - assert alertmanager.slack_title_template == "slack_title_template" - assert alertmanager.alertmanager_v2_backup_templates is None - - assert grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert grafana_alerting.alertmanager_v2_migrated_at is None - assert grafana_alerting.slack_title_template == "slack_title_template" - assert grafana_alerting.alertmanager_v2_backup_templates is None - - -@pytest.mark.django_db -def test_alertmanager_v2_migrate_forward_one(make_organization, make_alert_receive_channel): - organization = make_organization() - # org which is not going to be migrated - organization_2 = make_organization() - - legacy_alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_LEGACY_ALERTMANAGER, - slack_title_template="slack_title_template", - web_title_template="web_title_template", - grouping_id_template="grouping_id_template", - resolve_condition_template="resolve_condition_template", - ) - alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - slack_title_template="slack_title_template", - ) - legacy_grafana_alerting = make_alert_receive_channel( - organization, integration=AlertReceiveChannel.INTEGRATION_LEGACY_GRAFANA_ALERTING - ) - grafana_alerting = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - slack_title_template="slack_title_template", - ) - - # set up same set integrations for second org - legacy_alertmanager_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_LEGACY_ALERTMANAGER, - slack_title_template="slack_title_template", - web_title_template="web_title_template", - grouping_id_template="grouping_id_template", - resolve_condition_template="resolve_condition_template", - ) - alertmanager_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - slack_title_template="slack_title_template", - ) - legacy_grafana_alerting_2 = make_alert_receive_channel( - organization_2, integration=AlertReceiveChannel.INTEGRATION_LEGACY_GRAFANA_ALERTING - ) - grafana_alerting_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - slack_title_template="slack_title_template", - ) - - alertmanager_v2_migrate.Command().handle(backward=False, org_id=organization.id) - - legacy_alertmanager.refresh_from_db() - alertmanager.refresh_from_db() - legacy_grafana_alerting.refresh_from_db() - grafana_alerting.refresh_from_db() - - legacy_alertmanager_2.refresh_from_db() - alertmanager_2.refresh_from_db() - legacy_grafana_alerting_2.refresh_from_db() - grafana_alerting_2.refresh_from_db() - - assert legacy_alertmanager.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert legacy_alertmanager.alertmanager_v2_migrated_at is not None - assert legacy_alertmanager.slack_title_template is None - assert legacy_alertmanager.web_title_template is None - assert legacy_alertmanager.grouping_id_template is None - assert legacy_alertmanager.resolve_condition_template is None - assert legacy_alertmanager.alertmanager_v2_backup_templates["slack_title_template"] == "slack_title_template" - assert legacy_alertmanager.alertmanager_v2_backup_templates["web_title_template"] == "web_title_template" - assert legacy_alertmanager.alertmanager_v2_backup_templates["grouping_id_template"] == "grouping_id_template" - assert ( - legacy_alertmanager.alertmanager_v2_backup_templates["resolve_condition_template"] - == "resolve_condition_template" - ) - assert legacy_alertmanager.alertmanager_v2_backup_templates["messaging_backends_templates"] is None - - assert legacy_grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert legacy_grafana_alerting.alertmanager_v2_migrated_at is not None - assert legacy_grafana_alerting.alertmanager_v2_backup_templates is None - - assert alertmanager.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert alertmanager.alertmanager_v2_migrated_at is None - assert alertmanager.slack_title_template == "slack_title_template" - assert alertmanager.alertmanager_v2_backup_templates is None - - assert grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert grafana_alerting.alertmanager_v2_migrated_at is None - assert grafana_alerting.slack_title_template == "slack_title_template" - assert grafana_alerting.alertmanager_v2_backup_templates is None - - # check that second org is NOT affected - - # check that legacy alertmanager not affected - assert legacy_alertmanager_2.integration == AlertReceiveChannel.INTEGRATION_LEGACY_ALERTMANAGER - assert legacy_alertmanager_2.alertmanager_v2_migrated_at is None - assert legacy_alertmanager_2.slack_title_template == "slack_title_template" - assert legacy_alertmanager_2.web_title_template == "web_title_template" - assert legacy_alertmanager_2.grouping_id_template == "grouping_id_template" - assert legacy_alertmanager_2.resolve_condition_template == "resolve_condition_template" - assert legacy_alertmanager_2.alertmanager_v2_backup_templates is None - - # check that legacy grafana_alerting not affected - assert legacy_grafana_alerting_2.integration == AlertReceiveChannel.INTEGRATION_LEGACY_GRAFANA_ALERTING - assert legacy_grafana_alerting_2.alertmanager_v2_migrated_at is None - assert legacy_grafana_alerting_2.alertmanager_v2_backup_templates is None - - # check that alertmanager which shouldn't be affected even by migration not touched - assert alertmanager_2.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert alertmanager_2.alertmanager_v2_migrated_at is None - assert alertmanager_2.slack_title_template == "slack_title_template" - assert alertmanager_2.alertmanager_v2_backup_templates is None - - # same fpr grafana alerting - assert grafana_alerting_2.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert grafana_alerting_2.alertmanager_v2_migrated_at is None - assert grafana_alerting_2.slack_title_template == "slack_title_template" - assert grafana_alerting_2.alertmanager_v2_backup_templates is None - - -@pytest.mark.django_db -def test_alertmanager_v2_migrate_backward_one(make_organization, make_alert_receive_channel): - organization = make_organization() - - migrated_alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - alertmanager_v2_migrated_at=timezone.now(), - alertmanager_v2_backup_templates={ - "slack_title_template": "slack_title_template", - "web_title_template": "web_title_template", - "grouping_id_template": "grouping_id_template", - "resolve_condition_template": "resolve_condition_template", - }, - ) - - alertmanager = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - slack_title_template="slack_title_template", - ) - migrated_grafana_alerting = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - alertmanager_v2_migrated_at=timezone.now(), - ) - grafana_alerting = make_alert_receive_channel( - organization, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - slack_title_template="slack_title_template", - ) - - organization_2 = make_organization() - - migrated_alertmanager_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - alertmanager_v2_migrated_at=timezone.now(), - alertmanager_v2_backup_templates={ - "slack_title_template": "slack_title_template", - "web_title_template": "web_title_template", - "grouping_id_template": "grouping_id_template", - "resolve_condition_template": "resolve_condition_template", - }, - ) - alertmanager_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER, - slack_title_template="slack_title_template", - ) - migrated_grafana_alerting_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - alertmanager_v2_migrated_at=timezone.now(), - ) - grafana_alerting_2 = make_alert_receive_channel( - organization_2, - integration=AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING, - slack_title_template="slack_title_template", - ) - - alertmanager_v2_migrate.Command().handle(backward=True, org_id=organization.id) - - migrated_alertmanager.refresh_from_db() - alertmanager.refresh_from_db() - migrated_grafana_alerting.refresh_from_db() - grafana_alerting.refresh_from_db() - - assert migrated_alertmanager.integration == AlertReceiveChannel.INTEGRATION_LEGACY_ALERTMANAGER - assert migrated_alertmanager.alertmanager_v2_migrated_at is None - assert migrated_alertmanager.slack_title_template == "slack_title_template" - assert migrated_alertmanager.web_title_template == "web_title_template" - assert migrated_alertmanager.grouping_id_template == "grouping_id_template" - assert migrated_alertmanager.resolve_condition_template == "resolve_condition_template" - assert migrated_alertmanager.alertmanager_v2_backup_templates is None - - assert migrated_grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_LEGACY_GRAFANA_ALERTING - assert migrated_grafana_alerting.alertmanager_v2_migrated_at is None - assert migrated_grafana_alerting.slack_title_template is None - assert migrated_grafana_alerting.web_title_template is None - assert migrated_grafana_alerting.grouping_id_template is None - assert migrated_grafana_alerting.resolve_condition_template is None - assert migrated_grafana_alerting.alertmanager_v2_backup_templates is None - - assert alertmanager.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert alertmanager.alertmanager_v2_migrated_at is None - assert alertmanager.slack_title_template == "slack_title_template" - assert alertmanager.alertmanager_v2_backup_templates is None - - assert grafana_alerting.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert grafana_alerting.alertmanager_v2_migrated_at is None - assert grafana_alerting.slack_title_template == "slack_title_template" - assert grafana_alerting.alertmanager_v2_backup_templates is None - - migrated_alertmanager_2.refresh_from_db() - alertmanager_2.refresh_from_db() - migrated_grafana_alerting_2.refresh_from_db() - grafana_alerting_2.refresh_from_db() - - # check that migrated integrations is second org were not touced by backward migration of other org - assert migrated_alertmanager_2.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert migrated_alertmanager_2.alertmanager_v2_migrated_at is not None - assert migrated_alertmanager_2.slack_title_template is None - assert migrated_alertmanager_2.web_title_template is None - assert migrated_alertmanager_2.grouping_id_template is None - assert migrated_alertmanager_2.resolve_condition_template is None - assert migrated_alertmanager_2.alertmanager_v2_backup_templates is not None - - assert migrated_grafana_alerting_2.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert migrated_grafana_alerting_2.alertmanager_v2_migrated_at is not None - assert migrated_grafana_alerting_2.slack_title_template is None - assert migrated_grafana_alerting_2.web_title_template is None - assert migrated_grafana_alerting_2.grouping_id_template is None - assert migrated_grafana_alerting_2.resolve_condition_template is None - assert migrated_grafana_alerting_2.alertmanager_v2_backup_templates is None - - assert alertmanager_2.integration == AlertReceiveChannel.INTEGRATION_ALERTMANAGER - assert alertmanager_2.alertmanager_v2_migrated_at is None - assert alertmanager_2.slack_title_template == "slack_title_template" - assert alertmanager_2.alertmanager_v2_backup_templates is None - - assert grafana_alerting_2.integration == AlertReceiveChannel.INTEGRATION_GRAFANA_ALERTING - assert grafana_alerting_2.alertmanager_v2_migrated_at is None - assert grafana_alerting_2.slack_title_template == "slack_title_template" - assert grafana_alerting_2.alertmanager_v2_backup_templates is None diff --git a/engine/apps/alerts/tests/test_notify_user.py b/engine/apps/alerts/tests/test_notify_user.py index 4a4b92a309..e97a577b63 100644 --- a/engine/apps/alerts/tests/test_notify_user.py +++ b/engine/apps/alerts/tests/test_notify_user.py @@ -265,7 +265,7 @@ def test_perform_notification_reason_to_skip_escalation_in_slack( if not error_code: slack_channel = make_slack_channel(slack_team_identity=slack_team_identity) - make_slack_message(alert_group=alert_group, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group) with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification: perform_notification(log_record.pk, False) @@ -314,7 +314,7 @@ def test_perform_notification_slack_prevent_posting( ) slack_channel = make_slack_channel(slack_team_identity=slack_team_identity) - make_slack_message(alert_group=alert_group, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group) 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 2b79f478ec..247246ddca 100644 --- a/engine/apps/public_api/tests/test_alert_groups.py +++ b/engine/apps/public_api/tests/test_alert_groups.py @@ -162,7 +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=slack_channel, cached_permalink="the-link") + slack_message = make_slack_message(slack_channel, alert_group=alert_group, 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/tests/tasks/shift_swaps/test_slack_followups.py b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py index d0be1dc063..dddac77e21 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,9 +29,7 @@ def _shift_swap_request_test_setup(swap_start=None, swap_end=None, **kwargs): user = make_user(organization=organization) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message( - alert_group=None, channel=slack_channel, organization=organization, slack_id="12345" - ) + slack_message = make_slack_message(slack_channel) schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, slack_channel=slack_channel) 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 da57d29124..7e625a46ca 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 @@ -44,9 +44,7 @@ def test_create_shift_swap_request_message_post_message_to_channel_called( 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" - ) + slack_message = make_slack_message(slack_channel) MockBaseShiftSwapRequestStep.return_value.create_message.return_value = slack_message diff --git a/engine/apps/slack/0009_remove_slackmessage_active_update_task_id_db.py b/engine/apps/slack/0009_remove_slackmessage_active_update_task_id_db.py new file mode 100644 index 0000000000..15617dfda2 --- /dev/null +++ b/engine/apps/slack/0009_remove_slackmessage_active_update_task_id_db.py @@ -0,0 +1,22 @@ +# NOTE: this is being left in this directory on purpose, it will be moved to apps/alerts/migrations +# in a separate PR/release +# +# Generated by Django 4.2.16 on 2024-12-04 12:00 + +from django.db import migrations + +import common.migrations.remove_field + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0008_remove_slackmessage_active_update_task_id_state"), + ] + + operations = [ + common.migrations.remove_field.RemoveFieldDB( + model_name="SlackMessage", + name="active_update_task_id", + remove_state_migration=("slack", "0008_remove_slackmessage_active_update_task_id_state"), + ), + ] diff --git a/engine/apps/slack/alert_group_slack_service.py b/engine/apps/slack/alert_group_slack_service.py index 019a6e217c..512b50f49d 100644 --- a/engine/apps/slack/alert_group_slack_service.py +++ b/engine/apps/slack/alert_group_slack_service.py @@ -48,10 +48,7 @@ def publish_message_to_alert_group_thread( try: result = self._slack_client.chat_postMessage( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=slack_message.channel.slack_id, - channel=slack_message._channel_id, + channel=slack_message.channel.slack_id, text=text, attachments=attachments, thread_ts=slack_message.slack_id, @@ -66,11 +63,8 @@ def publish_message_to_alert_group_thread( ): return - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 alert_group.slack_messages.create( slack_id=result["ts"], - organization=alert_group.channel.organization, - _channel_id=slack_message.channel.slack_id, + _slack_team_identity=self.slack_team_identity, channel=slack_message.channel, ) diff --git a/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py b/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py index 35b7ed81ba..63206d628b 100644 --- a/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py +++ b/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py @@ -1,7 +1,3 @@ -# 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 diff --git a/engine/apps/slack/migrations/0008_remove_slackmessage_active_update_task_id_state.py b/engine/apps/slack/migrations/0008_remove_slackmessage_active_update_task_id_state.py new file mode 100644 index 0000000000..eaa7d5bef9 --- /dev/null +++ b/engine/apps/slack/migrations/0008_remove_slackmessage_active_update_task_id_state.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.16 on 2024-12-04 12:00 + +import common.migrations.remove_field +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('slack', '0007_migrate_slackmessage_channel_id'), + ] + + operations = [ + common.migrations.remove_field.RemoveFieldState( + model_name='SlackMessage', + name='active_update_task_id', + ), + ] diff --git a/engine/apps/slack/migrations/0009_drop_orphaned_messages_and_fill_in_missing_team_identity_values.py b/engine/apps/slack/migrations/0009_drop_orphaned_messages_and_fill_in_missing_team_identity_values.py new file mode 100644 index 0000000000..cff04a88cb --- /dev/null +++ b/engine/apps/slack/migrations/0009_drop_orphaned_messages_and_fill_in_missing_team_identity_values.py @@ -0,0 +1,137 @@ +# Generated by Django 4.2.16 on 2024-12-04 21:17 + +import logging + +from django.db import migrations +from django.db.models import Subquery, OuterRef + +logger = logging.getLogger(__name__) + + +def drop_orphaned_slack_messages(apps, schema_editor): + """ + At this point, in the 0007_migrate_slackmessage_channel_id migration, we have already migrated the old + SlackMessage._channel_id (char field) to the new SlackMessage.channel_id (FK field). + + This script is needed to identity, and drop, SlackMessage records that are "orphaned". This would've occured in two + cases: + - the organization switched their Slack team identity. We have historically never done clean-up of the old SlackMessage records + - a Slack channel was deleted. Previously, we did not have a FK constraint on the SlackMessage._channel_id + char field, so there was no CASCADE delete behaviour (now there is). + + In both of these scenarios, the SlackMessage records would be "orphaned" - they would have a non-null channel_id + foreign key relationship, which prevents us from setting NOT NULL constraint on the channel_id field. + """ + SlackMessage = apps.get_model("slack", "SlackMessage") + ShiftSwapRequest = apps.get_model("schedules", "ShiftSwapRequest") + + logger.info("Starting migration to drop orphaned SlackMessage records.") + + slack_messages = SlackMessage.objects.filter(channel__isnull=True) + + total_messages = slack_messages.count() + if total_messages == 0: + logger.info("No orphaned SlackMessages need to be dropped.") + return + + logger.info(f"Found {total_messages} orphaned SlackMessages to drop.") + + # Set the referencing ShiftSwapRequests' slack_message_id field to NULL (only for orphaned SlackMessages) + ShiftSwapRequest.objects.filter(slack_message__in=slack_messages).update(slack_message=None) + logger.info("Unset slack_message references from ShiftSwapRequests.") + + slack_messages.delete() + + logger.info("Finished migration to drop orphaned SlackMessages.") + + +def fill_in_missing_slack_team_identity_values(apps, schema_editor): + """ + This migration fills in the missing `slack_team_identity` field for SlackMessage records that were + created before the field was added. This field is required for the SlackMessage model to be valid, and it is + necessary for the SlackMessage model to be associated with a SlackChannel model. + """ + SlackMessage = apps.get_model("slack", "SlackMessage") + SlackChannel = apps.get_model("slack", "SlackChannel") + + logger.info("Starting migration to fill in missing SlackMessage.slack_team_identity values") + + if schema_editor.connection.vendor == "mysql": + # Use raw SQL for MySQL + with schema_editor.connection.cursor() as cursor: + # Update SlackMessages by joining with SlackChannel + sql_update = """ + UPDATE slack_slackmessage sm + JOIN slack_slackchannel sc ON sm.channel_id = sc.id + SET sm.slack_team_identity = sc.slack_team_identity_id + WHERE sm.slack_team_identity IS NULL + AND sm.channel_id IS NOT NULL + AND sc.slack_team_identity_id IS NOT NULL; + """ + cursor.execute(sql_update) + updated_rows = cursor.rowcount # Number of rows updated + + logger.info(f"Updated {updated_rows} SlackMessages with slack_team_identity from slack_channel.") + + # Count remaining messages that couldn't be updated + cursor.execute(""" + SELECT COUNT(*) + FROM slack_slackmessage sm + LEFT JOIN slack_slackchannel sc ON sm.channel_id = sc.id + WHERE sm.slack_team_identity IS NULL + AND (sm.channel_id IS NULL OR sc.slack_team_identity_id IS NULL); + """) + remaining_count = cursor.fetchone()[0] + + if remaining_count > 0: + logger.warning( + f"{remaining_count} SlackMessages could not be updated because slack_channel or slack_channel.slack_team_identity is missing." + ) + else: + # Use Django ORM for other databases + slack_messages = SlackMessage.objects.filter(_slack_team_identity__isnull=True) + + total_messages = slack_messages.count() + if total_messages == 0: + logger.info("No missing SlackMessage.slack_team_identity values") + return + + logger.info(f"Found {total_messages} SlackMessages which have missing slack_team_identity values.") + + # Subquery to get slack_team_identity from SlackChannel + channel_team_identity_subquery = SlackChannel.objects.filter( + pk=OuterRef('channel_id') + ).values('slack_team_identity_id')[:1] + + # Update from slack_channel + updated_count = slack_messages.filter( + channel__isnull=False, + channel__slack_team_identity__isnull=False + ).update( + _slack_team_identity=Subquery(channel_team_identity_subquery) + ) + + logger.info(f"Updated {updated_count} SlackMessages with slack_team_identity from slack_channel.") + + # Check if there are any SlackMessages that couldn't be updated + remaining_messages = SlackMessage.objects.filter(_slack_team_identity__isnull=True) + remaining_count = remaining_messages.count() + + if remaining_count > 0: + logger.warning( + f"{remaining_count} SlackMessages could not be updated because slack_channel or slack_channel.slack_team_identity is missing." + ) + + logger.info("Finished migration to fill in missing SlackMessage.slack_team_identity values") + + +class Migration(migrations.Migration): + + dependencies = [ + ('slack', '0008_remove_slackmessage_active_update_task_id_state'), + ] + + operations = [ + migrations.RunPython(drop_orphaned_slack_messages, migrations.RunPython.noop), + migrations.RunPython(fill_in_missing_slack_team_identity_values, migrations.RunPython.noop), + ] diff --git a/engine/apps/slack/models/slack_message.py b/engine/apps/slack/models/slack_message.py index 3355b701c1..16b032e2a6 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -23,7 +23,7 @@ if typing.TYPE_CHECKING: from apps.alerts.models import AlertGroup from apps.base.models import UserNotificationPolicy - from apps.slack.models import SlackChannel + from apps.slack.models import SlackChannel, SlackTeamIdentity from apps.user_management.models import User logger = logging.getLogger(__name__) @@ -48,8 +48,8 @@ class SlackMessage(models.Model): "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 + TODO: set null=False + remove default=None in a subsequent PR/release. + (a slack message always needs to have a slack channel associated with it) """ organization = models.ForeignKey( @@ -59,6 +59,12 @@ class SlackMessage(models.Model): default=None, related_name="slack_message", ) + """ + DEPRECATED/TODO: drop this field in a separate PR/release + + For alert group related slack messages, we can always get the organization from the alert_group.channel + For shift swap request related slack messages, the schedule has a reference to the organization + """ _slack_team_identity = models.ForeignKey( "slack.SlackTeamIdentity", @@ -69,9 +75,13 @@ class SlackMessage(models.Model): db_column="slack_team_identity", ) """ - DEPRECATED/TODO: drop this field in a separate PR/release + TODO: rename this from _slack_team_identity to slack_team_identity in a subsequent PR/release + + This involves also updating the Meta.constraints to use the new field name, this may involve + migrations.RemoveConstraint and migrations.AddConstraint operations, which we need to investigate further... - Instead of using this column, we can simply do self.organization.slack_team_identity + Also, set null=False + remove default in a subsequent PR/release (a slack message always needs to have + a slack team identity associated with it) """ ack_reminder_message_ts = models.CharField(max_length=100, null=True, default=None) @@ -89,11 +99,6 @@ class SlackMessage(models.Model): related_name="slack_messages", ) - active_update_task_id = models.CharField(max_length=100, null=True, default=None) - """ - DEPRECATED/TODO: drop this field in a separate PR/release - """ - class Meta: # slack_id is unique within the context of a channel or conversation constraints = [ @@ -101,8 +106,11 @@ class Meta: ] @property - def slack_team_identity(self): - return self.organization.slack_team_identity + def slack_team_identity(self) -> "SlackTeamIdentity": + """ + See TODO note under _slack_team_identity field + """ + return self._slack_team_identity @property def permalink(self) -> typing.Optional[str]: @@ -112,10 +120,7 @@ def permalink(self) -> typing.Optional[str]: try: result = SlackClient(self.slack_team_identity).chat_getPermalink( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=self.channel.slack_id, - channel=self._channel_id, + channel=self.channel.slack_id, message_ts=self.slack_id, ) except SlackAPIError: @@ -128,13 +133,10 @@ def permalink(self) -> typing.Optional[str]: @property def deep_link(self) -> str: - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - 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" + self, 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 @@ -148,7 +150,7 @@ def send_slack_notification( slack_message = alert_group.slack_message slack_channel = slack_message.channel - organization = alert_group.channel.organization + slack_team_identity = self.slack_team_identity channel_id = slack_channel.slack_id user_verbal = user.get_username_with_slack_verbal(mention=True) @@ -185,7 +187,7 @@ def send_slack_notification( } ] - sc = SlackClient(organization.slack_team_identity, enable_ratelimit_retry=True) + sc = SlackClient(slack_team_identity, enable_ratelimit_retry=True) try: result = sc.chat_postMessage( @@ -232,12 +234,9 @@ def send_slack_notification( ).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=organization, - _channel_id=slack_channel.slack_id, + _slack_team_identity=slack_team_identity, channel=slack_channel, ) diff --git a/engine/apps/slack/models/slack_user_identity.py b/engine/apps/slack/models/slack_user_identity.py index 7d04090b53..5788253634 100644 --- a/engine/apps/slack/models/slack_user_identity.py +++ b/engine/apps/slack/models/slack_user_identity.py @@ -132,12 +132,9 @@ def send_link_to_slack_message(self, slack_message: "SlackMessage"): "elements": [ { "type": "mrkdwn", - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # f"<#{slack_message.channel.slack_id}>.\n" "text": ( f"You received this message because you're not a member of " - f"<#{slack_message._channel_id}>.\n" + 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 1a201ecd39..2f7a0113fe 100644 --- a/engine/apps/slack/scenarios/alertgroup_appearance.py +++ b/engine/apps/slack/scenarios/alertgroup_appearance.py @@ -87,10 +87,7 @@ def process_scenario( slack_message = alert_group.slack_message self._slack_client.chat_update( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=slack_message.channel.slack_id, - channel=slack_message._channel_id, + 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 8be2095f05..66a9901d0a 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -158,12 +158,9 @@ def process_signal(self, alert: Alert) -> None: blocks=alert_group.render_slack_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, - _channel_id=slack_channel.slack_id, + _slack_team_identity=slack_team_identity, channel=slack_channel, ) @@ -554,10 +551,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: if slack_user_identity: self._slack_client.chat_postEphemeral( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=alert_group.slack_message.channel.slack_id, - channel=alert_group.slack_message._channel_id, + channel=alert_group.slack_message.channel.slack_id, user=slack_user_identity.slack_id, text="{}{}".format(ephemeral_text[:1].upper(), ephemeral_text[1:]), unfurl_links=True, @@ -804,10 +798,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: if slack_message.ack_reminder_message_ts: try: self._slack_client.chat_update( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=slack_message.channel.slack_id, - channel=slack_message._channel_id, + channel=slack_message.channel.slack_id, ts=slack_message.ack_reminder_message_ts, text=text, attachments=message_attachments, @@ -921,12 +912,9 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: 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=organization, - _channel_id=slack_channel.slack_id, + _slack_team_identity=slack_message.slack_team_identity, channel=slack_channel, ) @@ -981,13 +969,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None: # Remove alert group Slack messages for message in alert_group.slack_messages.all(): try: - self._slack_client.chat_delete( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=message.channel.slack_id, - 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 fc3992311d..d77286ad62 100644 --- a/engine/apps/slack/scenarios/notification_delivery.py +++ b/engine/apps/slack/scenarios/notification_delivery.py @@ -18,11 +18,7 @@ def process_signal(self, log_record: "AlertGroupLogRecord") -> None: user = log_record.author alert_group = log_record.alert_group - - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # slack_channel_id = alert_group_slack_message.channel.slack_id - slack_channel_id = alert_group.slack_message._channel_id + slack_channel_id = alert_group.slack_message.channel.slack_id user_verbal_with_mention = user.get_username_with_slack_verbal(mention=True) diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index 9fe9536871..f1a557be7e 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -92,13 +92,10 @@ def process_scenario( return try: - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=payload["message"]["thread_ts"], - organization__slack_team_identity=slack_team_identity, - _channel_id=channel_id, - # channel__slack_id=channel_id, + _slack_team_identity=slack_team_identity, + channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: if settings.UNIFIED_SLACK_APP_ENABLED: @@ -164,14 +161,10 @@ def process_scenario( slack_channel = SlackChannel.objects.get( slack_id=channel_id, slack_team_identity=slack_team_identity ) - - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=thread_ts, - organization__slack_team_identity=slack_team_identity, - # channel__slack_id=channel_id, - _channel_id=channel_id, + _slack_team_identity=slack_team_identity, + channel__slack_id=channel_id, ) alert_group = slack_message.alert_group @@ -262,14 +255,10 @@ def post_or_update_resolution_note_in_thread(self, resolution_note: "ResolutionN resolution_note_slack_message = resolution_note.resolution_note_slack_message alert_group = resolution_note.alert_group alert_group_slack_message = alert_group.slack_message + slack_channel_id = alert_group_slack_message.channel.slack_id blocks = self.get_resolution_note_blocks(resolution_note) - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # slack_channel_id = alert_group_slack_message.channel.slack_id - slack_channel_id = alert_group_slack_message._channel_id - slack_channel = SlackChannel.objects.get( slack_id=slack_channel_id, slack_team_identity=self.slack_team_identity ) diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index 48637131b7..5dd2142293 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -161,12 +161,9 @@ def create_message(self, shift_swap_request: "ShiftSwapRequest") -> SlackMessage 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=self.organization, - _channel_id=shift_swap_request.slack_channel_id, + _slack_team_identity=self.slack_team_identity, channel=shift_swap_request.slack_channel, ) @@ -186,11 +183,8 @@ def post_message_to_thread( if not shift_swap_request.slack_message: return - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 self._slack_client.chat_postMessage( - # channel=shift_swap_request.slack_message.channel.slack_id, - 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 140b2af7b3..158c289c17 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -66,13 +66,10 @@ def save_thread_message_for_resolution_note( message_ts = payload["event"]["ts"] try: - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=thread_ts, - organization__slack_team_identity=self.slack_team_identity, - _channel_id=channel_id, - # channel__slack_id=channel_id, + _slack_team_identity=self.slack_team_identity, + 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 8813ac32ed..12576ad6a5 100644 --- a/engine/apps/slack/scenarios/step_mixins.py +++ b/engine/apps/slack/scenarios/step_mixins.py @@ -63,12 +63,9 @@ def _repair_alert_group( 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, - _channel_id=slack_channel.slack_id, + _slack_team_identity=slack_team_identity, channel=slack_channel, alert_group=alert_group, ) @@ -179,12 +176,9 @@ def _get_alert_group_from_slack_message_in_db( logger.warning(f"alert_group_pk not found in payload, fetching SlackMessage from DB. message_ts: {message_ts}") # Get SlackMessage from DB - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=message_ts, - organization__slack_team_identity=slack_team_identity, - _channel_id=channel_id, - # channel__slack_id=channel_id, + _slack_team_identity=slack_team_identity, + channel__slack_id=channel_id, ) return slack_message.alert_group diff --git a/engine/apps/slack/tasks.py b/engine/apps/slack/tasks.py index deff2cba28..385ff60181 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -98,10 +98,7 @@ def update_alert_group_slack_message(slack_message_pk: int) -> None: try: slack_client.chat_update( - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p173255546 - # channel=slack_message.channel.slack_id, - channel=slack_message._channel_id, + channel=slack_message.channel.slack_id, ts=slack_message.slack_id, attachments=alert_group.render_slack_attachments(), blocks=alert_group.render_slack_blocks(), @@ -127,27 +124,6 @@ def update_alert_group_slack_message(slack_message_pk: int) -> None: slack_message.mark_active_update_task_as_complete() -@shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True) -def update_incident_slack_message(slack_team_identity_pk: int, alert_group_pk: int) -> None: - """ - TODO: this method has been deprecated, and all references to it removed, remove it once task queues no - longer reference it. - """ - from apps.alerts.models import AlertGroup - - alert_group = AlertGroup.objects.get(pk=alert_group_pk) - - # NOTE: alert_group can't be None here, AlertGroup.objects.get(pk=alert_group_pk) would - # raise AlertGroup.DoesNotExist in this case - if not alert_group.slack_message: - logger.info( - f"skipping update_incident_slack_message as AlertGroup {alert_group_pk} doesn't have a slack message" - ) - return - - alert_group.slack_message.update_alert_groups_message(debounce=False) - - @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True) def check_slack_message_exists_before_post_message_to_thread( alert_group_pk, 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 6898428911..7320492f58 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 @@ -254,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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) payload = { "message_ts": slack_message.slack_id, @@ -271,15 +271,13 @@ def test_get_alert_group_from_slack_message_in_db( @pytest.mark.django_db def test_get_alert_group_from_slack_message_in_db_no_alert_group( make_organization_and_user_with_slack_identities, - make_alert_receive_channel, - make_alert_group, make_slack_channel, make_slack_message, ): 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=slack_channel) + slack_message = make_slack_message(slack_channel) payload = { "message_ts": slack_message.slack_id, @@ -329,7 +327,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -375,7 +373,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -421,7 +419,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -467,7 +465,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -519,7 +517,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -577,7 +575,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, slack_id=SLACK_MESSAGE_TS) invitation = make_invitation(alert_group, user, second_user, pk=INVITATION_ID) @@ -630,7 +628,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -681,7 +679,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -721,7 +719,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -775,7 +773,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=slack_channel, slack_id=SLACK_MESSAGE_TS) + make_slack_message(slack_channel, alert_group=alert_group, 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) @@ -826,7 +824,7 @@ def test_step_format_alert( make_alert(alert_group, raw_request_data={}) 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) + make_slack_message(slack_channel, alert_group=alert_group, 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) 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 922a666406..ed6d33b95c 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=slack_channel, slack_id=MESSAGE_TS) + make_slack_message(slack_channel, slack_id=MESSAGE_TS, alert_group=alert_group) 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 28124202b5..e1523f6643 100644 --- a/engine/apps/slack/tests/scenario_steps/test_resolution_note.py +++ b/engine/apps/slack/tests/scenario_steps/test_resolution_note.py @@ -153,7 +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) - make_slack_message(alert_group=alert_group, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group) resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000) @@ -189,7 +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) - make_slack_message(alert_group=alert_group, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group) resolution_note = make_resolution_note(alert_group=alert_group, author=user, message_text="a" * 3000) make_resolution_note_slack_message( @@ -319,7 +319,7 @@ def test_resolution_notes_modal_closed_before_update( 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, slack_id="RANDOM_MESSAGE_ID") + make_slack_message(slack_channel, alert_group=alert_group) payload = { "trigger_id": "TEST", @@ -368,7 +368,7 @@ def test_add_to_resolution_note( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) payload = { "channel": {"id": slack_channel.slack_id}, @@ -436,7 +436,7 @@ def test_add_to_resolution_note_deleted_org( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) organization.delete() other_organization = make_organization(slack_team_identity=slack_team_identity) 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 906b856fa9..2e78341c86 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 @@ -160,7 +160,6 @@ def test_create_message(self, mock_generate_blocks, setup) -> None: ) assert slack_message.slack_id == ts - assert slack_message.organization == organization assert slack_message.channel.slack_id == ssr.slack_channel_id assert slack_message.slack_team_identity == slack_team_identity @@ -174,9 +173,7 @@ def test_update_message(self, mock_generate_blocks, setup, make_slack_channel, m 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, channel=slack_channel, slack_id=ts - ) + slack_message = make_slack_message(slack_channel, slack_id=ts) ssr.slack_message = slack_message ssr.save() @@ -213,9 +210,7 @@ def test_post_message_to_thread(self, setup, make_slack_channel, make_slack_mess 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=slack_channel - ) + slack_message = make_slack_message(slack_channel, slack_id=ts) step = scenarios.BaseShiftSwapRequestStep(slack_team_identity, organization) diff --git a/engine/apps/slack/tests/scenario_steps/test_slack_channel.py b/engine/apps/slack/tests/scenario_steps/test_slack_channel.py index a0205e7e1a..a79a77b330 100644 --- a/engine/apps/slack/tests/scenario_steps/test_slack_channel.py +++ b/engine/apps/slack/tests/scenario_steps/test_slack_channel.py @@ -3,7 +3,7 @@ import pytest from django.utils import timezone -from apps.slack.models import SlackChannel +from apps.slack.models import SlackChannel, SlackMessage from apps.slack.scenarios import slack_channel as slack_channel_scenarios @@ -84,6 +84,7 @@ def test_process_scenario_channel_deleted( self, make_organization_and_user_with_slack_identities, make_slack_channel, + make_slack_message, ) -> None: ( organization, @@ -92,6 +93,7 @@ def test_process_scenario_channel_deleted( slack_user_identity, ) = make_organization_and_user_with_slack_identities() slack_channel = make_slack_channel(slack_team_identity) + make_slack_message(slack_channel) slack_channel_id = slack_channel.slack_id # Ensure the SlackChannel exists @@ -100,6 +102,8 @@ def test_process_scenario_channel_deleted( slack_team_identity=slack_team_identity, ).exists() + assert SlackMessage.objects.count() == 1 + step = slack_channel_scenarios.SlackChannelDeletedEventStep(slack_team_identity, organization, user) step.process_scenario(slack_user_identity, slack_team_identity, {"event": {"channel": slack_channel_id}}) @@ -109,6 +113,9 @@ def test_process_scenario_channel_deleted( slack_team_identity=slack_team_identity, ).exists() + # Slack messages should be cascade deleted when their channel is deleted + assert SlackMessage.objects.count() == 0 + def test_process_scenario_channel_does_not_exist( self, make_organization_and_user_with_slack_identities, 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 a081cb0e06..3e2f4f6afd 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,7 +234,7 @@ def test_save_thread_message_for_resolution_note_really_long_text( thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group, slack_id=thread_ts) mock_permalink = "http://example.com" @@ -288,7 +288,7 @@ def test_save_thread_message_for_resolution_note_api_errors( thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group, slack_id=thread_ts) step = SlackChannelMessageEventStep(slack_team_identity, organization, user) step._slack_client = Mock() @@ -339,7 +339,7 @@ def test_save_thread_message_for_resolution_note( thread_ts = 16789.123 slack_channel = make_slack_channel(slack_team_identity) - make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group, slack_id=thread_ts) resolution_note_slack_message = None if resolution_note_slack_message_already_exists: @@ -447,7 +447,7 @@ def test_delete_thread_message_from_resolution_note( slack_channel = make_slack_channel(slack_team_identity, slack_id=channel_id) integration = make_alert_receive_channel(organization) alert_group = make_alert_group(integration) - make_slack_message(alert_group=alert_group, slack_id=thread_ts, channel=slack_channel) + make_slack_message(slack_channel, alert_group=alert_group, slack_id=thread_ts) payload = { "event": { @@ -495,12 +495,7 @@ def test_slack_message_has_no_alert_group( ) = make_organization_and_user_with_slack_identities() slack_channel = make_slack_channel(slack_team_identity) - make_slack_message( - alert_group=None, - organization=organization, - slack_id=thread_ts, - channel=slack_channel, - ) + make_slack_message(slack_channel, slack_id=thread_ts) payload = { "event": { diff --git a/engine/apps/slack/tests/tasks/test_update_alert_group_slack_message.py b/engine/apps/slack/tests/tasks/test_update_alert_group_slack_message.py index ee03833229..bd5b59a76b 100644 --- a/engine/apps/slack/tests/tasks/test_update_alert_group_slack_message.py +++ b/engine/apps/slack/tests/tasks/test_update_alert_group_slack_message.py @@ -53,7 +53,7 @@ def test_update_alert_group_slack_message_task_id_mismatch( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("original-task-id") update_alert_group_slack_message.apply((slack_message.pk,), task_id="different-task-id") @@ -73,9 +73,9 @@ def test_update_alert_group_slack_message_no_alert_group( """ Test that the task exits early if SlackMessage has no alert_group. """ - organization, slack_team_identity = make_organization_with_slack_team_identity() + _, slack_team_identity = make_organization_with_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_message = make_slack_message(slack_channel) update_alert_group_slack_message.apply((slack_message.pk,), task_id="task-id") @@ -106,7 +106,7 @@ def test_update_alert_group_slack_message_skip_escalation_in_slack( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") # Ensure skip_escalation_in_slack is True @@ -146,7 +146,7 @@ def test_update_alert_group_slack_message_alert_receive_channel_rate_limited( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") # Ensure is_rate_limited_in_slack is True @@ -183,14 +183,14 @@ def test_update_alert_group_slack_message_successful( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") update_alert_group_slack_message.apply((slack_message.pk,), task_id="task-id") # Assert that SlackClient.chat_update was called with correct parameters mock_chat_update.assert_called_once_with( - channel=slack_message._channel_id, + channel=slack_message.channel.slack_id, ts=slack_message.slack_id, attachments=alert_group.render_slack_attachments(), blocks=alert_group.render_slack_blocks(), @@ -230,7 +230,7 @@ def test_update_alert_group_slack_message_ratelimit_error_not_maintenance( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") # SlackClient.chat_update raises SlackAPIRatelimitError @@ -271,7 +271,7 @@ def test_update_alert_group_slack_message_ratelimit_error_is_maintenance( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") # SlackClient.chat_update raises SlackAPIRatelimitError @@ -322,7 +322,7 @@ def test_update_alert_group_slack_message_other_exceptions( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") # SlackClient.chat_update raises the exception class @@ -364,7 +364,7 @@ def test_update_alert_group_slack_message_unexpected_exception( 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("task-id") # SlackClient.chat_update raises a generic exception diff --git a/engine/apps/slack/tests/test_slack_message.py b/engine/apps/slack/tests/test_slack_message.py index 5a766654f2..cc660466ba 100644 --- a/engine/apps/slack/tests/test_slack_message.py +++ b/engine/apps/slack/tests/test_slack_message.py @@ -25,7 +25,7 @@ def _slack_message_setup(cached_permalink): 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 make_slack_message(slack_channel, alert_group=alert_group, cached_permalink=cached_permalink) return _slack_message_setup @@ -106,7 +106,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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) with patch("apps.slack.client.SlackClient.conversations_members") as mock_members: mock_members.return_value = {"members": [slack_user_identity.slack_id]} @@ -132,7 +132,7 @@ 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=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) expected = ( f"https://slack.com/app_redirect?channel={slack_channel.slack_id}" @@ -154,9 +154,9 @@ def test_update_alert_groups_message_no_alert_group( """ Test that the method exits early if alert_group is None. """ - organization, slack_team_identity = make_organization_with_slack_team_identity() + _, slack_team_identity = make_organization_with_slack_team_identity() slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(channel=slack_channel, alert_group=None, organization=organization) + slack_message = make_slack_message(slack_channel) slack_message.update_alert_groups_message(debounce=True) @@ -184,7 +184,7 @@ def test_update_alert_groups_message_active_task_exists( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(channel=slack_channel, alert_group=alert_group) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id(task_id) slack_message.update_alert_groups_message(debounce=True) @@ -219,7 +219,7 @@ def test_update_alert_groups_message_last_updated_none( alert_group = make_alert_group(alert_receive_channel) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(channel=slack_channel, alert_group=alert_group, last_updated=None) + slack_message = make_slack_message(slack_channel, alert_group=alert_group, last_updated=None) assert slack_message.get_active_update_task_id() is None @@ -260,7 +260,7 @@ def test_update_alert_groups_message_schedules_task_correctly( slack_channel = make_slack_channel(slack_team_identity) slack_message = make_slack_message( - channel=slack_channel, + slack_channel, alert_group=alert_group, last_updated=timezone.now() - timedelta(seconds=10), ) @@ -305,7 +305,7 @@ def test_update_alert_groups_message_handles_minimum_countdown( slack_channel = make_slack_channel(slack_team_identity) slack_message = make_slack_message( - channel=slack_channel, + slack_channel, alert_group=alert_group, last_updated=timezone.now() - timedelta(seconds=SlackMessage.ALERT_GROUP_UPDATE_DEBOUNCE_INTERVAL_SECONDS + 1), @@ -353,7 +353,7 @@ def test_update_alert_groups_message_debounce_false_schedules_immediately( slack_channel = make_slack_channel(slack_team_identity) # Set up SlackMessage with existing task ID in the cache - slack_message = make_slack_message(channel=slack_channel, alert_group=alert_group) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) slack_message.set_active_update_task_id("existing-task-id") slack_message.update_alert_groups_message(debounce=False) diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index e64ea15720..859567cbfc 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -516,13 +516,10 @@ def _message_ts() -> str | None: return None try: - # TODO: once _channel_id has been fully migrated to channel, remove _channel_id - # see https://raintank-corp.slack.com/archives/C06K1MQ07GS/p1732555465144099 slack_message = SlackMessage.objects.get( slack_id=message_ts, - organization__slack_team_identity=slack_team_identity, - _channel_id=channel_id, - # channel__slack_id=channel_id, + _slack_team_identity=slack_team_identity, + 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 cdc552d884..82579b22f6 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -143,7 +143,7 @@ def test_organization_hard_delete( telegram_message = make_telegram_message(alert_group=alert_group, message_type=TelegramMessage.ALERT_GROUP_MESSAGE) slack_channel = make_slack_channel(slack_team_identity) - slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel) + slack_message = make_slack_message(slack_channel, alert_group=alert_group) 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 bcd7c47cc7..f749c92a88 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -527,14 +527,11 @@ def _make_slack_user_identity(**kwargs): @pytest.fixture def make_slack_message(): - 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 + def _make_slack_message(channel, alert_group=None, **kwargs): return SlackMessageFactory( - alert_group=alert_group, - organization=organization or alert_group.channel.organization, - _channel_id=channel.slack_id, + _slack_team_identity=channel.slack_team_identity, channel=channel, + alert_group=alert_group, **kwargs, ) diff --git a/engine/engine/management/commands/alertmanager_v2_migrate.py b/engine/engine/management/commands/alertmanager_v2_migrate.py deleted file mode 100644 index c5f10c5def..0000000000 --- a/engine/engine/management/commands/alertmanager_v2_migrate.py +++ /dev/null @@ -1,168 +0,0 @@ -from django.core.management.base import BaseCommand -from django.db import transaction -from django.db.models import Q -from django.utils import timezone - -from apps.alerts.models import AlertReceiveChannel - -ALERTMANAGER = "alertmanager" -LEGACY_ALERTMANAGER = "legacy_alertmanager" -GRAFANA_ALERTING = "grafana_alerting" -LEGACY_GRAFANA_ALERTING = "legacy_grafana_alerting" -TEMPLATE_FIELDS = [ - "web_title_template", - "web_message_template", - "web_image_url_template", - "sms_title_template", - "phone_call_title_template", - "source_link_template", - "grouping_id_template", - "resolve_condition_template", - "acknowledge_condition_template", - "slack_title_template", - "slack_message_template", - "slack_image_url_template", - "telegram_title_template", - "telegram_message_template", - "telegram_image_url_template", - "messaging_backends_templates", -] - - -class Command(BaseCommand): - def add_arguments(self, parser): - parser.add_argument("--backward", action="store_true", help="Run the migration backward.") - parser.add_argument( - "--org_id", type=int, help="Org id to perform the migration. " "If not present will migrate all." - ) - - def handle(self, *args, **options): - org_id = options.get("org_id", None) - if options["backward"]: - self.migrate_backward(org_id) - else: - self.migrate_forward(org_id) - - @transaction.atomic - def migrate_forward(self, org_id=None): - now = timezone.now() - self.stdout.write(f"Forward migration started at {now}.") - - self.stdout.write( - "Migrating legacy Alertmanager integrations " - "(updating fields 'integration' and 'alertmanager_v2_migrated_at')." - ) - alertmanager_to_update = AlertReceiveChannel.objects.filter(integration=LEGACY_ALERTMANAGER) - if org_id: - alertmanager_to_update = alertmanager_to_update.filter(organization_id=org_id) - num_updated = alertmanager_to_update.update(integration=ALERTMANAGER, alertmanager_v2_migrated_at=now) - self.stdout.write(f"Migrated {num_updated} legacy Alertmanager integrations.") - - self.stdout.write( - "Migrating legacy Grafana Alerting integrations " - "(updating fields 'integration' and 'alertmanager_v2_migrated_at')." - ) - alerting_to_update = AlertReceiveChannel.objects.filter(integration=LEGACY_GRAFANA_ALERTING) - if org_id: - alerting_to_update = alerting_to_update.filter(organization_id=org_id) - num_updated = alerting_to_update.update(integration=GRAFANA_ALERTING, alertmanager_v2_migrated_at=now) - - self.stdout.write(f"Migrated {num_updated} legacy Grafana Alerting integrations.") - - self.stdout.write("Fetching integrations to back up & reset templates.") - alert_receive_channels = AlertReceiveChannel.objects.filter( - Q( - **{f"{field}__isnull": False for field in TEMPLATE_FIELDS}, - _connector=Q.OR, - ), - integration__in=[ALERTMANAGER, GRAFANA_ALERTING], - alertmanager_v2_migrated_at__isnull=False, - ) - if org_id: - alert_receive_channels = alert_receive_channels.filter(organization_id=org_id) - - self.stdout.write(f"Backing up & resetting templates for {len(alert_receive_channels)} integrations.") - - for alert_receive_channel in alert_receive_channels: - self.stdout.write( - f"Backing up & resetting templates for integration {alert_receive_channel.public_primary_key}." - ) - alert_receive_channel.alertmanager_v2_backup_templates = { - field: getattr(alert_receive_channel, field) for field in TEMPLATE_FIELDS - } - for field in TEMPLATE_FIELDS: - setattr(alert_receive_channel, field, None) - - self.stdout.write(f"Bulk updating templates for {len(alert_receive_channels)} integrations.") - num_updated = AlertReceiveChannel.objects.bulk_update( - alert_receive_channels, - fields=[ - *TEMPLATE_FIELDS, - "alertmanager_v2_backup_templates", - ], - batch_size=1000, - ) - self.stdout.write(f"Bulk updated templates for {num_updated} integrations.") - - self.stdout.write("Forward migration finished.") - - @transaction.atomic - def migrate_backward(self, org_id=None): - now = timezone.now() - self.stdout.write(f"Backward migration started at {now}.") - - self.stdout.write( - "Backward migrating Alertmanager integrations " - "(updating fields 'integration' and 'alertmanager_v2_migrated_at')." - ) - - alertmanagers_to_restore = AlertReceiveChannel.objects.filter( - integration=ALERTMANAGER, alertmanager_v2_migrated_at__isnull=False - ) - if org_id: - alertmanagers_to_restore = alertmanagers_to_restore.filter(organization_id=org_id) - num_updated = alertmanagers_to_restore.update(integration=LEGACY_ALERTMANAGER, alertmanager_v2_migrated_at=None) - self.stdout.write(f"Backward migrated {num_updated} Alertmanager integrations.") - - self.stdout.write( - "Backward migrating Grafana Alerting integrations " - "(updating fields 'integration' and 'alertmanager_v2_migrated_at')." - ) - - alerting_to_restore = AlertReceiveChannel.objects.filter( - integration=GRAFANA_ALERTING, alertmanager_v2_migrated_at__isnull=False - ) - if org_id: - alerting_to_restore = alerting_to_restore.filter(organization_id=org_id) - num_updated = alerting_to_restore.update(integration=LEGACY_GRAFANA_ALERTING, alertmanager_v2_migrated_at=None) - self.stdout.write(f"Backward migrated {num_updated} Grafana Alerting integrations.") - - self.stdout.write("Fetching integrations to restore templates from backup.") - alert_receive_channels = AlertReceiveChannel.objects.filter( - integration__in=[LEGACY_ALERTMANAGER, LEGACY_GRAFANA_ALERTING], - alertmanager_v2_backup_templates__isnull=False, - ) - if org_id: - alert_receive_channels = alert_receive_channels.filter(organization_id=org_id) - self.stdout.write(f"Restoring templates for {len(alert_receive_channels)} integrations.") - - for alert_receive_channel in alert_receive_channels: - self.stdout.write(f"Restoring templates for integration {alert_receive_channel.public_primary_key}.") - if alert_receive_channel.alertmanager_v2_backup_templates is None: - continue - for field in TEMPLATE_FIELDS: - setattr(alert_receive_channel, field, alert_receive_channel.alertmanager_v2_backup_templates.get(field)) - alert_receive_channel.alertmanager_v2_backup_templates = None - - self.stdout.write(f"Bulk updating templates for {len(alert_receive_channels)} integrations.") - num_updated = AlertReceiveChannel.objects.bulk_update( - alert_receive_channels, - fields=[ - *TEMPLATE_FIELDS, - "alertmanager_v2_backup_templates", - ], - batch_size=1000, - ) - self.stdout.write(f"Bulk updated templates for {num_updated} integrations.") - - self.stdout.write("Backward migration finished.") diff --git a/engine/engine/management/commands/batch_migrate_slack_message_channel.py b/engine/engine/management/commands/batch_migrate_slack_message_channel.py deleted file mode 100644 index 2c302b9473..0000000000 --- a/engine/engine/management/commands/batch_migrate_slack_message_channel.py +++ /dev/null @@ -1,99 +0,0 @@ -import time - -from django.core.management.base import BaseCommand -from django.db import connection, transaction - -from apps.slack.models import SlackChannel, SlackMessage -from apps.user_management.models import Organization - - -class Command(BaseCommand): - help = "Batch updates SlackMessage.channel_id in chunks to avoid locking the table." - - def handle(self, *args, **options): - start_time = time.time() - self.stdout.write("Starting batch update of SlackMessage.channel_id...") - - # Step 1: Determine the queryset to update - # qs is ordered by id to ensure consistent batching - # since id is indexed, this ordering operation "should" be more efficient (as opposed to say created_at - # which we don't have an index on) - qs = SlackMessage.objects.filter( - _channel_id__isnull=False, # old column - organization__isnull=False, - channel_id__isnull=True, # new column - ).order_by("id") - - total_records = qs.count() - if total_records == 0: - self.stdout.write("No records to update.") - return - - self.stdout.write(f"Total records to update: {total_records}") - - # some considerations here.. - # - # Large IN clauses can be inefficient. Keep BATCH_SIZE reasonable (e.g., 1000) - # Fetching large batches of IDs consumes memory. With a BATCH_SIZE of 1000, this "should" be manageable - # - # references - # https://stackoverflow.com/a/5919165 - BATCH_SIZE = 1000 - - total_batches = (total_records + BATCH_SIZE - 1) // BATCH_SIZE - self.stdout.write(f"Batch size: {BATCH_SIZE}") - self.stdout.write(f"Total batches: {total_batches}") - - records_updated = 0 - batch_number = 1 - - # Process updates in batches - while True: - # Get the next batch of IDs - batch_qs = qs[:BATCH_SIZE] - - # collect the IDs to be updated - batch_ids = list(batch_qs.values_list("id", flat=True)) - - if not batch_ids: - break # No more records to process - - placeholders = ", ".join(["%s"] * len(batch_ids)) - update_query = f""" - UPDATE - {SlackMessage._meta.db_table} AS sm - INNER JOIN {Organization._meta.db_table} AS org - ON org.id = sm.organization_id - INNER JOIN {SlackChannel._meta.db_table} AS sc - ON sc.slack_id = sm._channel_id - AND sc.slack_team_identity_id = org.slack_team_identity_id - SET - sm.channel_id = sc.id - WHERE - sm.id IN ({placeholders}) - """ - params = batch_ids - - try: - # Execute the update - with transaction.atomic(): - with connection.cursor() as cursor: - cursor.execute(update_query, params) - batch_records_updated = cursor.rowcount - records_updated += batch_records_updated - - self.stdout.write(f"Batch {batch_number}/{total_batches}: Updated {batch_records_updated} records") - except Exception as e: - self.stderr.write(f"Error updating batch {batch_number}: {e}") - # Optionally, decide whether to continue or abort - continue - - # Remove processed records from queryset for next batch - qs = qs.exclude(id__in=batch_ids) - - batch_number += 1 - - end_time = time.time() - total_time = end_time - start_time - self.stdout.write(f"Batch update completed successfully. Total records updated: {records_updated}") - self.stdout.write(f"Total time taken: {total_time:.2f} seconds") diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index 08f42631a4..37861c433c 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -170,8 +170,6 @@ "apps.slack.tasks.send_message_to_thread_if_bot_not_in_channel": {"queue": "slack"}, "apps.slack.tasks.start_update_slack_user_group_for_schedules": {"queue": "slack"}, "apps.slack.tasks.unpopulate_slack_user_identities": {"queue": "slack"}, - # TODO: remove apps.slack.tasks.update_incident_slack_message after current tasks in queue have been processed - "apps.slack.tasks.update_incident_slack_message": {"queue": "slack"}, "apps.slack.tasks.update_alert_group_slack_message": {"queue": "slack"}, "apps.slack.tasks.update_slack_user_group_for_schedules": {"queue": "slack"}, "apps.slack.representatives.alert_group_representative.on_create_alert_slack_representative_async": {