chore(aci): put new ui links flag back #103241
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103241 +/- ##
============================================
+ Coverage 69.26% 80.71% +11.44%
============================================
Files 9220 9226 +6
Lines 393584 393961 +377
Branches 25094 25094
============================================
+ Hits 272604 317970 +45366
+ Misses 120532 75543 -44989
Partials 448 448 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| 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") | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
that's something different
saponifi3d
left a comment
saponifi3d
left a comment
There was a problem hiding this comment.
lgtm, i tried to match up to the other PR and didn't notice anything.
|
|
||
| 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") | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Is this not the same comment as before? Bad bot.
| 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]), |
There was a problem hiding this comment.
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
We want to take a different approach with links, so undo consolidation of the links flag with the ui rollout for now.
We want to take a different approach with links, so undo consolidation of the links flag with the ui rollout for now.