Browse Source

feat(slack): Add flags to toggle features on organization integration (#70620)

Features that we add to integrations should have the ability to be
turned off by users. Use the Slack Threads feature as a stepping stone
for this pattern to add feature toggle to organization integrations,
allowing organizations to turn features on or off.

The features are turned on by default as we are about to GA the feature.
Yash Kamothi 10 months ago
parent
commit
64a5b812f0

+ 1 - 2
src/sentry/api/serializers/models/integration.py

@@ -155,13 +155,12 @@ class OrganizationIntegrationSerializer(Serializer):
         try:
         try:
             installation = integration.get_installation(organization_id=obj.organization_id)
             installation = integration.get_installation(organization_id=obj.organization_id)
         except NotImplementedError:
         except NotImplementedError:
-            # slack doesn't have an installation implementation
             config_data = obj.config if include_config else None
             config_data = obj.config if include_config else None
         else:
         else:
             try:
             try:
                 # just doing this to avoid querying for an object we already have
                 # just doing this to avoid querying for an object we already have
                 installation._org_integration = obj
                 installation._org_integration = obj
-                config_data = installation.get_config_data() if include_config else None  # type: ignore[assignment]
+                config_data = installation.get_config_data() if include_config else None
                 dynamic_display_information = installation.get_dynamic_display_information()
                 dynamic_display_information = installation.get_dynamic_display_information()
             except ApiError as e:
             except ApiError as e:
                 # If there is an ApiError from our 3rd party integration
                 # If there is an ApiError from our 3rd party integration

+ 1 - 1
src/sentry/integrations/base.py

@@ -354,7 +354,7 @@ class IntegrationInstallation:
             config=config,
             config=config,
         )
         )
 
 
-    def get_config_data(self) -> Mapping[str, str]:
+    def get_config_data(self) -> dict[str, Any]:
         if not self.org_integration:
         if not self.org_integration:
             return {}
             return {}
         return self.org_integration.config
         return self.org_integration.config

+ 63 - 3
src/sentry/integrations/slack/integration.py

@@ -1,7 +1,8 @@
 from __future__ import annotations
 from __future__ import annotations
 
 
+import logging
 from collections import namedtuple
 from collections import namedtuple
-from collections.abc import Mapping, Sequence
+from collections.abc import Mapping, MutableMapping, Sequence
 from typing import Any
 from typing import Any
 
 
 from django.utils.translation import gettext_lazy as _
 from django.utils.translation import gettext_lazy as _
@@ -67,18 +68,77 @@ metadata = IntegrationMetadata(
     aspects={"alerts": [setup_alert]},
     aspects={"alerts": [setup_alert]},
 )
 )
 
 
+_default_logger = logging.getLogger(__name__)
+
 
 
 class SlackIntegration(SlackNotifyBasicMixin, IntegrationInstallation):
 class SlackIntegration(SlackNotifyBasicMixin, IntegrationInstallation):
+    _FLAGS_KEY: str = "toggleableFlags"
+    _ISSUE_ALERTS_THREAD_FLAG: str = "issueAlertsThreadFlag"
+    _METRIC_ALERTS_THREAD_FLAG: str = "metricAlertsThreadFlag"
+    _SUPPORTED_FLAGS_WITH_DEFAULTS: dict[str, bool] = {
+        _ISSUE_ALERTS_THREAD_FLAG: True,
+        _METRIC_ALERTS_THREAD_FLAG: True,
+    }
+
     def get_client(self) -> SlackClient:
     def get_client(self) -> SlackClient:
         return SlackClient(integration_id=self.model.id)
         return SlackClient(integration_id=self.model.id)
 
 
-    def get_config_data(self) -> Mapping[str, str]:
+    def get_config_data(self) -> Mapping[str, Any]:
+        base_data = super().get_config_data()
+
+        # Add installationType key to config data
         metadata_ = self.model.metadata
         metadata_ = self.model.metadata
         # Classic bots had a user_access_token in the metadata.
         # Classic bots had a user_access_token in the metadata.
         default_installation = (
         default_installation = (
             "classic_bot" if "user_access_token" in metadata_ else "workspace_app"
             "classic_bot" if "user_access_token" in metadata_ else "workspace_app"
         )
         )
-        return {"installationType": metadata_.get("installation_type", default_installation)}
+        base_data["installationType"] = metadata_.get("installation_type", default_installation)
+
+        # Add missing toggleable feature flags
+        stored_flag_data = base_data.get(self._FLAGS_KEY, {})
+        for flag_name, default_flag_value in self._SUPPORTED_FLAGS_WITH_DEFAULTS.items():
+            if flag_name not in stored_flag_data:
+                stored_flag_data[flag_name] = default_flag_value
+
+        base_data[self._FLAGS_KEY] = stored_flag_data
+        return base_data
+
+    def _update_and_clean_flags_in_organization_config(
+        self, data: MutableMapping[str, Any]
+    ) -> None:
+        """
+        Checks the new provided data for the flags key.
+        If the key does not exist, uses the default set values.
+        """
+
+        cleaned_flags_data = data.get(self._FLAGS_KEY, {})
+        # ensure we add the default supported flags if they don't already exist
+        for flag_name, default_flag_value in self._SUPPORTED_FLAGS_WITH_DEFAULTS.items():
+            flag_value = cleaned_flags_data.get(flag_name, None)
+            if flag_value is None:
+                cleaned_flags_data[flag_name] = default_flag_value
+            else:
+                # if the type for the flag is not the same as the default, use the default value as an override
+                if type(flag_value) is not type(default_flag_value):
+                    _default_logger.info(
+                        "Flag value was not correct, overriding with default",
+                        extra={
+                            "flag_name": flag_name,
+                            "flag_value": flag_value,
+                            "default_flag_value": default_flag_value,
+                        },
+                    )
+                    cleaned_flags_data[flag_name] = default_flag_value
+
+        data[self._FLAGS_KEY] = cleaned_flags_data
+
+    def update_organization_config(self, data: MutableMapping[str, Any]) -> None:
+        """
+        Update the organization's configuration, but make sure to properly handle specific things for Slack installation
+        before passing it off to the parent method
+        """
+        self._update_and_clean_flags_in_organization_config(data=data)
+        super().update_organization_config(data=data)
 
 
 
 
 class SlackIntegrationProvider(IntegrationProvider):
 class SlackIntegrationProvider(IntegrationProvider):

+ 0 - 0
tests/sentry/integrations/slack/integration/__init__.py


+ 134 - 0
tests/sentry/integrations/slack/integration/test_slack_integration.py

@@ -0,0 +1,134 @@
+from typing import Any
+
+from sentry.integrations.slack import SlackIntegration
+from sentry.testutils.cases import TestCase
+from sentry.testutils.silo import control_silo_test
+
+
+class _BaseTestCase(TestCase):
+    def setUp(self) -> None:
+        self.slack_provider_integration = self.create_provider_integration(
+            provider="slack", name="Slack", metadata={}
+        )
+        self.slack_installation = SlackIntegration(
+            self.slack_provider_integration, self.organization.id
+        )
+
+
+@control_silo_test
+class TestGetConfigData(_BaseTestCase):
+    def test_gets_default_flags_when_no_data_exists(self) -> None:
+        self.create_organization_integration(
+            organization_id=self.organization.id,
+            integration=self.slack_provider_integration,
+            config={},
+        )
+        data = self.slack_installation.get_config_data()
+        results = data.get("toggleableFlags")
+        assert results == {
+            "issueAlertsThreadFlag": True,
+            "metricAlertsThreadFlag": True,
+        }
+
+    def test_gets_missing_flags(self) -> None:
+        self.create_organization_integration(
+            organization_id=self.organization.id,
+            integration=self.slack_provider_integration,
+            config={
+                "toggleableFlags": {
+                    "issueAlertsThreadFlag": False,
+                }
+            },
+        )
+        data = self.slack_installation.get_config_data()
+        results = data.get("toggleableFlags")
+        assert results == {
+            "issueAlertsThreadFlag": False,
+            "metricAlertsThreadFlag": True,
+        }
+
+    def test_gets_correct_data(self) -> None:
+        self.create_organization_integration(
+            organization_id=self.organization.id,
+            integration=self.slack_provider_integration,
+            config={
+                "toggleableFlags": {
+                    "issueAlertsThreadFlag": False,
+                    "metricAlertsThreadFlag": False,
+                }
+            },
+        )
+        data = self.slack_installation.get_config_data()
+        results = data.get("toggleableFlags")
+        assert results == {
+            "issueAlertsThreadFlag": False,
+            "metricAlertsThreadFlag": False,
+        }
+
+
+@control_silo_test
+class TestUpdateAndCleanFlagsInOrganizationConfig(_BaseTestCase):
+    def test_adds_flags_key_when_it_does_not_exist(self) -> None:
+        data: dict[str, Any] = {}
+        self.slack_installation._update_and_clean_flags_in_organization_config(data=data)
+
+        assert "toggleableFlags" in data
+
+    def test_adds_default_flags_key_when_it_does_not_exist(self) -> None:
+        data: dict[str, Any] = {}
+        self.slack_installation._update_and_clean_flags_in_organization_config(data=data)
+
+        results = data["toggleableFlags"]
+        assert results == {
+            "issueAlertsThreadFlag": True,
+            "metricAlertsThreadFlag": True,
+        }
+
+    def test_adds_missing_flags(self) -> None:
+        data: dict[str, Any] = {
+            "toggleableFlags": {
+                "issueAlertsThreadFlag": False,
+            }
+        }
+        self.slack_installation._update_and_clean_flags_in_organization_config(data=data)
+
+        results = data["toggleableFlags"]
+        assert results == {
+            "issueAlertsThreadFlag": False,
+            "metricAlertsThreadFlag": True,
+        }
+
+    def test_corrects_bad_flag_values(self) -> None:
+        data: dict[str, Any] = {
+            "toggleableFlags": {
+                "issueAlertsThreadFlag": False,
+                "metricAlertsThreadFlag": 0,
+            }
+        }
+        self.slack_installation._update_and_clean_flags_in_organization_config(data=data)
+
+        results = data["toggleableFlags"]
+        assert results == {
+            "issueAlertsThreadFlag": False,
+            "metricAlertsThreadFlag": True,
+        }
+
+
+@control_silo_test
+class TestUpdateOrganizationConfig(_BaseTestCase):
+    def test_saves_flags(self) -> None:
+        org_integration = self.create_organization_integration(
+            organization_id=self.organization.id,
+            integration=self.slack_provider_integration,
+            config={},
+        )
+        data: dict[str, Any] = {
+            "toggleableFlags": {
+                "issueAlertsThreadFlag": False,
+                "metricAlertsThreadFlag": True,
+            }
+        }
+        self.slack_installation.update_organization_config(data=data)
+
+        org_integration.refresh_from_db()
+        assert org_integration.config == data