feat: convert organization.general_log_channel_id to `organization.…

…default_slack_channel` (#5191)

# What this PR does

Related to grafana/oncall-private#2947

Right now `general_log_channel_id` is just a string value representing
the Slack Channel ID (ex. `C043HQ70QMB`). This PR migrates this instead
to be a foreign key relationship on the `slack_slackchannel` table and
updates all references to `general_log_channel_id`.

Tested migrations locally:
```bash
Operations to perform:
  Apply all migrations: [redacted secret grafana-admin-creds:admin-user], alerts, auth, auth_token, base, contenttypes, email, exotel, fcm_django, google, heartbeat, labels, mobile_app, oss_installation, phone_notifications, schedules, sessions, slack, social_django, telegram, twilioapp, user_management, webhooks, zvonok
Running migrations:
  Applying user_management.0024_organization_general_log_slack_channel... OK
source=engine:app google_trace_id=none logger=apps.user_management.migrations.0025_auto_20241017_1919 Starting migration to populate general_log_slack_channel field.
source=engine:app google_trace_id=none logger=apps.user_management.migrations.0025_auto_20241017_1919 Total organizations to process: 1
source=engine:app google_trace_id=none logger=apps.user_management.migrations.0025_auto_20241017_1919 Organization 1 updated with SlackChannel 2 (slack_id: C043LL6RTS7).
source=engine:app google_trace_id=none logger=apps.user_management.migrations.0025_auto_20241017_1919 Finished migration. Total organizations processed: 1. Organizations updated: 1. Missing SlackChannels: 0.
  Applying user_management.0025_auto_20241017_1919... OK
```

## Future incoming PRs

- Drop `Organization.general_log_channel_id` column
- Migrate `ChannelFilter.slack_channel_id` and
`ResolutionNoteSlackMessage.slack_channel_id` to use foreign key
relationships

## 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/models/alert_receive_channel.py

@@ -617,7 +617,7 @@ def get_verbal(self):
     def force_disable_maintenance(self, user):
         disable_maintenance(alert_receive_channel_id=self.pk, force=True, user_id=user.pk)
 
-    def notify_about_maintenance_action(self, text, send_to_general_log_channel=True):
+    def notify_about_maintenance_action(self, text: str, send_to_general_log_channel=True) -> None:
         # TODO: this method should be refactored.
         # It's binded to slack and sending maintenance notification only there.
         channel_ids = list(
@@ -627,7 +627,7 @@ def notify_about_maintenance_action(self, text, send_to_general_log_channel=True
         )
 
         if send_to_general_log_channel:
-            general_log_channel_id = self.organization.general_log_channel_id
+            general_log_channel_id = self.organization.default_slack_channel_slack_id
             if general_log_channel_id is not None:
                 channel_ids.append(general_log_channel_id)
         unique_channels_id = set(channel_ids)

engine/apps/alerts/models/channel_filter.py

@@ -45,6 +45,7 @@ class ChannelFilter(OrderedModel):
     """
 
     alert_groups: "RelatedManager['AlertGroup']"
+    alert_receive_channel: "AlertReceiveChannel"
     filtering_labels: typing.Optional[list["LabelPair"]]
 
     order_with_respect_to = ["alert_receive_channel_id", "is_default"]
@@ -68,6 +69,14 @@ class ChannelFilter(OrderedModel):
     notify_in_telegram = models.BooleanField(null=True, default=False)
 
     slack_channel_id = models.CharField(max_length=100, null=True, default=None)
+    # TODO: migrate slack_channel_id to slack_channel
+    # slack_channel = models.ForeignKey(
+    #     'slack.SlackChannel',
+    #     null=True,
+    #     default=None,
+    #     on_delete=models.SET_NULL,
+    #     related_name='+',
+    # )
 
     telegram_channel = models.ForeignKey(
         "telegram.TelegramToOrganizationConnector",
@@ -167,7 +176,7 @@ def slack_channel_id_or_general_log_id(self):
         if slack_team_identity is None:
             return None
         if self.slack_channel_id is None:
-            return organization.general_log_channel_id
+            return organization.default_slack_channel_slack_id
         else:
             return self.slack_channel_id
 

engine/apps/alerts/models/resolution_note.py

@@ -73,7 +73,17 @@ class ResolutionNoteSlackMessage(models.Model):
         related_name="added_resolution_note_slack_messages",
     )
     text = models.TextField(max_length=3000, default=None, null=True)
+
     slack_channel_id = models.CharField(max_length=100, null=True, default=None)
+    # TODO: migrate slack_channel_id to slack_channel
+    # slack_channel = models.ForeignKey(
+    #     'slack.SlackChannel',
+    #     null=True,
+    #     default=None,
+    #     on_delete=models.SET_NULL,
+    #     related_name='+',
+    # )
+
     ts = models.CharField(max_length=100, null=True, default=None)
     thread_ts = models.CharField(max_length=100, null=True, default=None)
     permalink = models.CharField(max_length=250, null=True, default=None)

engine/apps/alerts/tests/test_alert_receiver_channel.py

@@ -167,7 +167,7 @@ def test_send_demo_alert_not_enabled(mocked_create_alert, make_organization, mak
 
 @pytest.mark.django_db
 def test_notify_maintenance_no_general_channel(make_organization, make_alert_receive_channel):
-    organization = make_organization(general_log_channel_id=None)
+    organization = make_organization(default_slack_channel=None)
     alert_receive_channel = make_alert_receive_channel(organization)
 
     with patch("apps.alerts.models.alert_receive_channel.post_message_to_channel") as mock_post_message:
@@ -177,21 +177,34 @@ def test_notify_maintenance_no_general_channel(make_organization, make_alert_rec
 
 
 @pytest.mark.django_db
-def test_notify_maintenance_with_general_channel(make_organization, make_alert_receive_channel):
-    organization = make_organization(general_log_channel_id="CHANNEL-ID")
+def test_notify_maintenance_with_general_channel(
+    make_organization,
+    make_alert_receive_channel,
+    make_slack_team_identity,
+    make_slack_channel,
+):
+    slack_channel = make_slack_channel(make_slack_team_identity())
+    organization = make_organization(default_slack_channel=slack_channel)
     alert_receive_channel = make_alert_receive_channel(organization)
 
     with patch("apps.alerts.models.alert_receive_channel.post_message_to_channel") as mock_post_message:
         alert_receive_channel.notify_about_maintenance_action("maintenance mode enabled")
 
     mock_post_message.assert_called_once_with(
-        organization, organization.general_log_channel_id, "maintenance mode enabled"
+        organization, organization.default_slack_channel.slack_id, "maintenance mode enabled"
     )
 
 
 @pytest.mark.django_db
-def test_get_or_create_manual_integration_deleted_team(make_organization, make_team, make_alert_receive_channel):
-    organization = make_organization(general_log_channel_id="CHANNEL-ID")
+def test_get_or_create_manual_integration_deleted_team(
+    make_organization,
+    make_team,
+    make_slack_team_identity,
+    make_slack_channel,
+):
+    slack_channel = make_slack_channel(make_slack_team_identity())
+    organization = make_organization(default_slack_channel=slack_channel)
+
     # setup general manual integration
     general_manual = AlertReceiveChannel.get_or_create_manual_integration(
         organization=organization, team=None, integration=AlertReceiveChannel.INTEGRATION_MANUAL, defaults={}

engine/apps/api/serializers/organization.py

@@ -2,6 +2,7 @@
 
 from rest_framework import serializers
 
+from apps.api.serializers.slack_channel import SlackChannelSerializer
 from apps.base.messaging import get_messaging_backend_from_id
 from apps.base.models import LiveSetting
 from apps.phone_notifications.phone_provider import get_phone_provider
@@ -21,7 +22,7 @@ class OrganizationSerializer(EagerLoadingMixin, serializers.ModelSerializer):
     slack_team_identity = FastSlackTeamIdentitySerializer(read_only=True)
 
     name = serializers.CharField(required=False, allow_null=True, allow_blank=True, source="org_title")
-    slack_channel = serializers.SerializerMethodField()
+    slack_channel = SlackChannelSerializer(read_only=True, allow_null=True, required=False)
 
     rbac_enabled = serializers.BooleanField(read_only=True, source="is_rbac_permissions_enabled")
     grafana_incident_enabled = serializers.BooleanField(read_only=True, source="is_grafana_incident_enabled")
@@ -47,22 +48,6 @@ class Meta:
             "grafana_incident_enabled",
         ]
 
-    def get_slack_channel(self, obj):
-        from apps.slack.models import SlackChannel
-
-        if obj.general_log_channel_id is None or obj.slack_team_identity is None:
-            return None
-        try:
-            channel = obj.slack_team_identity.get_cached_channels().get(slack_id=obj.general_log_channel_id)
-        except SlackChannel.DoesNotExist:
-            return {"display_name": None, "slack_id": obj.general_log_channel_id, "id": None}
-
-        return {
-            "display_name": channel.name,
-            "slack_id": channel.slack_id,
-            "id": channel.public_primary_key,
-        }
-
 
 class CurrentOrganizationSerializer(OrganizationSerializer):
     env_status = serializers.SerializerMethodField()

engine/apps/api/tests/test_set_general_log_channel.py → engine/apps/api/tests/test_set_org_default_slack_channel.py

@@ -20,7 +20,7 @@
         (LegacyAccessControlRole.NONE, status.HTTP_403_FORBIDDEN),
     ],
 )
-def test_set_general_log_channel_permissions(
+def test_set_org_default_slack_channel_permissions(
     make_organization_and_user_with_plugin_token,
     make_user_auth_headers,
     role,
@@ -29,8 +29,10 @@ def test_set_general_log_channel_permissions(
     _, user, token = make_organization_and_user_with_plugin_token(role)
     client = APIClient()
 
-    url = reverse("api-internal:api-set-general-log-channel")
-    with patch("apps.api.views.organization.SetGeneralChannel.post", return_value=Response(status=status.HTTP_200_OK)):
+    url = reverse("api-internal:set-default-slack-channel")
+    with patch(
+        "apps.api.views.organization.SetDefaultSlackChannel.post", return_value=Response(status=status.HTTP_200_OK)
+    ):
         response = client.post(url, format="json", **make_user_auth_headers(user, token))
 
     assert response.status_code == expected_status

engine/apps/api/urls.py

@@ -22,7 +22,7 @@
     GetChannelVerificationCode,
     GetTelegramVerificationCode,
     OrganizationConfigChecksView,
-    SetGeneralChannel,
+    SetDefaultSlackChannel,
 )
 from .views.preview_template_options import PreviewTemplateOptionsView
 from .views.public_api_tokens import PublicApiTokenView
@@ -71,7 +71,7 @@
 urlpatterns = [
     path("", include(router.urls)),
     optional_slash_path("user", CurrentUserView.as_view(), name="api-user"),
-    optional_slash_path("set_general_channel", SetGeneralChannel.as_view(), name="api-set-general-log-channel"),
+    optional_slash_path("set_general_channel", SetDefaultSlackChannel.as_view(), name="set-default-slack-channel"),
     optional_slash_path("organization", CurrentOrganizationView.as_view(), name="api-organization"),
     optional_slash_path(
         "organization/config-checks",

engine/apps/api/views/organization.py

@@ -108,7 +108,7 @@ def get(self, request):
         return Response(code)
 
 
-class SetGeneralChannel(APIView):
+class SetDefaultSlackChannel(APIView):
     authentication_classes = (PluginAuthentication,)
     permission_classes = (IsAuthenticated, RBACPermission)
 
@@ -127,6 +127,6 @@ def post(self, request):
             public_primary_key=slack_channel_id, slack_team_identity=slack_team_identity
         )
 
-        organization.set_general_log_channel(slack_channel.slack_id, slack_channel.name, request.user)
+        organization.set_default_slack_channel(slack_channel, request.user)
 
         return Response(status=200)

engine/apps/integrations/tasks.py

@@ -169,9 +169,17 @@ def notify_about_integration_ratelimit_in_slack(organization_id, text, **kwargs)
     else:
         cache.set(cache_key, True, 60 * 15)  # Set cache before sending message to make sure we don't ratelimit slack
         slack_team_identity = organization.slack_team_identity
-        if slack_team_identity is not None:
+        org_default_slack_channel_id = organization.default_slack_channel_slack_id
+
+        if slack_team_identity is not None and org_default_slack_channel_id is not None:
             try:
                 sc = SlackClient(slack_team_identity, enable_ratelimit_retry=True)
-                sc.chat_postMessage(channel=organization.general_log_channel_id, text=text)
+                sc.chat_postMessage(channel=org_default_slack_channel_id, text=text)
             except SlackAPIError as e:
                 logger.warning(f"Slack exception {e} while sending message for organization {organization_id}")
+        else:
+            logger.info(
+                f"Slack team identity or general log channel is not set for organization {organization_id} "
+                f"skipping rest of notify_about_integration_ratelimit_in_slack "
+                f"slack_team_identity={slack_team_identity} org_default_slack_channel_id={org_default_slack_channel_id}"
+            )

engine/apps/slack/models/slack_team_identity.py

@@ -19,10 +19,13 @@
 if typing.TYPE_CHECKING:
     from django.db.models.manager import RelatedManager
 
+    from apps.slack.models import SlackChannel
+
 logger = logging.getLogger(__name__)
 
 
 class SlackTeamIdentity(models.Model):
+    cached_channels: "RelatedManager['SlackChannel']"
     organizations: "RelatedManager[Organization]"
 
     id = models.AutoField(primary_key=True)

engine/apps/slack/scenarios/distribute_alerts.py

@@ -71,7 +71,7 @@ def process_signal(self, alert: Alert) -> None:
                     alert.group.channel_filter.slack_channel_id_or_general_log_id
                     if alert.group.channel_filter
                     # if channel filter is deleted mid escalation, use default Slack channel
-                    else alert.group.channel.organization.general_log_channel_id
+                    else alert.group.channel.organization.default_slack_channel_slack_id
                 )
                 self._send_first_alert(alert, channel_id)
             except (SlackAPIError, TimeoutError):

engine/apps/slack/tasks.py

@@ -198,8 +198,8 @@ def unpopulate_slack_user_identities(organization_pk, force=False, ts=None):
 
     if force:
         organization.slack_team_identity = None
-        organization.general_log_channel_id = None
-        organization.save(update_fields=["slack_team_identity", "general_log_channel_id"])
+        organization.default_slack_channel = None
+        organization.save(update_fields=["slack_team_identity", "default_slack_channel"])
 
 
 @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=0)
@@ -555,11 +555,14 @@ def clean_slack_integration_leftovers(organization_id, *args, **kwargs):
 @shared_dedicated_queue_retry_task(autoretry_for=(Exception,), retry_backoff=True, max_retries=10)
 def clean_slack_channel_leftovers(slack_team_identity_id, slack_channel_id):
     """
-    This task removes binding to slack channel after channel arcived or deleted in slack.
+    TODO: once we add/migrate to ChannelFilter.slack_channel, this will mean that we no longer need this task
+    and it can be safely removed (foreign key relationships to a slack channel that is deleted in the db will
+    automatically be set to None due to on_delete=models.SET_NULL)
+
+    This task removes binding to slack channel after channel archived or deleted in slack.
     """
     from apps.alerts.models import ChannelFilter
     from apps.slack.models import SlackTeamIdentity
-    from apps.user_management.models import Organization
 
     try:
         sti = SlackTeamIdentity.objects.get(id=slack_team_identity_id)
@@ -569,16 +572,7 @@ def clean_slack_channel_leftovers(slack_team_identity_id, slack_channel_id):
         )
         return
 
-    orgs_to_clean_general_log_channel_id = []
     for org in sti.organizations.all():
-        if org.general_log_channel_id == slack_channel_id:
-            logger.info(
-                f"Set general_log_channel_id to None for org_id={org.id}  slack_channel_id={slack_channel_id} since slack_channel is arcived or deleted"
-            )
-            org.general_log_channel_id = None
-            orgs_to_clean_general_log_channel_id.append(org)
         ChannelFilter.objects.filter(alert_receive_channel__organization=org, slack_channel_id=slack_channel_id).update(
             slack_channel_id=None
         )
-
-    Organization.objects.bulk_update(orgs_to_clean_general_log_channel_id, ["general_log_channel_id"], batch_size=5000)

engine/apps/slack/tests/test_reset_slack.py

@@ -69,10 +69,19 @@ def test_clean_slack_integration_leftovers(
 
 @pytest.mark.django_db
 def test_unpopulate_slack_user_identities(
-    make_organization_and_user_with_slack_identities, make_user_with_slack_user_identity
+    make_slack_team_identity,
+    make_slack_channel,
+    make_organization,
+    make_user_for_organization,
+    make_user_with_slack_user_identity,
 ):
     # create organization and user with Slack connected
-    organization, user, slack_team_identity, _ = make_organization_and_user_with_slack_identities()
+    slack_team_identity = make_slack_team_identity()
+    slack_channel = make_slack_channel(slack_team_identity)
+    organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel)
+    user = make_user_for_organization(organization)
+
+    assert organization.default_slack_channel_slack_id is not None
 
     # create & delete user with Slack connected
     deleted_user, _ = make_user_with_slack_user_identity(slack_team_identity, organization)
@@ -90,4 +99,4 @@ def test_unpopulate_slack_user_identities(
 
     # check that Slack specific info is reset for organization
     assert organization.slack_team_identity is None
-    assert organization.general_log_channel_id is None
+    assert organization.default_slack_channel_slack_id is None

engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py

@@ -55,16 +55,16 @@ def test_skip_escalations_error(
 @pytest.mark.django_db
 def test_timeout_error(
     make_slack_team_identity,
+    make_slack_channel,
     make_organization,
     make_alert_receive_channel,
     make_alert_group,
     make_alert,
 ):
     SlackAlertShootingStep = ScenarioStep.get_step("distribute_alerts", "AlertShootingStep")
     slack_team_identity = make_slack_team_identity()
-    organization = make_organization(
-        slack_team_identity=slack_team_identity, general_log_channel_id="DEFAULT_CHANNEL_ID"
-    )
+    slack_channel = make_slack_channel(slack_team_identity)
+    organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel)
     alert_receive_channel = make_alert_receive_channel(organization)
     alert_group = make_alert_group(alert_receive_channel)
     alert = make_alert(alert_group, raw_request_data="{}")
@@ -89,15 +89,15 @@ def test_timeout_error(
 def test_alert_shooting_no_channel_filter(
     mock_post_alert_group_to_slack,
     make_slack_team_identity,
+    make_slack_channel,
     make_organization,
     make_alert_receive_channel,
     make_alert_group,
     make_alert,
 ):
     slack_team_identity = make_slack_team_identity()
-    organization = make_organization(
-        slack_team_identity=slack_team_identity, general_log_channel_id="DEFAULT_CHANNEL_ID"
-    )
+    slack_channel = make_slack_channel(slack_team_identity, slack_id="DEFAULT_CHANNEL_ID")
+    organization = make_organization(slack_team_identity=slack_team_identity, default_slack_channel=slack_channel)
     alert_receive_channel = make_alert_receive_channel(organization)
 
     # simulate an alert group with channel filter deleted in the middle of the escalation

engine/apps/user_management/migrations/0025_organization_default_slack_channel.py

@@ -0,0 +1,20 @@
+# Generated by Django 4.2.16 on 2024-10-17 19:07
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('slack', '0005_slackteamidentity__unified_slack_app_installed'),
+        ('user_management', '0024_organization_direct_paging_prefer_important_policy'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='organization',
+            name='default_slack_channel',
+            field=models.ForeignKey(default=None, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to='slack.slackchannel'),
+        ),
+    ]