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
8 changes: 2 additions & 6 deletions src/sentry/notifications/notification_action/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,29 +114,25 @@ def execute_via_metric_alert_handler(invocation: ActionInvocation) -> None:

def issue_notification_data_factory(invocation: ActionInvocation) -> IssueNotificationData:
from sentry.notifications.notification_action.types import BaseIssueAlertHandler
from sentry.workflow_engine.typings.notification_action import SlackDataBlob

action = invocation.action
detector = invocation.detector
event_data = invocation.event_data

blob = SlackDataBlob(**action.data)
tags_set = {t.strip() for t in blob.tags.split(",") if t.strip()} if blob.tags else set()

rule_instance = BaseIssueAlertHandler.create_rule_instance_from_action(
action=action,
detector=detector,
event_data=event_data,
)
rule_instance.data["tags"] = action.data.get("tags", "")
rule_instance.data["notes"] = action.data.get("notes", "")
rule = SerializableRuleProxy.from_rule(rule_instance)

event_id = getattr(event_data.event, "event_id", None) if event_data.event else None

return IssueNotificationData(
event_id=event_id,
group_id=event_data.group.id,
tags=tags_set,
notes=blob.notes,
notification_uuid=invocation.notification_uuid,
rule=rule,
)
9 changes: 6 additions & 3 deletions src/sentry/notifications/platform/slack/renderers/issue.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from sentry import eventstore
from sentry.models.group import Group
from sentry.notifications.platform.renderer import NotificationRenderer
from sentry.notifications.platform.slack.provider import SlackRenderable
Expand All @@ -26,14 +27,16 @@ def render[DataT: NotificationData](
group = Group.objects.get_from_cache(id=data.group_id)
Comment thread
sentry-warden[bot] marked this conversation as resolved.
event = None
if data.event_id:
event = group.get_latest_event()
event = eventstore.backend.get_event_by_id(group.project.id, data.event_id)

tags = data.rule.data.get("tags", None) or None
blocks_dict = SlackIssuesMessageBuilder(
group=group,
Comment thread
sentry[bot] marked this conversation as resolved.
event=event,
tags=data.tags or None,
tags=set(tag.strip() for tag in tags.split(",")) if tags else None,
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.

Tag parsing drops empty-string filter from set comprehension

Low Severity

The old tag parsing in the factory used {t.strip() for t in blob.tags.split(",") if t.strip()}, which filtered out empty/whitespace-only entries. The new parsing set(tag.strip() for tag in tags.split(",")) omits the if tag.strip() guard. For inputs with trailing commas or double commas (e.g. "environment,,level" or "environment,"), the resulting set will include an empty string "", which is a minor behavioral regression from the prior code.

Fix in Cursor Fix in Web

rules=[data.rule.to_rule()] if data.rule else None,
notes=data.notes or None,
notes=data.rule.data.get("notes", None) or None,
Comment thread
cursor[bot] marked this conversation as resolved.
link_to_event=True,
).build(notification_uuid=data.notification_uuid)

return SlackRenderable(
Expand Down
17 changes: 10 additions & 7 deletions src/sentry/notifications/platform/templates/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from typing import Any

from pydantic import BaseModel, ConfigDict, Field
from pydantic import BaseModel, ConfigDict

from sentry.models.rule import Rule
from sentry.notifications.platform.registry import template_registry
Expand Down Expand Up @@ -61,9 +61,7 @@ class IssueNotificationData(NotificationData):

group_id: int
event_id: str | None = None
rule: SerializableRuleProxy | None = None
tags: set[str] = Field(default_factory=set)
Comment thread
cursor[bot] marked this conversation as resolved.
notes: str = ""
rule: SerializableRuleProxy
notification_uuid: str = ""
Comment thread
sentry[bot] marked this conversation as resolved.


Expand All @@ -73,11 +71,16 @@ class IssueNotificationTemplate(NotificationTemplate[IssueNotificationData]):
example_data = IssueNotificationData(
group_id=1,
event_id="abc123",
tags={"environment", "level"},
notes="example note",
notification_uuid="test-uuid",
rule=SerializableRuleProxy(
id=1, project_id=2, environment_id=3, label="Example Rule", data={}
id=1,
project_id=2,
label="Example Rule",
data={
"actions": [{"workflow_id": 3}],
"tags": "environment,level",
"notes": "example note",
},
),
)
hide_from_debugger = True
Expand Down
63 changes: 51 additions & 12 deletions tests/sentry/notifications/platform/slack/renderers/test_issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ def _create_invocation(

class IssueNotificationDataTest(IssueAlertInvocationMixin):
def test_source(self) -> None:
data = IssueNotificationData(group_id=self.group.id)
data = IssueNotificationData(
group_id=self.group.id,
rule=SerializableRuleProxy(
id=1, label="Test Detector", data={}, project_id=self.project.id
),
)
assert data.source == NotificationSource.ISSUE

def test_from_action_invocation(self) -> None:
Expand All @@ -91,14 +96,22 @@ def test_from_action_invocation(self) -> None:
assert result.group_id == invocation.event_data.group.id
assert isinstance(invocation.event_data.event, GroupEvent)
assert result.event_id == invocation.event_data.event.event_id
assert result.tags == {"environment", "level"}
assert result.notes == "test note"
assert result.notification_uuid == "test-uuid-123"

assert isinstance(result.rule, SerializableRuleProxy)
assert result.rule.id == invocation.action.id
assert result.rule.label == "Test Detector"
assert result.rule.environment_id is None
assert result.rule.label == invocation.detector.name
assert result.rule.data["tags"] == "environment,level"
assert result.rule.data["notes"] == "test note"
assert len(result.rule.data["actions"]) == 1
action_blob = result.rule.data["actions"][0]
assert action_blob["workflow_id"] == getattr(invocation.action, "workflow_id", None)
assert (
action_blob["id"] == "sentry.integrations.slack.notify_action.SlackNotifyServiceAction"
)
assert action_blob["workspace"] == invocation.action.integration_id
assert action_blob["channel_id"] == "C12345"
assert action_blob["channel"] == "#test-channel"
assert result.rule.project_id == self.project.id

def test_from_action_invocation_empty_tags(self) -> None:
invocation = self._create_invocation(tags="", notes="")
Expand All @@ -108,9 +121,17 @@ def test_from_action_invocation_empty_tags(self) -> None:
assert result.source == NotificationSource.ISSUE
assert isinstance(invocation.event_data.event, GroupEvent)
assert result.event_id == invocation.event_data.event.event_id
assert result.tags == set()
assert result.notes == ""
assert result.rule is not None
assert result.rule.data["tags"] == ""
assert result.rule.data["notes"] == ""
assert len(result.rule.data["actions"]) == 1
action_blob = result.rule.data["actions"][0]
assert action_blob["workflow_id"] == getattr(invocation.action, "workflow_id", None)
assert (
action_blob["id"] == "sentry.integrations.slack.notify_action.SlackNotifyServiceAction"
)
assert action_blob["workspace"] == invocation.action.integration_id
assert action_blob["channel_id"] == "C12345"
assert action_blob["channel"] == "#test-channel"


class IssueAlertNotificationTemplateTest(TestCase):
Expand Down Expand Up @@ -142,6 +163,7 @@ def _build_expected_blocks(
*,
group: Group,
workflow_id: int,
event_id: str,
title: str = "test event",
notes: str | None = None,
tags_text: str | None = None,
Expand All @@ -156,7 +178,8 @@ def _build_expected_blocks(

issue_url = (
f"http://testserver/organizations/{org_slug}/issues/{group_id}/"
f"?referrer=slack&notification_uuid=test-uuid"
f"events/{event_id}/"
f"?referrer=slack"
f"&workflow_id={workflow_id}&alert_type=issue"
)
alert_url = f"http://testserver/organizations/{org_slug}/monitors/alerts/{workflow_id}/"
Expand Down Expand Up @@ -262,9 +285,11 @@ def test_render_produces_blocks(self, mock_summary: Any) -> None:
rendered_template=rendered_template,
)

assert isinstance(invocation.event_data.event, GroupEvent)
assert result == self._build_expected_blocks(
group=invocation.event_data.group,
workflow_id=getattr(invocation.action, "workflow_id"),
event_id=invocation.event_data.event.event_id,
)

@patch(
Expand All @@ -281,9 +306,11 @@ def test_render_with_notes(self, mock_summary: Any) -> None:
rendered_template=rendered_template,
)

assert isinstance(invocation.event_data.event, GroupEvent)
assert result == self._build_expected_blocks(
group=invocation.event_data.group,
workflow_id=getattr(invocation.action, "workflow_id"),
event_id=invocation.event_data.event.event_id,
notes="important note",
)

Expand All @@ -304,25 +331,37 @@ def test_render_with_tags(self, mock_summary: Any) -> None:
rendered_template=rendered_template,
)

assert isinstance(invocation.event_data.event, GroupEvent)
assert result == self._build_expected_blocks(
group=invocation.event_data.group,
workflow_id=getattr(invocation.action, "workflow_id"),
event_id=invocation.event_data.event.event_id,
title="tagged event",
tags_text="level: `error` ",
)


class IssueAlertProviderDispatchTest(TestCase):
def test_provider_returns_issue_renderer(self) -> None:
data = IssueNotificationData(group_id=self.group.id)
data = IssueNotificationData(
group_id=self.group.id,
rule=SerializableRuleProxy(
id=1, label="Test Detector", data={}, project_id=self.project.id
),
)
renderer = SlackNotificationProvider.get_renderer(
data=data,
category=NotificationCategory.ISSUE,
)
assert renderer is IssueSlackRenderer

def test_provider_returns_default_for_unknown_category(self) -> None:
data = IssueNotificationData(group_id=self.group.id)
data = IssueNotificationData(
group_id=self.group.id,
rule=SerializableRuleProxy(
id=1, label="Test Detector", data={}, project_id=self.project.id
),
)
renderer = SlackNotificationProvider.get_renderer(
data=data,
category=NotificationCategory.DEBUG,
Expand Down
Loading