Skip to content

Commit

Permalink
fix: ONLY fetch apps for the ORGANIZATION (#71036)
Browse files Browse the repository at this point in the history
App installations were fetching ALL installations for an app.

This was causing requests to timeout via HC
  • Loading branch information
nhsiehgit authored and cmanallen committed May 21, 2024
1 parent 2ec15fe commit 5abeaed
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/incidents/models/alert_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 5abeaed

Please sign in to comment.