Skip to content

fix(alerts): Stronger typing around human_desc #117883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kcons wants to merge 3 commits into
base: master
Choose a base branch
from
+78 −42
Open

fix(alerts): Stronger typing around human_desc #117883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4e264a
fix(alerts): Stronger typing around human_desc
kcons Jun 16, 2026
bf5b08f
address pre-existing action_type_to_string incompleteness issue
kcons Jun 17, 2026
6dfa85b
fix pre-existing pagerduty bug
kcons Jun 17, 2026
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
87 changes: 53 additions & 34 deletions src/sentry/incidents/endpoints/serializers/alert_rule_trigger_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,64 @@
from __future__ import annotations

import logging
from collections.abc import Mapping
from typing import Any

from django.contrib.auth.models import AnonymousUser

from sentry.api.serializers import Serializer, register
from sentry.incidents.models.alert_rule import AlertRuleTriggerAction
from sentry.models.organizationmember import OrganizationMember
from sentry.models.team import Team
from sentry.users.models.user import User
from sentry.users.services.user.model import RpcUser
from sentry.workflow_engine.utils.legacy_metric_tracking import report_used_legacy_models

logger = logging.getLogger(__name__)


def human_desc(
action_type,
target_type,
target_identifier,
target,
target_display=None,
action_target=None,
priority=None,
):
# Returns a human readable description to display in the UI
action_type: int,
target_identifier: str | None,
target: OrganizationMember | Team | str | None,
target_display: str | None = None,
priority: str | None = None,
) -> str | None:
"""Return a human-readable description of a metric alert action for display in the UI.

Args:
action_type: An ``ActionService`` enum value (e.g. EMAIL, SLACK, PAGERDUTY).
priority: On-call severity/priority string when applicable.
PagerDuty: "default", "critical", "warning", "error", "info".
Opsgenie: "P1"–"P5".
"""
if priority:
priority += " level"

slack_desc = f"Send a Slack notification to {target_display}"
action_type_to_string = {
AlertRuleTriggerAction.Type.PAGERDUTY.value: f"Send a {priority} PagerDuty notification to {target_display}",
AlertRuleTriggerAction.Type.SLACK.value: f"Send a Slack notification to {target_display}",
AlertRuleTriggerAction.Type.PAGERDUTY.value: (
f"Send a {priority} PagerDuty notification to {target_display}"
if priority
else f"Send a PagerDuty notification to {target_display}"
),
AlertRuleTriggerAction.Type.SLACK.value: slack_desc,
AlertRuleTriggerAction.Type.SLACK_STAGING.value: slack_desc,
AlertRuleTriggerAction.Type.MSTEAMS.value: f"Send a Microsoft Teams notification to {target_display}",
AlertRuleTriggerAction.Type.SENTRY_APP.value: f"Send a notification via {target_display}",
}

if action_type == AlertRuleTriggerAction.Type.EMAIL.value:
if action_target:
if target_type == AlertRuleTriggerAction.TargetType.USER.value:
email = target.get_email() if target else "[removed]"
return "Send a notification to " + email
elif target_type == AlertRuleTriggerAction.TargetType.TEAM.value:
slug = "#" + target.slug if target else "[removed]"
return "Send an email to members of " + slug
else:
logger.info("email.action.description.no_action_target")
return "Send an email to [removed]"
if isinstance(target, OrganizationMember):
return "Send a notification to " + target.get_email()
elif isinstance(target, Team):
return "Send an email to members of #" + target.slug
logger.info("email.action.description.no_action_target")
return "Send an email to [removed]"
elif action_type == AlertRuleTriggerAction.Type.OPSGENIE.value:
if priority:
return f"Send a {priority} Opsgenie notification to {target_display}"
return "Send an Opsgenie notification to {target_display}"
return f"Send an Opsgenie notification to {target_display}"
elif action_type == AlertRuleTriggerAction.Type.DISCORD.value:
if not target_display:
logger.info(
Expand All @@ -50,7 +67,7 @@ def human_desc(
)
return f"Send a Discord notification to {target_display}"
else:
return action_type_to_string[action_type]
return action_type_to_string.get(action_type)


def get_identifier_from_action(action_type, target_identifier, target_display=None):
Expand All @@ -74,15 +91,23 @@ def get_input_channel_id(action_type, target_identifier=None):


@register(AlertRuleTriggerAction)
class AlertRuleTriggerActionSerializer(Serializer):
def serialize(self, obj, attrs, user, **kwargs):
class AlertRuleTriggerActionSerializer(Serializer[dict[str, Any]]):
def serialize(
self,
obj: AlertRuleTriggerAction,
attrs: Mapping[str, Any],
user: User | RpcUser | AnonymousUser,
**kwargs: Any,
) -> dict[str, Any]:
# Mark that we're using legacy AlertRuleTriggerAction models
report_used_legacy_models()

from sentry.incidents.serializers import ACTION_TARGET_TYPE_TO_STRING

priority = (
obj.sentry_app_config.get("priority") if isinstance(obj.sentry_app_config, dict) else ""
priority: str | None = (
obj.sentry_app_config.get("priority")
if isinstance(obj.sentry_app_config, dict)
else None
)
result = {
"id": str(obj.id),
Expand All @@ -102,18 +127,12 @@ def serialize(self, obj, attrs, user, **kwargs):
"dateCreated": obj.date_added,
"desc": human_desc(
obj.type,
obj.target_type,
obj.target_identifier,
obj.target,
obj.target_display,
obj.target,
priority,
),
"priority": (
obj.sentry_app_config.get("priority", None)
if isinstance(obj.sentry_app_config, dict)
else None
),
"priority": priority,
}

# Check if action is a Sentry App that has Alert Rule UI Component settings
Expand Down
11 changes: 5 additions & 6 deletions src/sentry/incidents/endpoints/serializers/workflow_engine_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ def serialize(
alert_rule_trigger_id = kwargs.get("alert_rule_trigger_id", -1)

aarta = attrs.get("aarta")
priority = obj.data.get("priority")
priority: str | None = obj.data.get("priority")
type_value = ActionService.get_value(obj.type)
assert type_value is not None, f"Unknown ActionService for type {obj.type}"

@sentry sentry Bot Jun 17, 2026

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 assert in WorkflowEngineActionSerializer will cause a crash when serializing workflow actions with types like GITHUB or JIRA, as they are not supported by ActionService.
Severity: HIGH

Suggested Fix

Instead of asserting, the serializer should gracefully handle unsupported action types. This could involve filtering them out from the serialized output or logging a warning and skipping the problematic action, which would prevent the API endpoint from crashing.

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/incidents/endpoints/serializers/workflow_engine_action.py#L58

Potential issue: The `WorkflowEngineActionSerializer` uses an `assert` to validate that
the `obj.type` of a `workflow_engine.Action` is a valid `ActionService`. However, the
`Action.Type` enum allows for types like `GITHUB`, `JIRA`, `WEBHOOK`, and `PLUGIN`,
which are not defined in `ActionService`. When `ActionService.get_value()` is called
with one of these unsupported types, it returns `None`. This triggers the `assert
type_value is not None` statement, raising an `AssertionError` and crashing the API
endpoint responsible for serializing metric alert rules. This will prevent users with
alert rules configured with these action types from viewing or managing them.

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

@kcons kcons Jun 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok, valid point.

target = attrs.get("target")

target_type = obj.config.get("target_type")
target_identifier = obj.config.get("target_identifier")
target_display = obj.config.get("target_display")
target_type: int = obj.config.get("target_type")
target_identifier: str | None = obj.config.get("target_identifier")
target_display: str | None = obj.config.get("target_display")

sentry_app_id = None
sentry_app_config = None
Expand All @@ -85,11 +86,9 @@ def serialize(
"dateCreated": obj.date_added,
"desc": human_desc(
type_value,
target_type,
target_identifier,
target,
target_display,
target_identifier,
priority,
),
"priority": priority,
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Expand Down
22 changes: 20 additions & 2 deletions tests/sentry/incidents/endpoints/serializers/test_alert_rule_trigger_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_simple(self) -> None:
)
result = serialize(action)
self.assert_action_serialized(action, result)
assert result["desc"] == action.target_display
assert result["desc"] == "Send an email to [removed]"

@responses.activate
def test_discord(self) -> None:
Expand Down Expand Up @@ -135,7 +135,25 @@ def test_pagerduty_priority(self, mock_get: MagicMock) -> None:
result = serialize(action)
self.assert_action_serialized(action, result)
assert result["priority"] == priority
assert result["desc"] == "Send a critical level PagerDuty notification to test"

@patch(
"sentry.incidents.logic.get_target_identifier_display_for_integration",
return_value=AlertTarget("123", "test"),
)
def test_pagerduty_no_priority(self, mock_get: MagicMock) -> None:
alert_rule = self.create_alert_rule()
trigger = create_alert_rule_trigger(alert_rule, "hi", 1000)
action = create_alert_rule_trigger_action(
trigger,
AlertRuleTriggerAction.Type.PAGERDUTY,
AlertRuleTriggerAction.TargetType.SPECIFIC,
target_identifier="123",
)
result = serialize(action)
self.assert_action_serialized(action, result)
assert result["priority"] is None
assert "None" not in result["desc"]
assert result["desc"] == "Send a PagerDuty notification to test"

@responses.activate
@patch(
Expand Down
Loading