Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,7 @@ def render_label(
alert_rule_component = component
break

if not alert_rule_component:
raise ValidationError(
f"Alert Actions are not enabled for the {sentry_app.name} integration."
)

schema_title = alert_rule_component.app_schema.get("title")
schema_title = None
Comment on lines 109 to +110
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The code breaks after finding the first alert-rule-action component, potentially missing a title defined in a subsequent component and causing inconsistent behavior with other parts of the system.
Severity: LOW

Suggested Fix

Modify the loop in sentry_app_issue_alert_handler.py to match the logic in notify_event.py. Instead of breaking on the first alert-rule-action component, iterate through all of them until a non-null title is found. This will ensure consistent title resolution across the application.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
src/sentry/notifications/notification_action/issue_alert_registry/handlers/sentry_app_issue_alert_handler.py#L109-L110

Potential issue: There is a behavioral inconsistency in how the alert rule action title
is resolved. The new code in `sentry_app_issue_alert_handler.py` iterates through a
Sentry App's components and breaks after finding the first one with type
`alert-rule-action`. It then checks for a `title`. If this first component has no title,
it defaults to `sentry_app.name`. However, other parts of the system, like
`notify_event.py`, continue iterating to find any component with a title. This can lead
to different results if a Sentry App has multiple `alert-rule-action` components and the
first one lacks a title while a subsequent one has it.

Did we get this right? 👍 / 👎 to inform future reviews.

if alert_rule_component is not None:
schema_title = alert_rule_component.app_schema.get("title")
return schema_title if schema_title is not None else sentry_app.name
9 changes: 7 additions & 2 deletions src/sentry/rules/actions/sentry_apps/notify_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ def render_label(self) -> str:
raise ValidationError("Could not identify integration from the installation uuid.")

sentry_app = installations[0].sentry_app
alert_rule_component = self._get_alert_rule_component(sentry_app.id, sentry_app.name)
components = app_service.find_app_components(app_id=sentry_app.id)
for component in components:
if component.type == "alert-rule-action":
title = component.app_schema.get("title")
if title is not None:
return str(title)

return str(alert_rule_component.app_schema.get("title"))
return sentry_app.name
5 changes: 2 additions & 3 deletions tests/sentry/api/serializers/test_rule.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import pytest
import responses
from django.db import DEFAULT_DB_ALIAS, connections
from django.test.utils import CaptureQueriesContext
Expand Down Expand Up @@ -895,8 +894,8 @@ def test_sentry_app_render_label_no_alert_rule_action_schema(self) -> None:
include_legacy_rule_id=False,
include_workflow_id=False,
)
with pytest.raises(ValidationError):
self.assert_equal_serializers(rule)

self.assert_equal_serializers(rule)

@responses.activate
def test_sentry_app_render_label_no_installation(self) -> None:
Expand Down
Loading