Skip to content

feat(aci): Support issue category include/exclude in issue alerts#107966

Merged
malwilley merged 7 commits intomasterfrom
malwilley/feat/support-issue-category-include-exclude-in-issue-alerts
Feb 12, 2026
Merged

feat(aci): Support issue category include/exclude in issue alerts#107966
malwilley merged 7 commits intomasterfrom
malwilley/feat/support-issue-category-include-exclude-in-issue-alerts

Conversation

@malwilley
Copy link
Copy Markdown
Member

@malwilley malwilley commented Feb 10, 2026

Ref ISWF-1991

Updates the old UI, which requires changing the legacy condition schema. Unfortunately, there isn't a good way to display a choices field with non-string values, so I used "true"/"false". This means that the compatibility layer needed to be updated to handle that instead of booleans that I assumed were going to be used.

CleanShot 2026-02-10 at 11 25 44@2x

@malwilley malwilley requested a review from a team as a code owner February 10, 2026 19:28
@linear
Copy link
Copy Markdown

linear bot commented Feb 10, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 10, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@malwilley malwilley marked this pull request as ready for review February 11, 2026 17:45
@malwilley malwilley requested a review from a team as a code owner February 11, 2026 17:45
Comment on lines +53 to +56
self.assertDoesNotPass(
self.get_rule(data={"value": GroupCategory.ERROR.value, "include": "false"}),
event,
)
Copy link
Copy Markdown
Member

@ceorourke ceorourke Feb 11, 2026

Choose a reason for hiding this comment

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

Why does this one not pass? Can you not exclude error issues?

Oh wait does "passes" here mean the rule would fire? I think I see

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.

The event being tested is an error event, so this is saying that a filter which excludes errors (include: false) will fail with an error event

lots of double negatives making this confusing to read 😅

Copy link
Copy Markdown
Member

@ceorourke ceorourke left a comment

Choose a reason for hiding this comment

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

Nice to have this functionality

except (TypeError, ValueError):
return False

include_category = self.get_option("include", "true") != "false"
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 comparison != "false" doesn't account for historical boolean False values for the include option, causing rules configured to exclude categories to incorrectly include them.
Severity: HIGH

Suggested Fix

Normalize the value from get_option to a string before the comparison to ensure both boolean and string values are handled correctly. For example: str(self.get_option("include", "true")).lower() != "false".

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/rules/filters/issue_category.py#L44

Potential issue: The code at `src/sentry/rules/filters/issue_category.py:44` determines
if an issue category should be included based on the `include` option. The line
`self.get_option("include", "true") != "false"` performs a strict string comparison.
However, the `get_option` method can return a boolean value from historical rule data
stored in `Rule.data`. If an existing rule has `include` set to the boolean `False`, the
expression `False != "false"` evaluates to `True`. This incorrectly inverts the rule's
logic, causing categories that should be excluded to be included, leading to unwanted
alerts.

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.

There are no historical True/False values that we need to worry about

@malwilley malwilley merged commit cdb34f0 into master Feb 12, 2026
75 checks passed
@malwilley malwilley deleted the malwilley/feat/support-issue-category-include-exclude-in-issue-alerts branch February 12, 2026 20:47
@github-actions github-actions bot locked and limited conversation to collaborators Feb 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants