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
10 changes: 9 additions & 1 deletion src/sentry/testutils/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ def create_project_rule(
action_match="all",
filter_match="all",
frequency=30,
include_legacy_rule_id=True,
include_workflow_id=True,
**kwargs,
):
actions = None
Expand Down Expand Up @@ -662,7 +664,13 @@ def create_project_rule(
**kwargs,
)
# dual write the rule to the workflow engine
IssueAlertMigrator(rule).run()
workflow = IssueAlertMigrator(rule).run()

# annotate the rule with legacy_rule_id and workflow_id
if include_legacy_rule_id:
rule.data["actions"][0]["legacy_rule_id"] = rule.id
if include_workflow_id:
rule.data["actions"][0]["workflow_id"] = workflow.id

return rule

Expand Down
55 changes: 13 additions & 42 deletions tests/sentry/api/endpoints/test_project_rule_actions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from unittest import mock

import pytest
import sentry_sdk

from sentry.integrations.jira.integration import JiraIntegration
Expand All @@ -22,7 +23,7 @@
from sentry.shared_integrations.exceptions import IntegrationFormError
from sentry.silo.base import SiloMode
from sentry.testutils.cases import APITestCase
from sentry.testutils.helpers import with_feature
from sentry.testutils.helpers.options import override_options
from sentry.testutils.silo import assume_test_silo_mode
from sentry.testutils.skips import requires_snuba
from tests.sentry.workflow_engine.test_base import BaseWorkflowTest
Expand Down Expand Up @@ -155,7 +156,6 @@ def test_no_events(self) -> None:
assert response.status_code == 400


@with_feature("organizations:workflow-engine-single-process-workflows")
class ProjectRuleActionsEndpointWorkflowEngineTest(APITestCase, BaseWorkflowTest):
endpoint = "sentry-api-0-project-rule-actions"
method = "POST"
Expand All @@ -165,6 +165,15 @@ def setUp(self) -> None:
self.login_as(self.user)
self.workflow = self.create_workflow()

@pytest.fixture(autouse=True)
def with_feature_flags(self):
with override_options(
{
"workflow_engine.issue_alert.group.type_id.ga": [1],
}
):
yield

def setup_pd_service(self) -> PagerDutyServiceDict:
service_info = {
"type": "service",
Expand Down Expand Up @@ -226,7 +235,7 @@ def test_name_action_default(
self, mock_get_issue_alert_handler, mock_get_group_type_handler, mock_send_trigger
):
"""
Tests that label will be used as 'Test Alert' if not present. Uses PagerDuty since those
Tests that label will be used as 'Error Monitor' if not present. Uses PagerDuty since those
notifications will differ based on the name of the alert.
"""
service_info = self.setup_pd_service()
Expand All @@ -242,41 +251,7 @@ def test_name_action_default(

assert mock_send_trigger.call_count == 1
pagerduty_data = mock_send_trigger.call_args.kwargs.get("data")
assert pagerduty_data["payload"]["summary"].startswith("[Test Alert]:")

@mock.patch.object(PagerDutyClient, "send_trigger")
@mock.patch(
"sentry.notifications.notification_action.registry.group_type_notification_registry.get",
return_value=IssueAlertRegistryHandler,
)
@mock.patch(
"sentry.notifications.notification_action.registry.issue_alert_handler_registry.get",
return_value=PagerDutyIssueAlertHandler,
)
def test_name_action_with_custom_name(
Copy link
Member

Choose a reason for hiding this comment

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

Do we not have custom names in workflow engine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error detector is always named Error Monitor... we can change this though

self, mock_get_issue_alert_handler, mock_get_group_type_handler, mock_send_trigger
):
"""
Tests that custom names can be provided to the test notification. Uses PagerDuty since those
notifications will differ based on the name of the alert.
"""
service_info = self.setup_pd_service()
action_data = [
{
"account": service_info["integration_id"],
"service": service_info["id"],
"severity": "info",
"id": "sentry.integrations.pagerduty.notify_action.PagerDutyNotifyServiceAction",
}
]
custom_alert_name = "Check #feed-issues"

self.get_success_response(
self.organization.slug, self.project.slug, actions=action_data, name=custom_alert_name
)
assert mock_send_trigger.call_count == 1
pagerduty_data = mock_send_trigger.call_args.kwargs.get("data")
assert pagerduty_data["payload"]["summary"].startswith(f"[{custom_alert_name}]:")
assert pagerduty_data["payload"]["summary"].startswith("[Error Monitor]:")

@mock.patch.object(JiraIntegration, "create_issue")
@mock.patch.object(sentry_sdk, "capture_exception")
Expand Down Expand Up @@ -343,9 +318,6 @@ def test_email_action(

assert mock_notify.call_count == 1

@mock.patch(
"sentry.api.endpoints.project_rule_actions.should_fire_workflow_actions", return_value=True
)
@mock.patch(
"sentry.rules.actions.sentry_apps.utils.app_service.trigger_sentry_app_action_creators"
)
Expand All @@ -362,7 +334,6 @@ def test_sentry_app_action(
mock_get_issue_alert_handler,
mock_get_group_type_handler,
mock_trigger_sentry_app_action_creators: mock.MagicMock,
mock_should_fire_workflow_actions: mock.MagicMock,
) -> None:
self.create_detector(project=self.project)
schema = {
Expand Down
12 changes: 12 additions & 0 deletions tests/sentry/api/endpoints/test_project_rule_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,8 @@ def test_update_duplicate_rule(self) -> None:
project=self.project,
action_data=self.notify_issue_owners_action,
condition_data=conditions,
include_legacy_rule_id=False,
include_workflow_id=False,
)
conditions.append(
{
Expand All @@ -815,6 +817,8 @@ def test_update_duplicate_rule(self) -> None:
project=self.project,
action_data=self.notify_issue_owners_action,
condition_data=conditions,
include_legacy_rule_id=False,
include_workflow_id=False,
)
conditions.pop(1)
payload = {
Expand Down Expand Up @@ -842,11 +846,15 @@ def test_duplicate_rule_environment(self) -> None:
project=self.project,
action_data=self.notify_issue_owners_action,
condition_data=self.first_seen_condition,
include_legacy_rule_id=False,
include_workflow_id=False,
)
env_rule = self.create_project_rule(
project=self.project,
action_data=self.notify_issue_owners_action,
condition_data=self.first_seen_condition,
include_legacy_rule_id=False,
include_workflow_id=False,
)
payload = {
"name": "hello world",
Expand Down Expand Up @@ -887,12 +895,16 @@ def test_duplicate_rule_both_have_environments(self) -> None:
condition_data=self.first_seen_condition,
name="rule_with_env",
environment_id=self.environment.id,
include_legacy_rule_id=False,
include_workflow_id=False,
)
rule2 = self.create_project_rule(
project=self.project,
action_data=self.notify_issue_owners_action,
condition_data=self.first_seen_condition,
name="rule_wo_env",
include_legacy_rule_id=False,
include_workflow_id=False,
)
payload = {
"name": "hello world",
Expand Down
12 changes: 10 additions & 2 deletions tests/sentry/api/endpoints/test_project_rule_enable.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,19 @@ def test_duplicate_rule(self) -> None:
}
]
rule = self.create_project_rule(
project=self.project, action_data=actions, condition_data=conditions
project=self.project,
action_data=actions,
condition_data=conditions,
include_legacy_rule_id=False,
include_workflow_id=False,
)

rule2 = self.create_project_rule(
project=self.project, action_data=actions, condition_data=conditions
project=self.project,
action_data=actions,
condition_data=conditions,
include_legacy_rule_id=False,
include_workflow_id=False,
)
rule2.status = ObjectStatus.DISABLED
rule2.save()
Expand Down
5 changes: 4 additions & 1 deletion tests/sentry/digests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ def project(self) -> Project:

@cached_property
def rule(self) -> Rule:
return self.event.project.rule_set.all()[0]
rule = self.event.project.rule_set.all()[0]
rule.data["actions"][0]["legacy_rule_id"] = rule.id
rule.save()
return rule

@cached_property
def record(self) -> Record:
Expand Down
59 changes: 14 additions & 45 deletions tests/sentry/digests/test_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ def test_get_event_from_groups_in_digest(self) -> None:

def test_event_to_record_with_workflow_id(self) -> None:
project = self.create_project(fire_project_created=True)
workflow = self.create_workflow(organization=self.organization)
rule = self.create_project_rule(project, action_data=[{"workflow_id": workflow.id}])
rule = self.create_project_rule(project, include_legacy_rule_id=False)

event = self.store_event(
data={"fingerprint": ["group1"], "timestamp": before_now(minutes=2).isoformat()},
Expand All @@ -93,12 +92,11 @@ def test_event_to_record_with_workflow_id(self) -> None:
assert len(records) == 1
record = records[0]
assert record.value.identifier_key == IdentifierKey.WORKFLOW
assert record.value.rules == [workflow.id]
assert record.value.rules == [rule.data["actions"][0]["workflow_id"]]

def test_event_to_record_with_legacy_rule_id(self) -> None:
project = self.create_project(fire_project_created=True)
shadow_rule = self.create_project_rule(project)
rule = self.create_project_rule(project, action_data=[{"legacy_rule_id": shadow_rule.id}])
rule = self.create_project_rule(project, include_workflow_id=False)

event = self.store_event(
data={"fingerprint": ["group1"], "timestamp": before_now(minutes=2).isoformat()},
Expand All @@ -109,7 +107,7 @@ def test_event_to_record_with_legacy_rule_id(self) -> None:
assert len(records) == 1
record = records[0]
assert record.value.identifier_key == IdentifierKey.RULE
assert record.value.rules == [shadow_rule.id]
assert record.value.rules == [rule.data["actions"][0]["legacy_rule_id"]]


def assert_rule_ids(digest: Digest, expected_rule_ids: list[int]) -> None:
Expand Down Expand Up @@ -216,16 +214,8 @@ def setUp(self) -> None:

self.rule = self.create_project_rule()

# Represents the "original" rule during dual-write
self.shadow_rule = self.create_project_rule()
self.workflow = self.create_workflow(organization=self.organization)

self.rule_with_workflow_id = self.create_project_rule(
action_data=[{"workflow_id": self.workflow.id}]
)
self.rule_with_legacy_rule_id = self.create_project_rule(
action_data=[{"legacy_rule_id": self.shadow_rule.id}]
)
self.rule_with_workflow_id = self.create_project_rule(include_legacy_rule_id=False)
self.rule_with_legacy_rule_id = self.create_project_rule(include_workflow_id=False)

def create_events_from_filenames(
self, project: Project, filenames: Sequence[str] | None = None
Expand Down Expand Up @@ -287,7 +277,9 @@ def test_simple_with_legacy_rule_id(self) -> None:
}

assert_get_personalized_digests(self.project, digest, expected_result)
assert_rule_ids(digest, [self.shadow_rule.id])
assert_rule_ids(
digest, [self.rule_with_legacy_rule_id.data["actions"][0]["legacy_rule_id"]]
)

def test_direct_email(self) -> None:
"""When the action type is not Issue Owners, then the target actor gets a digest."""
Expand Down Expand Up @@ -326,7 +318,9 @@ def test_direct_email_with_legacy_rule_id(self) -> None:
assert_get_personalized_digests(
self.project, digest, expected_result, ActionTargetType.MEMBER, self.user1.id
)
assert_rule_ids(digest, [self.shadow_rule.id])
assert_rule_ids(
digest, [self.rule_with_legacy_rule_id.data["actions"][0]["legacy_rule_id"]]
)

def test_team_without_members(self) -> None:
team = self.create_team()
Expand Down Expand Up @@ -374,11 +368,7 @@ def test_team_without_members_with_workflow_id(self) -> None:
def test_team_without_members_with_legacy_rule_id(self) -> None:
team = self.create_team()
project = self.create_project(teams=[team], fire_project_created=True)
self.shadow_rule.project_id = project.id
self.shadow_rule.save()
rule = self.create_project_rule(
project, action_data=[{"legacy_rule_id": self.shadow_rule.id}]
)
rule = self.create_project_rule(project, include_workflow_id=False)
ProjectOwnership.objects.create(
project_id=project.id,
schema=dump_schema([Rule(Matcher("path", "*.cpp"), [Owner("team", team.slug)])]),
Expand All @@ -397,7 +387,7 @@ def test_team_without_members_with_legacy_rule_id(self) -> None:
assert not {
actor for actors in participants_by_provider_by_event.values() for actor in actors
} # no users in this team no digests should be processed
assert_rule_ids(digest, [self.shadow_rule.id])
assert_rule_ids(digest, [rule.data["actions"][0]["legacy_rule_id"]])

def test_only_everyone(self) -> None:
events = self.create_events_from_filenames(
Expand Down Expand Up @@ -465,26 +455,5 @@ def test_everyone_with_owners_with_legacy_rule_id(self) -> None:
}
assert_get_personalized_digests(self.project, digest, expected_result)

def test_simple_with_workflow_id_flag_off_fallback(self) -> None:
"""
Test that when workflow_ids are present but the feature flag is off,
it falls back to using the linked AlertRule ID via AlertRuleWorkflow.
"""
self.create_alert_rule_workflow(workflow=self.workflow, rule_id=self.shadow_rule.id)

records = []
for event in self.team1_events + self.team2_events + self.user4_events:
records.extend(_get_records(self.project, (self.rule_with_workflow_id,), event))
digest = build_digest(self.project, sort_records(records))[0]

assert_rule_ids(digest, [self.shadow_rule.id])

expected_result = {
self.user2.id: set(self.team2_events),
self.user3.id: set(self.team1_events + self.team2_events),
self.user4.id: set(self.user4_events),
}
assert_get_personalized_digests(self.project, digest, expected_result)

def test_empty_records(self) -> None:
assert build_digest(self.project, []) == DigestInfo({}, {}, {})
7 changes: 4 additions & 3 deletions tests/sentry/integrations/opsgenie/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def test_send_notification_with_workflow_engine_trigger_actions(
"details": {
"Project Name": self.project.name,
"Triggering Rules": rule.label,
"Triggering Rule URLs": f"http://example.com/organizations/baz/alerts/rules/{self.project.slug}/{123}/details/",
"Triggering Rule URLs": f"http://example.com/organizations/baz/alerts/rules/{self.project.slug}/{rule.data['actions'][0]['legacy_rule_id']}/details/",
"Sentry Group": "Hello world",
"Sentry ID": group_id,
"Logger": "",
Expand Down Expand Up @@ -210,7 +210,8 @@ def test_send_notification_with_workflow_engine_ui_links(self, mock_record: Magi
{
"workflow_id": "123",
}
]
],
include_legacy_rule_id=False,
)
client: OpsgenieClient = self.installation.get_keyring_client("team-123")
with self.options({"system.url-prefix": "http://example.com"}):
Expand All @@ -234,7 +235,7 @@ def test_send_notification_with_workflow_engine_ui_links(self, mock_record: Magi
"details": {
"Project Name": self.project.name,
"Triggering Workflows": rule.label,
"Triggering Workflow URLs": f"http://example.com/organizations/{self.organization.slug}/monitors/alerts/{123}/",
"Triggering Workflow URLs": f"http://example.com/organizations/{self.organization.slug}/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/",
"Sentry Group": "Hello world",
"Sentry ID": group_id,
"Logger": "",
Expand Down
Loading
Loading