Conversation
There was a problem hiding this comment.
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.
| self.assertDoesNotPass( | ||
| self.get_rule(data={"value": GroupCategory.ERROR.value, "include": "false"}), | ||
| event, | ||
| ) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 😅
ceorourke
left a comment
There was a problem hiding this comment.
Nice to have this functionality
…ude-exclude-in-issue-alerts
| except (TypeError, ValueError): | ||
| return False | ||
|
|
||
| include_category = self.get_option("include", "true") != "false" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
There are no historical True/False values that we need to worry about
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.