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
4 changes: 2 additions & 2 deletions src/sentry/digests/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def event_to_record(
assert event.group is not None
rule_ids = []
identifier_key = IdentifierKey.RULE
if features.has("organizations:workflow-engine-ui", event.organization):
if features.has("organizations:workflow-engine-ui-links", event.organization):
identifier_key = IdentifierKey.WORKFLOW
for rule in rules:
rule_ids.append(int(get_key_from_rule_data(rule, "workflow_id")))
Expand Down Expand Up @@ -194,7 +194,7 @@ def get_rules_from_workflows(project: Project, workflow_ids: set[int]) -> dict[i

# We are only processing the workflows in the digest if under the new flag
# This should be ok since we should only add workflow_ids to redis when under this flag
if features.has("organizations:workflow-engine-ui", project.organization):
if features.has("organizations:workflow-engine-ui-links", project.organization):
for workflow_id, workflow in workflows.items():
assert (
workflow.organization_id == project.organization_id
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/incidents/action_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ def generate_incident_trigger_email_context(
if notification_uuid:
alert_link_params["notification_uuid"] = notification_uuid

if features.has("organizations:workflow-engine-ui", organization):
if features.has("organizations:workflow-engine-ui-links", organization):
assert (
metric_issue_context.group is not None
), "Group should not be None when workflow engine ui links are enabled"
Expand Down Expand Up @@ -632,7 +632,7 @@ def generate_incident_trigger_email_context(
snooze_alert = False
# We don't have user muting for workflows in the new workflow engine system
# so we don't need to show the snooze alert url
if not features.has("organizations:workflow-engine-ui", organization):
if not features.has("organizations:workflow-engine-ui-links", organization):
snooze_alert = True
snooze_alert_url = alert_link + "&" + urlencode({"mute": "1"})

Expand Down
4 changes: 2 additions & 2 deletions src/sentry/integrations/discord/message_builder/issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def build(self, notification_uuid: str | None = None) -> DiscordMessage:
rule_environment_id = None
if self.rules:
rule_environment_id = self.rules[0].environment_id
if features.has("organizations:workflow-engine-ui", self.group.organization):
if features.has("organizations:workflow-engine-ui-links", self.group.organization):
rule_id = int(get_key_from_rule_data(self.rules[0], "workflow_id"))
elif should_fire_workflow_actions(self.group.organization, self.group.type):
rule_id = int(get_key_from_rule_data(self.rules[0], "legacy_rule_id"))
Expand All @@ -71,7 +71,7 @@ def build(self, notification_uuid: str | None = None) -> DiscordMessage:

url = None

if features.has("organizations:workflow-engine-ui", self.group.organization):
if features.has("organizations:workflow-engine-ui-links", self.group.organization):
url = get_title_link_workflow_engine_ui(
self.group,
self.event,
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/messaging/message_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ def build_footer(
) -> str:
footer = f"{group.qualified_short_id}"
if rules:
if features.has("organizations:workflow-engine-ui", group.organization):
if features.has("organizations:workflow-engine-ui-links", group.organization):
rule_url = absolute_uri(
create_link_to_workflow(
group.organization.id, get_key_from_rule_data(rules[0], "workflow_id")
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/metric_alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ def incident_attachment_info(

title_link = build_title_link(alert_rule_id, organization, workflow_engine_params)

elif features.has("organizations:workflow-engine-ui", organization):
elif features.has("organizations:workflow-engine-ui-links", organization):
if metric_issue_context.group is None:
raise ValueError("Group is required for workflow engine UI links")

Comment on lines 253 to 259

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's something different

Comment on lines 253 to 259

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this not the same comment as before? Bad bot.

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/opsgenie/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def build_issue_alert_payload(
if notification_uuid:
group_params["notification_uuid"] = notification_uuid
rule_workflow_context = {}
if features.has("organizations:workflow-engine-ui", group.project.organization):
if features.has("organizations:workflow-engine-ui-links", group.project.organization):
workflow_urls = self._get_workflow_urls(group, rules)
rule_workflow_context = {
"Triggering Workflows": ", ".join([rule.label for rule in rules]),
Comment on lines 92 to 98
Copy link

Choose a reason for hiding this comment

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

Bug: MetricIssue.build_visible_feature_name() uses an outdated feature flag, causing inconsistent visibility.
Severity: HIGH | Confidence: 0.95

🔍 Detailed Analysis

The MetricIssue.build_visible_feature_name() method incorrectly returns the old feature flag "organizations:workflow-engine-ui". This conflicts with other functional code in the pull request that has been updated to check the new feature flag "organizations:workflow-engine-ui-links" for generating workflow links. This mismatch leads to inconsistent behavior where MetricIssue group types might not be visible even if workflow links are generated, violating the intended separation of concerns.

💡 Suggested Fix

Update MetricIssue.build_visible_feature_name() in src/sentry/incidents/grouptype.py to return "organizations:workflow-engine-ui-links".

🤖 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/integrations/opsgenie/client.py#L92-L98

Potential issue: The `MetricIssue.build_visible_feature_name()` method incorrectly
returns the old feature flag `"organizations:workflow-engine-ui"`. This conflicts with
other functional code in the pull request that has been updated to check the new feature
flag `"organizations:workflow-engine-ui-links"` for generating workflow links. This
mismatch leads to inconsistent behavior where `MetricIssue` group types might not be
visible even if workflow links are generated, violating the intended separation of
concerns.

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

Reference_id: 2620863

Expand Down
4 changes: 2 additions & 2 deletions src/sentry/integrations/slack/message_builder/issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock:
rule_id = None
rule_environment_id = None
if self.rules:
if features.has("organizations:workflow-engine-ui", self.group.organization):
if features.has("organizations:workflow-engine-ui-links", self.group.organization):
rule_id = int(get_key_from_rule_data(self.rules[0], "workflow_id"))
elif should_fire_workflow_actions(self.group.organization, self.group.type):
rule_id = int(get_key_from_rule_data(self.rules[0], "legacy_rule_id"))
Expand All @@ -680,7 +680,7 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock:
has_action = True

title_link = None
if features.has("organizations:workflow-engine-ui", self.group.organization):
if features.has("organizations:workflow-engine-ui-links", self.group.organization):
title_link = get_title_link_workflow_engine_ui(
self.group,
self.event,
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/integrations/slack/message_builder/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def build_slack_footer(
footer = f"{group.qualified_short_id}"

if rules:
if features.has("organizations:workflow-engine-ui", group.organization):
if features.has("organizations:workflow-engine-ui-links", group.organization):
rule_url = absolute_uri(
create_link_to_workflow(
group.organization.id, get_key_from_rule_data(rules[0], "workflow_id")
Expand Down
6 changes: 4 additions & 2 deletions src/sentry/notifications/notification_action/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,11 @@ def create_rule_instance_from_action(
workflow_id = getattr(action, "workflow_id", None)

label = detector.name
# We need to pass the legacy rule id when the workflow-engine-ui feature flag is disabled
# We need to pass the legacy rule id when the workflow-engine-ui-links feature flag is disabled
# This is so we can build the old link to the rule
if not features.has("organizations:workflow-engine-ui", detector.project.organization):
if not features.has(
"organizations:workflow-engine-ui-links", detector.project.organization
):
if workflow_id is None:
raise ValueError("Workflow ID is required when triggering an action")

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/notifications/notifications/digest.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def get_context(self) -> MutableMapping[str, Any]:

sentry_query_params = self.get_sentry_query_params(ExternalProviders.EMAIL)

if not features.has("organizations:workflow-engine-ui", self.project.organization):
if not features.has("organizations:workflow-engine-ui-links", self.project.organization):
# TODO(iamrajjoshi): This actually mutes a rule for a user, something we have not ported over in the new system
# By not including this context, the template will not show the mute button
snooze_alert = len(rule_details) > 0
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/notifications/notifications/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def get_context(self) -> MutableMapping[str, Any]:

# We don't show the snooze alert if the organization has not enabled the workflow engine UI links
# This is because in the new UI/system a user can't individually disable a workflow
if not features.has("organizations:workflow-engine-ui", self.organization):
if not features.has("organizations:workflow-engine-ui-links", self.organization):
if len(self.rules) > 0:
context["snooze_alert"] = True
context["snooze_alert_url"] = get_snooze_url(
Expand Down Expand Up @@ -299,7 +299,7 @@ def get_notification_title(
title_str = "Alert triggered"

if self.rules:
if features.has("organizations:workflow-engine-ui", self.organization):
if features.has("organizations:workflow-engine-ui-links", self.organization):
rule_url = absolute_uri(
create_link_to_workflow(
self.organization.id, get_key_from_rule_data(self.rules[0], "workflow_id")
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/notifications/utils/links.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def get_rules(
) -> Sequence[NotificationRuleDetails]:
from sentry.notifications.notification_action.utils import should_fire_workflow_actions

if features.has("organizations:workflow-engine-ui", organization):
if features.has("organizations:workflow-engine-ui-links", organization):
return get_workflow_links(rules, organization, project)
elif type_id is None or should_fire_workflow_actions(organization, type_id):
return get_rules_with_legacy_ids(rules, organization, project)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
installation, IssueBasicIntegration
), "Installation must be an IssueBasicIntegration to create a ticket"
data["title"] = installation.get_group_title(event.group, event)
if features.has("organizations:workflow-engine-ui", organization):
if features.has("organizations:workflow-engine-ui-links", organization):
workflow_id = data.get("workflow_id")
assert workflow_id is not None
data["description"] = build_description_workflow_engine_ui(
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/sentry_apps/tasks/sentry_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ def notify_sentry_app(event: GroupEvent, futures: Sequence[RuleFuture]):
# if we are using the new workflow engine, we need to use the legacy rule id
# Ignore test notifications
if int(id) != -1:
if features.has("organizations:workflow-engine-ui", event.group.organization):
if features.has("organizations:workflow-engine-ui-links", event.group.organization):
id = get_key_from_rule_data(f.rule, "workflow_id")
elif should_fire_workflow_actions(event.group.organization, event.group.type):
id = get_key_from_rule_data(f.rule, "legacy_rule_id")
Expand Down
12 changes: 6 additions & 6 deletions tests/sentry/digests/test_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def test_get_event_from_groups_in_digest(self) -> None:
# Make sure that the target_identifier = RULE
assert {record.value.identifier_key == IdentifierKey.RULE for record in records}

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
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)
Expand Down Expand Up @@ -242,7 +242,7 @@ def test_simple(self) -> None:

assert_get_personalized_digests(self.project, digest, expected_result)

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_simple_with_workflow_id(self) -> None:
records = [
event_to_record(event, (self.rule_with_workflow_id,))
Expand Down Expand Up @@ -286,7 +286,7 @@ def test_direct_email(self) -> None:
self.project, digest, expected_result, ActionTargetType.MEMBER, self.user1.id
)

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_direct_email_with_workflow_id(self) -> None:
"""When the action type is not Issue Owners, then the target actor gets a digest. - Workflow ID"""
self.project_ownership.update(fallthrough=False)
Expand Down Expand Up @@ -339,7 +339,7 @@ def test_team_without_members(self) -> None:
actor for actors in participants_by_provider_by_event.values() for actor in actors
} # no users in this team no digests should be processed

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_team_without_members_with_workflow_id(self) -> None:
team = self.create_team()
project = self.create_project(teams=[team], fire_project_created=True)
Expand Down Expand Up @@ -422,7 +422,7 @@ def test_everyone_with_owners(self) -> None:
}
assert_get_personalized_digests(self.project, digest, expected_result)

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_everyone_with_owners_with_workflow_id(self) -> None:
events = self.create_events_from_filenames(
self.project, ["hello.moz", "goodbye.moz", "hola.moz", "adios.moz"]
Expand Down Expand Up @@ -468,7 +468,7 @@ def test_simple_with_workflow_id_flag_off_fallback(self) -> None:
"""
self.create_alert_rule_workflow(workflow=self.workflow, rule_id=self.shadow_rule.id)

with self.feature("organizations:workflow-engine-ui"):
with self.feature("organizations:workflow-engine-ui-links"):
records = [
event_to_record(event, (self.rule_with_workflow_id,))
for event in self.team1_events + self.team2_events + self.user4_events
Expand Down
2 changes: 1 addition & 1 deletion tests/sentry/integrations/opsgenie/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def test_send_notification_with_workflow_engine_trigger_actions(

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_send_notification_with_workflow_engine_ui_links(self, mock_record: MagicMock) -> None:
resp_data = {
"result": "Request will be processed",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ def test_after_noa_test_action(
assert thread_ts_success.args[0] == EventLifecycleOutcome.SUCCESS

@with_feature("organizations:workflow-engine-trigger-actions")
@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@patch("sentry.integrations.slack.sdk_client.SlackSdkClient.chat_postMessage")
@patch("slack_sdk.web.client.WebClient._perform_urllib_http_request")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def test_generic_issue_alert_user_block_workflow_engine_dual_write(
return_value=TEST_ISSUE_OCCURRENCE,
new_callable=mock.PropertyMock,
)
@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_generic_issue_alert_user_block_workflow_engine_ui_links(
self, occurrence: MagicMock
) -> None:
Expand Down
2 changes: 1 addition & 1 deletion tests/sentry/mail/test_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ def test_notify_users_does_email(self, mock_logger, mock_func) -> None:

@mock_notify
@mock.patch("sentry.notifications.notifications.rules.logger")
@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_notify_users_does_email_workflow_engine_ui_links(self, mock_logger, mock_func) -> None:
self.create_user_option(user=self.user, key="timezone", value="Europe/Vienna")
event_manager = EventManager({"message": "hello world", "level": "error"})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def test_create_rule_instance_from_action(self) -> None:
assert rule.status == ObjectStatus.ACTIVE
assert rule.source == RuleSource.ISSUE

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_create_rule_instance_from_action_with_workflow_engine_ui_feature_flag(self) -> None:
"""Test that create_rule_instance_from_action creates a Rule with correct attributes"""
rule = self.handler.create_rule_instance_from_action(
Expand Down Expand Up @@ -222,7 +222,7 @@ def test_create_rule_instance_from_action_no_environment(self) -> None:
assert rule.status == ObjectStatus.ACTIVE
assert rule.source == RuleSource.ISSUE

@with_feature("organizations:workflow-engine-ui")
@with_feature("organizations:workflow-engine-ui-links")
def test_create_rule_instance_from_action_no_environment_with_workflow_engine_ui_feature_flag(
self,
):
Expand Down
Loading