Browse Source

feat(ui-components): Get Alert Rule UI Components in Metric Alerts (#29008)

Objective:
- added Sentry App Alert Rule UI Component to the Metric Alerts actions
- moved duplicate Sentry App "preparer" logic into a helper function.
- added validation logic for our Sentry App schema to prevent multiple of the same UI Component type.
NisanthanNanthakumar 3 years ago
parent
commit
ad8e3017e3

+ 14 - 2
src/sentry/api/validators/sentry_apps/schema.py

@@ -225,8 +225,7 @@ def check_each_element_for_error(instance):
         found_type = element["type"]
         if found_type not in element_types:
             raise SchemaValidationError(
-                "Element has type '%s'. Type must be one of the following: %s"
-                % (found_type, element_types)
+                f"Element has type '{found_type}'. Type must be one of the following: {element_types}"
             )
         try:
             validate_component(element)
@@ -237,11 +236,24 @@ def check_each_element_for_error(instance):
             )
 
 
+def check_only_one_of_each_element(instance):
+    if "elements" not in instance:
+        return
+    found = {}
+    for element in instance["elements"]:
+        if element["type"]:
+            if element["type"] not in found:
+                found[element["type"]] = 1
+            else:
+                raise SchemaValidationError(f"Multiple elements of type: {element['type']}")
+
+
 def validate_ui_element_schema(instance):
     try:
         # schema validator will catch elements missing
         check_elements_is_array(instance)
         check_each_element_for_error(instance)
+        check_only_one_of_each_element(instance)
     except SchemaValidationError as e:
         raise e
     except Exception as e:

+ 21 - 8
src/sentry/incidents/endpoints/organization_alert_rule_available_action_index.py

@@ -10,10 +10,12 @@ from sentry.incidents.endpoints.bases import OrganizationEndpoint
 from sentry.incidents.endpoints.serializers import action_target_type_to_string
 from sentry.incidents.logic import get_available_action_integrations_for_org, get_pagerduty_services
 from sentry.incidents.models import AlertRuleTriggerAction
-from sentry.models import SentryApp
+from sentry.models import SentryAppInstallation
 
 
-def build_action_response(registered_type, integration=None, organization=None, sentry_app=None):
+def build_action_response(
+    registered_type, integration=None, organization=None, sentry_app_installation=None
+):
     """
     Build the "available action" objects for the API. Each one can have different fields.
 
@@ -42,10 +44,17 @@ def build_action_response(registered_type, integration=None, organization=None,
                 for service in get_pagerduty_services(organization, integration.id)
             ]
 
-    elif sentry_app:
-        action_response["sentryAppName"] = sentry_app.name
-        action_response["sentryAppId"] = sentry_app.id
-        action_response["status"] = SentryAppStatus.as_str(sentry_app.status)
+    elif sentry_app_installation:
+        action_response["sentryAppName"] = sentry_app_installation.sentry_app.name
+        action_response["sentryAppId"] = sentry_app_installation.sentry_app_id
+        action_response["status"] = SentryAppStatus.as_str(
+            sentry_app_installation.sentry_app.status
+        )
+
+        # Sentry Apps can be alertable but not have an Alert Rule UI Component
+        component = sentry_app_installation.prepare_sentry_app_components("alert-rule-action")
+        if component:
+            action_response["settings"] = component.schema.get("settings", {})
 
     return action_response
 
@@ -78,8 +87,12 @@ class OrganizationAlertRuleAvailableActionIndexEndpoint(OrganizationEndpoint):
             # Add all alertable SentryApps to the list.
             elif registered_type.type == AlertRuleTriggerAction.Type.SENTRY_APP:
                 actions += [
-                    build_action_response(registered_type, sentry_app=app)
-                    for app in SentryApp.objects.get_alertable_sentry_apps(organization.id)
+                    build_action_response(registered_type, sentry_app_installation=install)
+                    for install in SentryAppInstallation.get_installed_for_org(
+                        organization.id
+                    ).filter(
+                        sentry_app__is_alertable=True,
+                    )
                 ]
 
             else:

+ 1 - 1
src/sentry/mediators/sentry_app_components/preparer.py

@@ -9,7 +9,7 @@ from sentry.mediators.external_requests import SelectRequester
 class Preparer(Mediator):
     component = Param("sentry.models.SentryAppComponent")
     install = Param("sentry.models.SentryAppInstallation")
-    project = Param("sentry.models.Project")
+    project = Param("sentry.models.Project", required=False, default=None)
 
     def call(self):
         if self.component.type == "issue-link":

+ 22 - 1
src/sentry/models/sentryappinstallation.py

@@ -137,5 +137,26 @@ class SentryAppInstallation(ParanoidModel):
     @classmethod
     def get_installed_for_org(cls, organization_id):
         return cls.objects.filter(
-            organization_id=organization_id, status=SentryAppInstallationStatus.INSTALLED
+            organization_id=organization_id,
+            status=SentryAppInstallationStatus.INSTALLED,
+            date_deleted=None,
         )
+
+    def prepare_sentry_app_components(self, component_type, project=None):
+        from sentry.coreapi import APIError
+        from sentry.mediators import sentry_app_components
+        from sentry.models import SentryAppComponent
+
+        try:
+            component = SentryAppComponent.objects.get(
+                sentry_app_id=self.sentry_app_id, type=component_type
+            )
+        except SentryAppComponent.DoesNotExist:
+            return None
+
+        try:
+            sentry_app_components.Preparer.run(component=component, install=self, project=project)
+            return component
+        except APIError:
+            # TODO(nisanthan): For now, skip showing the UI Component if the API requests fail
+            return None

+ 12 - 21
src/sentry/rules/actions/notify_event_sentry_app.py

@@ -5,10 +5,8 @@ from typing import Any, Mapping, Optional, Sequence
 
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.sentry_app_component import SentryAppAlertRuleActionSerializer
-from sentry.coreapi import APIError
 from sentry.eventstore.models import Event
-from sentry.mediators import sentry_app_components
-from sentry.models import Project, SentryApp, SentryAppComponent, SentryAppInstallation
+from sentry.models import Project, SentryApp, SentryAppInstallation
 from sentry.rules.actions.base import EventAction
 from sentry.tasks.sentry_apps import notify_sentry_app
 
@@ -25,24 +23,17 @@ class NotifyEventSentryAppAction(EventAction):  # type: ignore
     def get_custom_actions(self, project: Project) -> Sequence[Mapping[str, Any]]:
         action_list = []
         for install in SentryAppInstallation.get_installed_for_org(project.organization_id):
-            _components = SentryAppComponent.objects.filter(
-                sentry_app_id=install.sentry_app_id, type="alert-rule-action"
-            )
-            for component in _components:
-                try:
-                    sentry_app_components.Preparer.run(
-                        component=component, install=install, project=project
-                    )
-                    kwargs = {
-                        "install": install,
-                        "event_action": self,
-                    }
-                    action_details = serialize(
-                        component, None, SentryAppAlertRuleActionSerializer(), **kwargs
-                    )
-                    action_list.append(action_details)
-                except APIError:
-                    continue
+            component = install.prepare_sentry_app_components("alert-rule-action", project)
+            if component:
+                kwargs = {
+                    "install": install,
+                    "event_action": self,
+                }
+                action_details = serialize(
+                    component, None, SentryAppAlertRuleActionSerializer(), **kwargs
+                )
+                action_list.append(action_details)
+
         return action_list
 
     def get_sentry_app(self, event: Event) -> Optional[SentryApp]:

+ 10 - 0
tests/sentry/api/validators/sentry_apps/test_schema.py

@@ -108,3 +108,13 @@ class TestSchemaValidation(TestCase):
             "elements": [{"url": "/stacktrace/github/getsentry/sentry", "type": "stacktrace-link"}]
         }
         validate_ui_element_schema(schema)
+
+    @invalid_schema_with_error_message("Multiple elements of type: stacktrace-link")
+    def test_multiple_of_same_element_type(self):
+        schema = {
+            "elements": [
+                {"uri": "/stacktrace/github/getsentry/sentry", "type": "stacktrace-link"},
+                {"uri": "/stacktrace/github/getsentry/sentry", "type": "stacktrace-link"},
+            ]
+        }
+        validate_ui_element_schema(schema)

+ 15 - 13
tests/sentry/incidents/endpoints/test_organization_alert_rule_available_action_index.py

@@ -32,10 +32,10 @@ class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
             name=name, organization=self.organization, is_alertable=True, verify_install=False
         )
         sentry_app = self.create_sentry_app(**kwargs)
-        self.create_sentry_app_installation(
+        installation = self.create_sentry_app_installation(
             slug=sentry_app.slug, organization=self.organization, user=self.user
         )
-        return sentry_app
+        return installation
 
     def test_build_action_response_email(self):
         data = build_action_response(self.email)
@@ -73,13 +73,9 @@ class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
         assert data["options"] == [{"value": service.id, "label": service_name}]
 
     def test_build_action_response_sentry_app(self):
-        sentry_app = self.create_sentry_app(
-            name="foo", organization=self.organization, is_alertable=True, verify_install=False
-        )
-        self.create_sentry_app_installation(
-            slug=sentry_app.slug, organization=self.organization, user=self.user
-        )
-        data = build_action_response(self.sentry_app, sentry_app=sentry_app)
+        installation = self.install_new_sentry_app("foo")
+
+        data = build_action_response(self.sentry_app, sentry_app_installation=installation)
 
         assert data["type"] == "sentry_app"
         assert data["allowedTargetTypes"] == ["sentry_app"]
@@ -138,24 +134,30 @@ class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
         self.get_error_response(self.organization.slug, status_code=404)
 
     def test_sentry_apps(self):
-        sentry_app = self.install_new_sentry_app("foo")
+        installation = self.install_new_sentry_app("foo")
 
         with self.feature("organizations:incidents"):
             response = self.get_success_response(self.organization.slug)
 
         assert len(response.data) == 2
         assert build_action_response(self.email) in response.data
-        assert build_action_response(self.sentry_app, sentry_app=sentry_app) in response.data
+        assert (
+            build_action_response(self.sentry_app, sentry_app_installation=installation)
+            in response.data
+        )
 
     def test_published_sentry_apps(self):
         # Should show up in available actions.
-        published_app = self.install_new_sentry_app("published", published=True)
+        installation = self.install_new_sentry_app("published", published=True)
 
         with self.feature("organizations:incidents"):
             response = self.get_success_response(self.organization.slug)
 
         assert len(response.data) == 2
-        assert build_action_response(self.sentry_app, sentry_app=published_app) in response.data
+        assert (
+            build_action_response(self.sentry_app, sentry_app_installation=installation)
+            in response.data
+        )
 
     def test_no_ticket_actions(self):
         integration = Integration.objects.create(external_id="1", provider="jira")