refactor SlackMessage.channel_id (CHAR field) to SlackMessage.channel (foreign key relationship)

engine/apps/alerts/models/alert_group.py

@@ -1983,7 +1983,7 @@ def slack_channel_id(self) -> str | None:
         if not self.channel.organization.slack_team_identity:
             return None
         elif self.slack_message:
-            return self.slack_message.channel_id
+            return self.slack_message.channel_slack_id
         elif self.channel_filter:
             return self.channel_filter.slack_channel_id_or_org_default_id
         return None

engine/apps/alerts/tests/test_alert_group.py

@@ -14,7 +14,6 @@
 )
 from apps.slack.client import SlackClient
 from apps.slack.errors import SlackAPIMessageNotFoundError, SlackAPIRatelimitError
-from apps.slack.models import SlackMessage
 from apps.slack.tests.conftest import build_slack_response
 
 
@@ -24,14 +23,15 @@ def test_render_for_phone_call(
     make_alert_receive_channel,
     make_alert_group,
     make_alert,
+    make_slack_channel,
+    make_slack_message,
 ):
-    organization, _ = make_organization_with_slack_team_identity()
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
     alert_receive_channel = make_alert_receive_channel(organization)
 
     alert_group = make_alert_group(alert_receive_channel)
-    SlackMessage.objects.create(channel_id="CWER1ASD", alert_group=alert_group)
-
-    alert_group = make_alert_group(alert_receive_channel)
+    slack_channel = make_slack_channel(slack_team_identity)
+    make_slack_message(alert_group=alert_group, channel=slack_channel)
 
     make_alert(
         alert_group,
@@ -105,7 +105,7 @@ def test_delete(
     make_alert(alert_group, raw_request_data={})
 
     # Create Slack messages
-    slack_message = make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+    slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel1)
     resolution_note_1 = make_resolution_note_slack_message(
         alert_group=alert_group,
         user=user,
@@ -154,7 +154,7 @@ def test_delete(
     assert mock_chat_delete.call_args_list[0] == call(
         channel=resolution_note_1.slack_channel_id, ts=resolution_note_1.ts
     )
-    assert mock_chat_delete.call_args_list[1] == call(channel=slack_message.channel_id, ts=slack_message.slack_id)
+    assert mock_chat_delete.call_args_list[1] == call(channel=slack_message.channel_slack_id, ts=slack_message.slack_id)
     mock_reactions_remove.assert_called_once_with(
         channel=resolution_note_2.slack_channel_id, name="memo", timestamp=resolution_note_2.ts
     )
@@ -188,7 +188,7 @@ def test_delete_slack_ratelimit(
     make_alert(alert_group, raw_request_data={})
 
     # Create Slack messages
-    make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+    make_slack_message(alert_group=alert_group, channel=slack_channel1)
     make_resolution_note_slack_message(
         alert_group=alert_group,
         user=user,
@@ -259,7 +259,7 @@ def test_delete_slack_api_error_other_than_ratelimit(
     make_alert(alert_group, raw_request_data={})
 
     # Create Slack messages
-    make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+    make_slack_message(alert_group=alert_group, channel=slack_channel1)
     make_resolution_note_slack_message(
         alert_group=alert_group,
         user=user,

engine/apps/alerts/tests/test_notify_user.py

@@ -232,39 +232,44 @@ def test_notify_user_perform_notification_skip_if_resolved(
 def test_perform_notification_reason_to_skip_escalation_in_slack(
     reason_to_skip_escalation,
     error_code,
-    make_organization,
-    make_slack_team_identity,
+    make_organization_with_slack_team_identity,
     make_user,
     make_user_notification_policy,
     make_alert_receive_channel,
     make_alert_group,
     make_user_notification_policy_log_record,
+    make_slack_channel,
     make_slack_message,
 ):
-    organization = make_organization()
-    slack_team_identity = make_slack_team_identity()
-    organization.slack_team_identity = slack_team_identity
-    organization.save()
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
+
     user = make_user(organization=organization)
     user_notification_policy = make_user_notification_policy(
         user=user,
         step=UserNotificationPolicy.Step.NOTIFY,
         notify_by=UserNotificationPolicy.NotificationChannel.SLACK,
     )
     alert_receive_channel = make_alert_receive_channel(organization=organization)
-    alert_group = make_alert_group(alert_receive_channel=alert_receive_channel)
-    alert_group.reason_to_skip_escalation = reason_to_skip_escalation
-    alert_group.save()
+
+    alert_group = make_alert_group(
+        alert_receive_channel=alert_receive_channel,
+        reason_to_skip_escalation=reason_to_skip_escalation,
+    )
+
     log_record = make_user_notification_policy_log_record(
         author=user,
         alert_group=alert_group,
         notification_policy=user_notification_policy,
         type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED,
     )
+
     if not error_code:
-        make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+        slack_channel = make_slack_channel(slack_team_identity=slack_team_identity)
+        make_slack_message(alert_group=alert_group, channel=slack_channel)
+
     with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification:
         perform_notification(log_record.pk, False)
+
     last_log_record = UserNotificationPolicyLogRecord.objects.last()
 
     if error_code:
@@ -280,25 +285,24 @@ def test_perform_notification_reason_to_skip_escalation_in_slack(
 
 @pytest.mark.django_db
 def test_perform_notification_slack_prevent_posting(
-    make_organization,
-    make_slack_team_identity,
+    make_organization_with_slack_team_identity,
     make_user,
     make_user_notification_policy,
     make_alert_receive_channel,
     make_alert_group,
     make_user_notification_policy_log_record,
+    make_slack_channel,
     make_slack_message,
 ):
-    organization = make_organization()
-    slack_team_identity = make_slack_team_identity()
-    organization.slack_team_identity = slack_team_identity
-    organization.save()
+    organization, slack_team_identity = make_organization_with_slack_team_identity()
+
     user = make_user(organization=organization)
     user_notification_policy = make_user_notification_policy(
         user=user,
         step=UserNotificationPolicy.Step.NOTIFY,
         notify_by=UserNotificationPolicy.NotificationChannel.SLACK,
     )
+
     alert_receive_channel = make_alert_receive_channel(organization=organization)
     alert_group = make_alert_group(alert_receive_channel=alert_receive_channel)
     log_record = make_user_notification_policy_log_record(
@@ -308,7 +312,9 @@ def test_perform_notification_slack_prevent_posting(
         type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED,
         slack_prevent_posting=True,
     )
-    make_slack_message(alert_group=alert_group, channel_id="test_channel_id", slack_id="test_slack_id")
+
+    slack_channel = make_slack_channel(slack_team_identity=slack_team_identity)
+    make_slack_message(alert_group=alert_group, channel=slack_channel)
 
     with patch.object(SlackMessage, "send_slack_notification") as mocked_send_slack_notification:
         perform_notification(log_record.pk, False)

engine/apps/public_api/tests/test_alert_groups.py

@@ -162,9 +162,7 @@ def test_get_alert_group_slack_links(
     organization.slack_team_identity = slack_team_identity
     organization.save()
     slack_channel = make_slack_channel(slack_team_identity)
-    slack_message = make_slack_message(
-        alert_group=alert_group, channel_id=slack_channel.slack_id, cached_permalink="the-link"
-    )
+    slack_message = make_slack_message(alert_group=alert_group, channel=slack_channel, cached_permalink="the-link")
 
     url = reverse("api-public:alert_groups-detail", kwargs={"pk": expected_response["id"]})
     response = client.get(url, format="json", HTTP_AUTHORIZATION=token)

engine/apps/schedules/models/shift_swap_request.py

@@ -17,7 +17,7 @@
 if typing.TYPE_CHECKING:
     from apps.schedules.models import OnCallSchedule
     from apps.schedules.models.on_call_schedule import ScheduleEvents
-    from apps.slack.models import SlackMessage
+    from apps.slack.models import SlackChannel, SlackMessage
     from apps.user_management.models import Organization, User
 
 
@@ -164,7 +164,15 @@ def status(self) -> str:
         return self.Statuses.OPEN
 
     @property
-    def slack_channel_id(self) -> str | None:
+    def slack_channel(self) -> typing.Optional["SlackChannel"]:
+        """
+        This is only set if the schedule associated with the shift swap request
+        has a Slack channel configured for it.
+        """
+        return self.schedule.slack_channel
+
+    @property
+    def slack_channel_id(self) -> typing.Optional[str]:
         """
         This is only set if the schedule associated with the shift swap request
         has a Slack channel configured for it.

engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py

@@ -29,7 +29,7 @@ def _shift_swap_request_test_setup(swap_start=None, swap_end=None, **kwargs):
         user = make_user(organization=organization)
 
         slack_channel = make_slack_channel(slack_team_identity)
-        slack_message = make_slack_message(alert_group=None, organization=organization, slack_id="12345")
+        slack_message = make_slack_message(alert_group=None, channel=slack_channel, organization=organization, slack_id="12345")
 
         schedule = make_schedule(organization, schedule_class=OnCallScheduleWeb, slack_channel=slack_channel)
 
@@ -161,7 +161,7 @@ def test_send_shift_swap_request_followup(mock_slack_chat_post_message, shift_sw
     send_shift_swap_request_slack_followup(shift_swap_request.pk)
 
     mock_slack_chat_post_message.assert_called_once_with(
-        channel=shift_swap_request.slack_message.channel_id,
+        channel=shift_swap_request.slack_message.channel_slack_id,
         thread_ts=shift_swap_request.slack_message.slack_id,
         reply_broadcast=True,
         blocks=ANY,

engine/apps/slack/alert_group_slack_service.py

@@ -39,7 +39,7 @@ def update_alert_group_slack_message(self, alert_group: "AlertGroup") -> None:
 
         try:
             self._slack_client.chat_update(
-                channel=alert_group.slack_message.channel_id,
+                channel=alert_group.slack_message.channel_slack_id,
                 ts=alert_group.slack_message.slack_id,
                 attachments=alert_group.render_slack_attachments(),
                 blocks=alert_group.render_slack_blocks(),
@@ -71,15 +71,17 @@ def publish_message_to_alert_group_thread(
         if alert_group.channel.is_rate_limited_in_slack:
             return
 
+        slack_message = alert_group.slack_message
+
         if attachments is None:
             attachments = []
 
         try:
             result = self._slack_client.chat_postMessage(
-                channel=alert_group.slack_message.channel_id,
+                channel=slack_message.channel_slack_id,
                 text=text,
                 attachments=attachments,
-                thread_ts=alert_group.slack_message.slack_id,
+                thread_ts=slack_message.slack_id,
                 mrkdwn=mrkdwn,
                 unfurl_links=unfurl_links,
             )
@@ -91,9 +93,4 @@ def publish_message_to_alert_group_thread(
         ):
             return
 
-        alert_group.slack_messages.create(
-            slack_id=result["ts"],
-            organization=alert_group.channel.organization,
-            _slack_team_identity=self.slack_team_identity,
-            channel_id=alert_group.slack_message.channel_id,
-        )
+        alert_group.slack_messages.create(slack_id=result["ts"], channel=slack_message.channel)

engine/apps/slack/migrations/0006_rename_channel_id_slackmessage__channel_id_and_more.py

@@ -0,0 +1,26 @@
+# Generated by Django 4.2.16 on 2024-11-22 12:36
+
+from django.db import migrations, models
+import django.db.models.deletion
+import django_migration_linter as linter
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('slack', '0005_slackteamidentity__unified_slack_app_installed'),
+    ]
+
+    operations = [
+        linter.IgnoreMigration(),
+        migrations.RenameField(
+            model_name='slackmessage',
+            old_name='channel_id',
+            new_name='_channel_id',
+        ),
+        migrations.AddField(
+            model_name='slackmessage',
+            name='channel',
+            field=models.ForeignKey(default=None, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='slack_messages', to='slack.slackchannel'),
+        ),
+    ]

engine/apps/slack/migrations/0007_migrate_slackmessage_channel_id.py

@@ -0,0 +1,47 @@
+# Generated by Django 4.2.16 on 2024-11-01 10:58
+import logging
+
+from django.db import migrations
+import django_migration_linter as linter
+
+logger = logging.getLogger(__name__)
+
+def populate_slack_channel(apps, schema_editor):
+    SlackMessage = apps.get_model("slack", "SlackMessage")
+    SlackChannel = apps.get_model("slack", "SlackChannel")
+
+    logger.info("Starting migration to populate channel field.")
+
+    sql = f"""
+    UPDATE
+        {SlackMessage._meta.db_table} AS sm
+    JOIN {SlackChannel._meta.db_table} AS sc
+        ON sc.slack_id = sm._channel_id
+        AND sc.slack_team_identity_id = sm.slack_team_identity
+    SET
+        sm.channel_id = sc.id
+    WHERE
+        sm._channel_id IS NOT NULL
+        AND sm.slack_team_identity IS NOT NULL;
+    """
+
+    with schema_editor.connection.cursor() as cursor:
+        cursor.execute(sql)
+        updated_rows = cursor.rowcount  # Number of rows updated
+
+    logger.info(f"Bulk updated {updated_rows} SlackMessages with their Slack channel.")
+    logger.info("Finished migration to populate channel field.")
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('slack', '0006_rename_channel_id_slackmessage__channel_id_and_more'),
+    ]
+
+    operations = [
+        # simply setting this new field is okay, we are not deleting the value of _channel_id
+        # therefore, no need to revert it
+        linter.IgnoreMigration(),
+        migrations.RunPython(populate_slack_channel, migrations.RunPython.noop),
+    ]

engine/apps/slack/models/slack_channel.py

@@ -1,9 +1,16 @@
+import typing
+
 from django.conf import settings
 from django.core.validators import MinLengthValidator
 from django.db import models
 
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 
+if typing.TYPE_CHECKING:
+    from django.db.models.manager import RelatedManager
+
+    from apps.slack.models import SlackMessage, SlackTeamIdentity
+
 
 def generate_public_primary_key_for_slack_channel():
     prefix = "H"
@@ -20,6 +27,9 @@ def generate_public_primary_key_for_slack_channel():
 
 
 class SlackChannel(models.Model):
+    slack_team_identity: "SlackTeamIdentity"
+    slack_messages: "RelatedManager['SlackMessage']"
+
     public_primary_key = models.CharField(
         max_length=20,
         validators=[MinLengthValidator(settings.PUBLIC_PRIMARY_KEY_MIN_LENGTH + 1)],