wip

engine/apps/api/serializers/alert_group.py

@@ -138,7 +138,7 @@ class AlertGroupListSerializer(
         "labels",
         Prefetch(
             "slack_messages",
-            queryset=SlackMessage.objects.select_related("slack_team_identity").order_by("created_at")[:1],
+            queryset=SlackMessage.objects.select_related("_slack_team_identity").order_by("created_at")[:1],
             to_attr="prefetched_slack_messages",
         ),
         Prefetch(

engine/apps/public_api/serializers/alert_groups.py

@@ -36,7 +36,7 @@ class AlertGroupSerializer(EagerLoadingMixin, serializers.ModelSerializer):
         "labels",
         Prefetch(
             "slack_messages",
-            queryset=SlackMessage.objects.select_related("slack_team_identity").order_by("created_at")[:1],
+            queryset=SlackMessage.objects.select_related("_slack_team_identity").order_by("created_at")[:1],
             to_attr="prefetched_slack_messages",
         ),
         Prefetch(

engine/apps/slack/alert_group_slack_service.py

@@ -65,6 +65,6 @@ def publish_message_to_alert_group_thread(
 
         alert_group.slack_messages.create(
             slack_id=result["ts"],
-            slack_team_identity=self.slack_team_identity,
+            _slack_team_identity=self.slack_team_identity,
             channel=slack_message.channel,
         )

engine/apps/slack/migrations/0009_drop_orphaned_messages_and_fill_in_missing_team_identity_values.py

@@ -0,0 +1,132 @@
+# 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")
+
+    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.")
+
+    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),
+    ]

engine/apps/slack/models/slack_message.py

@@ -23,7 +23,7 @@
 if typing.TYPE_CHECKING:
     from apps.alerts.models import AlertGroup
     from apps.base.models import UserNotificationPolicy
-    from apps.slack.models import SlackChannel
+    from apps.slack.models import SlackChannel, SlackTeamIdentity
     from apps.user_management.models import User
 
 logger = logging.getLogger(__name__)
@@ -45,19 +45,11 @@ class SlackMessage(models.Model):
     """
 
     channel = models.ForeignKey(
-        "slack.SlackChannel", on_delete=models.CASCADE, null=False, default=None, related_name="slack_messages"
+        "slack.SlackChannel", on_delete=models.CASCADE, null=True, default=None, related_name="slack_messages"
     )
     """
-    TODO: remove default=None in a subsequent PR/release. Can't (easily) be done in the same PR as settings null=False
-    because manage.py makemigrations warns us with the following:
-
-    It is impossible to change a nullable field 'channel' on slackmessage to non-nullable without providing a default.
-    This is because the database needs something to populate existing rows.
-    Please select a fix:
-    1) Provide a one-off default now (will be set on all existing rows with a null value for this column)
-    2) Ignore for now. Existing rows that contain NULL values will have to be handled manually,
-    for example with a RunPython or RunSQL operation.
-    3) Quit and manually define a default value in models.py.
+    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(
@@ -74,14 +66,23 @@ class SlackMessage(models.Model):
     For shift swap request related slack messages, the schedule has a reference to the organization
     """
 
-    slack_team_identity = models.ForeignKey(
+    _slack_team_identity = models.ForeignKey(
         "slack.SlackTeamIdentity",
         on_delete=models.PROTECT,
         null=True,
         default=None,
         related_name="slack_message",
         db_column="slack_team_identity",
     )
+    """
+    TODO: rename this from _slack_team_identity to slack_team_identity in a subsequent PR/release
+
+    This involves also updating the Meta.constraints to use the new field name, this may involve
+    migrations.RemoveConstraint and migrations.AddConstraint operations, which we need to investigate further...
+
+    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)
 
@@ -101,9 +102,16 @@ class SlackMessage(models.Model):
     class Meta:
         # slack_id is unique within the context of a channel or conversation
         constraints = [
-            models.UniqueConstraint(fields=["slack_id", "channel_id", "slack_team_identity"], name="unique slack_id")
+            models.UniqueConstraint(fields=["slack_id", "channel_id", "_slack_team_identity"], name="unique slack_id")
         ]
 
+    @property
+    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]:
         # Don't send request for permalink if slack token has been revoked
@@ -228,7 +236,7 @@ def send_slack_notification(
         else:
             alert_group.slack_messages.create(
                 slack_id=result["ts"],
-                slack_team_identity=slack_team_identity,
+                _slack_team_identity=slack_team_identity,
                 channel=slack_channel,
             )
 

engine/apps/slack/scenarios/distribute_alerts.py

@@ -160,7 +160,7 @@ def process_signal(self, alert: Alert) -> None:
 
                 alert_group.slack_messages.create(
                     slack_id=result["ts"],
-                    slack_team_identity=slack_team_identity,
+                    _slack_team_identity=slack_team_identity,
                     channel=slack_channel,
                 )
 
@@ -914,7 +914,7 @@ def process_signal(self, log_record: AlertGroupLogRecord) -> None:
             else:
                 alert_group.slack_messages.create(
                     slack_id=response["ts"],
-                    slack_team_identity=slack_message.slack_team_identity,
+                    _slack_team_identity=slack_message.slack_team_identity,
                     channel=slack_channel,
                 )
 

engine/apps/slack/scenarios/resolution_note.py

@@ -94,7 +94,7 @@ def process_scenario(
         try:
             slack_message = SlackMessage.objects.get(
                 slack_id=payload["message"]["thread_ts"],
-                slack_team_identity=slack_team_identity,
+                _slack_team_identity=slack_team_identity,
                 channel__slack_id=channel_id,
             )
         except SlackMessage.DoesNotExist:
@@ -163,7 +163,7 @@ def process_scenario(
                     )
                     slack_message = SlackMessage.objects.get(
                         slack_id=thread_ts,
-                        slack_team_identity=slack_team_identity,
+                        _slack_team_identity=slack_team_identity,
                         channel__slack_id=channel_id,
                     )
                     alert_group = slack_message.alert_group

engine/apps/slack/scenarios/shift_swap_requests.py

@@ -163,7 +163,7 @@ def create_message(self, shift_swap_request: "ShiftSwapRequest") -> SlackMessage
 
         return SlackMessage.objects.create(
             slack_id=result["ts"],
-            slack_team_identity=self.slack_team_identity,
+            _slack_team_identity=self.slack_team_identity,
             channel=shift_swap_request.slack_channel,
         )