From e115617528bfaed1fa3be8771e36c9d94403d097 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 6 Dec 2024 11:43:40 -0500 Subject: [PATCH] chore: drop usage of `SlackMessage.organization` + drop orphaned `SlackMessage`s (#5330) # What this PR does - Stops writing `SlackMessage.organization` + removes references to this field. [As we discussed](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733315887463279?thread_ts=1733311105.095309&cid=C083TU81TCH), we do not need this field on this model/table, `SlackMessage._slack_team_identity` is sufficient (`organization` will be dropped in a separate PR) - Adds a data migration script which: - drops orphaned `SlackMessage` records; ie. ones which, even after the [`engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py`](https://github.com/grafana/oncall/blob/dev/engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py) migration, still don't have a `SlackMessage.channel` id filled in (we discussed + agreed on dropping these records [here](https://raintank-corp.slack.com/archives/C083TU81TCH/p1733329914516859?thread_ts=1733311105.095309&cid=C083TU81TCH)) - fills in empty `SlackMessage.slack_team_identity` values (from `slack_message.channel.slack_team_identity`) ### Other notes On the `organization` topic. We store records in `SlackMessage` for two purposes (AFAICT), and in both cases, we have references back to the `organization`: - alert groups - `slack_message.alert_group.channel.organization` - shift swap requests - `shift_swap_request.schedule.organization` ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/alerts/tests/test_alert_group.py | 8 +- engine/apps/alerts/tests/test_notify_user.py | 4 +- .../public_api/tests/test_alert_groups.py | 2 +- .../tasks/shift_swaps/test_slack_followups.py | 4 +- .../tasks/shift_swaps/test_slack_messages.py | 4 +- .../apps/slack/alert_group_slack_service.py | 2 +- .../0007_migrate_slackmessage_channel_id.py | 4 - ...nd_fill_in_missing_team_identity_values.py | 137 ++++++++++++++++++ engine/apps/slack/models/slack_message.py | 36 +++-- .../apps/slack/scenarios/distribute_alerts.py | 4 +- .../apps/slack/scenarios/resolution_note.py | 4 +- .../slack/scenarios/shift_swap_requests.py | 2 +- .../scenarios/slack_channel_integration.py | 2 +- engine/apps/slack/scenarios/step_mixins.py | 4 +- engine/apps/slack/tasks.py | 21 --- .../test_alert_group_actions.py | 28 ++-- .../scenario_steps/test_manage_responders.py | 2 +- .../scenario_steps/test_resolution_note.py | 10 +- .../test_shift_swap_requests.py | 9 +- .../scenario_steps/test_slack_channel.py | 2 +- .../test_slack_channel_integration.py | 15 +- .../test_update_alert_group_slack_message.py | 20 +-- engine/apps/slack/tests/test_slack_message.py | 20 +-- engine/apps/slack/views.py | 2 +- .../tests/test_organization.py | 2 +- engine/conftest.py | 6 +- engine/settings/celery_task_routes.py | 2 - 27 files changed, 231 insertions(+), 125 deletions(-) create mode 100644 engine/apps/slack/migrations/0009_drop_orphaned_messages_and_fill_in_missing_team_identity_values.py diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index 4e21c3e2..a29ae9ca 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, diff --git a/engine/apps/alerts/tests/test_notify_user.py b/engine/apps/alerts/tests/test_notify_user.py index 4a4b92a3..e97a577b 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 2b79f478..247246dd 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 d0be1dc0..dddac77e 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( 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 da57d291..7e625a46 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/alert_group_slack_service.py b/engine/apps/slack/alert_group_slack_service.py index dc46873a..512b50f4 100644 --- a/engine/apps/slack/alert_group_slack_service.py +++ b/engine/apps/slack/alert_group_slack_service.py @@ -65,6 +65,6 @@ class AlertGroupSlackService: alert_group.slack_messages.create( slack_id=result["ts"], - organization=alert_group.channel.organization, + _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 35b7ed81..63206d62 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/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 00000000..cff04a88 --- /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 de9eb539..16b032e2 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -23,7 +23,7 @@ from apps.slack.tasks import update_alert_group_slack_message 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 - Instead of using this column, we can simply do self.organization.slack_team_identity + 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... + + 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) @@ -96,8 +106,11 @@ class SlackMessage(models.Model): ] @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]: @@ -122,9 +135,8 @@ class SlackMessage(models.Model): def deep_link(self) -> str: 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 @@ -138,7 +150,7 @@ class SlackMessage(models.Model): 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) @@ -175,7 +187,7 @@ class SlackMessage(models.Model): } ] - sc = SlackClient(organization.slack_team_identity, enable_ratelimit_retry=True) + sc = SlackClient(slack_team_identity, enable_ratelimit_retry=True) try: result = sc.chat_postMessage( @@ -224,7 +236,7 @@ class SlackMessage(models.Model): else: alert_group.slack_messages.create( slack_id=result["ts"], - organization=organization, + _slack_team_identity=slack_team_identity, channel=slack_channel, ) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index b22cc2a3..66a9901d 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -160,7 +160,7 @@ class IncomingAlertStep(scenario_step.ScenarioStep): alert_group.slack_messages.create( slack_id=result["ts"], - organization=alert_group.channel.organization, + _slack_team_identity=slack_team_identity, channel=slack_channel, ) @@ -914,7 +914,7 @@ class AcknowledgeConfirmationStep(AcknowledgeGroupStep): else: alert_group.slack_messages.create( slack_id=response["ts"], - organization=organization, + _slack_team_identity=slack_message.slack_team_identity, channel=slack_channel, ) diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index 63174b08..f1a557be 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -94,7 +94,7 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): try: slack_message = SlackMessage.objects.get( slack_id=payload["message"]["thread_ts"], - organization__slack_team_identity=slack_team_identity, + _slack_team_identity=slack_team_identity, channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: @@ -163,7 +163,7 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): ) slack_message = SlackMessage.objects.get( slack_id=thread_ts, - organization__slack_team_identity=slack_team_identity, + _slack_team_identity=slack_team_identity, channel__slack_id=channel_id, ) alert_group = slack_message.alert_group diff --git a/engine/apps/slack/scenarios/shift_swap_requests.py b/engine/apps/slack/scenarios/shift_swap_requests.py index 79085c7a..5dd21422 100644 --- a/engine/apps/slack/scenarios/shift_swap_requests.py +++ b/engine/apps/slack/scenarios/shift_swap_requests.py @@ -163,7 +163,7 @@ class BaseShiftSwapRequestStep(scenario_step.ScenarioStep): return SlackMessage.objects.create( slack_id=result["ts"], - organization=self.organization, + _slack_team_identity=self.slack_team_identity, channel=shift_swap_request.slack_channel, ) diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index 7e04e024..158c289c 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -68,7 +68,7 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): try: slack_message = SlackMessage.objects.get( slack_id=thread_ts, - organization__slack_team_identity=self.slack_team_identity, + _slack_team_identity=self.slack_team_identity, channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: diff --git a/engine/apps/slack/scenarios/step_mixins.py b/engine/apps/slack/scenarios/step_mixins.py index c8a367b7..12576ad6 100644 --- a/engine/apps/slack/scenarios/step_mixins.py +++ b/engine/apps/slack/scenarios/step_mixins.py @@ -65,7 +65,7 @@ class AlertGroupActionsMixin: SlackMessage.objects.create( slack_id=message_id, - organization=alert_group.channel.organization, + _slack_team_identity=slack_team_identity, channel=slack_channel, alert_group=alert_group, ) @@ -178,7 +178,7 @@ class AlertGroupActionsMixin: # Get SlackMessage from DB slack_message = SlackMessage.objects.get( slack_id=message_ts, - organization__slack_team_identity=slack_team_identity, + _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 72908179..385ff601 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -124,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 68984289..7320492f 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 922a6664..ed6d33b9 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 28124202..e1523f66 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 906b856f..2e78341c 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 @@ class TestBaseShiftSwapRequestStep: ) 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 @@ class TestBaseShiftSwapRequestStep: 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 @@ class TestBaseShiftSwapRequestStep: 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 4ca4b27a..a79a77b3 100644 --- a/engine/apps/slack/tests/scenario_steps/test_slack_channel.py +++ b/engine/apps/slack/tests/scenario_steps/test_slack_channel.py @@ -93,7 +93,7 @@ class TestSlackChannelDeletedEventStep: slack_user_identity, ) = make_organization_and_user_with_slack_identities() slack_channel = make_slack_channel(slack_team_identity) - make_slack_message(slack_channel, organization=organization) + make_slack_message(slack_channel) slack_channel_id = slack_channel.slack_id # Ensure the SlackChannel exists 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 a081cb0e..3e2f4f6a 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 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: 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 @@ class TestSlackChannelMessageEventStep: ) = 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 1cd42345..bd5b59a7 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 @@ class TestUpdateAlertGroupSlackMessageTask: 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 @@ class TestUpdateAlertGroupSlackMessageTask: """ 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 @@ class TestUpdateAlertGroupSlackMessageTask: 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 @@ class TestUpdateAlertGroupSlackMessageTask: 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,7 +183,7 @@ class TestUpdateAlertGroupSlackMessageTask: 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") @@ -230,7 +230,7 @@ class TestUpdateAlertGroupSlackMessageTask: 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 @@ class TestUpdateAlertGroupSlackMessageTask: 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 @@ class TestUpdateAlertGroupSlackMessageTask: 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 @@ class TestUpdateAlertGroupSlackMessageTask: 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 5a766654..cc660466 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( 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 @@ class TestSlackMessageUpdateAlertGroupsMessage: """ 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 @@ class TestSlackMessageUpdateAlertGroupsMessage: 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 @@ class TestSlackMessageUpdateAlertGroupsMessage: 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 @@ class TestSlackMessageUpdateAlertGroupsMessage: 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 @@ class TestSlackMessageUpdateAlertGroupsMessage: 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 @@ class TestSlackMessageUpdateAlertGroupsMessage: 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 681640c7..859567cb 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -518,7 +518,7 @@ class SlackEventApiEndpointView(APIView): try: slack_message = SlackMessage.objects.get( slack_id=message_ts, - organization__slack_team_identity=slack_team_identity, + _slack_team_identity=slack_team_identity, channel__slack_id=channel_id, ) except SlackMessage.DoesNotExist: diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index cdc552d8..82579b22 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 5fcd1da6..f749c92a 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -527,11 +527,11 @@ def make_slack_user_identity(): @pytest.fixture def make_slack_message(): - def _make_slack_message(channel, alert_group=None, organization=None, **kwargs): + def _make_slack_message(channel, alert_group=None, **kwargs): return SlackMessageFactory( - alert_group=alert_group, - organization=organization or alert_group.channel.organization, + _slack_team_identity=channel.slack_team_identity, channel=channel, + alert_group=alert_group, **kwargs, ) diff --git a/engine/settings/celery_task_routes.py b/engine/settings/celery_task_routes.py index 08f42631..37861c43 100644 --- a/engine/settings/celery_task_routes.py +++ b/engine/settings/celery_task_routes.py @@ -170,8 +170,6 @@ CELERY_TASK_ROUTES = { "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": {