centralcloud/oncall-engine
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

chore: drop usage of SlackMessage.organization + drop orphaned SlackMessages (#5330)

Browse source 
# 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.
This commit is contained in:
Joey Orlando 2024-12-06 11:43:40 -05:00 • committed by GitHub
parent 3977c6e9ef
commit e115617528
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
27 changed files with 231 additions and 125 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
engine/apps/alerts/tests/test_alert_group.py
View file

@ -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,

4
engine/apps/alerts/tests/test_notify_user.py
View file

@ -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)

2
engine/apps/public_api/tests/test_alert_groups.py
View file

@ -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)

4
engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py
View file

@ -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)

4
engine/apps/schedules/tests/tasks/shift_swaps/test_slack_messages.py
View file

@ -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

2
engine/apps/slack/alert_group_slack_service.py
View file

@ -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,
)

4
engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py
View file

@ -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

137
engine/apps/slack/migrations/0009_drop_orphaned_messages_and_fill_in_missing_team_identity_values.py Normal file
View file

@ -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),
]

36
engine/apps/slack/models/slack_message.py
View file

@ -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,
)

4
engine/apps/slack/scenarios/distribute_alerts.py
View file

@ -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,
)

4
engine/apps/slack/scenarios/resolution_note.py
View file

@ -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

2
engine/apps/slack/scenarios/shift_swap_requests.py
View file

@ -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,
)

2
engine/apps/slack/scenarios/slack_channel_integration.py
View file

@ -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:

4
engine/apps/slack/scenarios/step_mixins.py
View file

@ -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

21
engine/apps/slack/tasks.py
View file

@ -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,

28
engine/apps/slack/tests/scenario_steps/test_alert_group_actions.py
View file

@ -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)

2
engine/apps/slack/tests/scenario_steps/test_manage_responders.py
View file

@ -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

10
engine/apps/slack/tests/scenario_steps/test_resolution_note.py
View file

@ -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)

9
engine/apps/slack/tests/scenario_steps/test_shift_swap_requests.py
View file

@ -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)

2
engine/apps/slack/tests/scenario_steps/test_slack_channel.py
View file

@ -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

15
engine/apps/slack/tests/scenario_steps/test_slack_channel_integration.py
View file

@ -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": {

20
engine/apps/slack/tests/tasks/test_update_alert_group_slack_message.py
View file

@ -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

20
engine/apps/slack/tests/test_slack_message.py
View file

@ -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)

2
engine/apps/slack/views.py
View file

@ -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:

2
engine/apps/user_management/tests/test_organization.py
View file

@ -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)

6
engine/conftest.py
View file

@ -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,
)

2
engine/settings/celery_task_routes.py
View file

@ -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": {