From 5abeaeddafe9840a25d600cb592bdd07bb45e4d9 Mon Sep 17 00:00:00 2001 From: Nathan Hsieh <6186377+nhsiehgit@users.noreply.github.com> Date: Thu, 16 May 2024 13:15:29 -0700 Subject: [PATCH] fix: ONLY fetch apps for the ORGANIZATION (#71036) App installations were fetching ALL installations for an app. This was causing requests to timeout via HC --- .../organization_alert_rule_details.py | 6 +- src/sentry/incidents/models/alert_rule.py | 2 +- .../test_organization_alert_rule_details.py | 74 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index cad99b962d8ad6..3206ce6ee734b2 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -58,9 +58,11 @@ def fetch_alert_rule(request: Request, organization, alert_rule): action.get("_sentry_app_installation", {}).get("sentry_app_id", None) ) if sentry_app_ids: + fetched_rpc_installations = app_service.get_many( + filter=dict(app_ids=sentry_app_ids, organization_id=organization.id) + ) sentry_app_map = { - install.sentry_app.id: install.sentry_app - for install in app_service.get_many(filter=dict(app_ids=sentry_app_ids)) + install.sentry_app.id: install.sentry_app for install in fetched_rpc_installations } # Prepare AlertRuleTriggerActions that are SentryApp components diff --git a/src/sentry/incidents/models/alert_rule.py b/src/sentry/incidents/models/alert_rule.py index 746e5e66b1aa5b..bb6aa548ffdeed 100644 --- a/src/sentry/incidents/models/alert_rule.py +++ b/src/sentry/incidents/models/alert_rule.py @@ -451,7 +451,7 @@ def get_queryset(self): @region_silo_model class AlertRuleTriggerAction(AbstractNotificationAction): """ - This model represents an action that occurs when a trigger is fired. This is + This model represents an action that occurs when a trigger (over/under) is fired. This is typically some sort of notification. """ diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index ec86636a01fb6e..a4d1222cf67d3a 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -3,6 +3,7 @@ from copy import deepcopy from functools import cached_property from typing import Any +from unittest import mock from unittest.mock import patch import responses @@ -245,6 +246,79 @@ def test_with_sentryapp_success(self): assert "sentry/members" in action["formFields"]["optional_fields"][-1]["uri"] assert "bob" == action["formFields"]["optional_fields"][-1]["choices"][0][0] + @responses.activate + def test_with_sentryapp_multiple_installations_filters_by_org(self): + self.superuser = self.create_user("admin@localhost", is_superuser=True) + self.login_as(user=self.superuser) + self.create_team(organization=self.organization, members=[self.superuser]) + + org2 = self.create_organization(name="org2") + + sentry_app = self.create_sentry_app( + organization=self.organization, + published=True, + verify_install=False, + name="Super Awesome App", + schema={"elements": [self.create_alert_rule_action_schema()]}, + ) + self.create_sentry_app_installation( + slug=sentry_app.slug, organization=self.organization, user=self.superuser + ) + self.create_sentry_app_installation( + slug=sentry_app.slug, organization=org2, user=self.superuser + ) + + getmany_response = app_service.get_many( + filter=dict(app_ids=[sentry_app.id], organization_id=self.organization.id) + ) + + rule = self.create_alert_rule() + trigger = self.create_alert_rule_trigger(rule, "hi", 1000) + self.create_alert_rule_trigger_action( + alert_rule_trigger=trigger, + target_identifier=sentry_app.id, + type=AlertRuleTriggerAction.Type.SENTRY_APP, + target_type=AlertRuleTriggerAction.TargetType.SENTRY_APP, + sentry_app=sentry_app, + sentry_app_config=[ + {"name": "title", "value": "An alert"}, + {"summary": "Something happened here..."}, + {"name": "points", "value": "3"}, + {"name": "assignee", "value": "Nisanthan"}, + ], + ) + + responses.add( + responses.GET, + "https://example.com/sentry/members", + json=[ + {"value": "bob", "label": "Bob"}, + {"value": "jess", "label": "Jess"}, + ], + status=200, + ) + with self.feature("organizations:incidents"): + with mock.patch.object(app_service, "get_many") as mock_get_many: + mock_get_many.return_value = getmany_response + resp = self.get_response(self.organization.slug, rule.id) + + assert mock_get_many.call_count == 1 + mock_get_many.assert_called_with( + filter={ + "app_ids": [sentry_app.id], + "organization_id": self.organization.id, + }, + ) + + assert resp.status_code == 200 + assert len(responses.calls) == 1 + assert "errors" not in resp.data + + action = resp.data["triggers"][0]["actions"][0] + assert "select" == action["formFields"]["optional_fields"][-1]["type"] + assert "sentry/members" in action["formFields"]["optional_fields"][-1]["uri"] + assert "bob" == action["formFields"]["optional_fields"][-1]["choices"][0][0] + @responses.activate def test_with_unresponsive_sentryapp(self): self.superuser = self.create_user("admin@localhost", is_superuser=True)