-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(search): Support IN operator for semver release.version filters #107088
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
When users select multiple release versions via checkboxes in the Issues page search bar, the frontend generates a query like `release.version:[1.2.3,1.2.4]` with an IN operator. Previously this raised "Invalid operation 'IN' for semantic version filter" because the semver filter converters didn't handle the IN operator. This change adds support for the IN operator with list values in: - `_semver_filter_converter` and `semver_filter_converter`: iterate through each version in the list, query matching releases for each, and combine results - `_semver_build_filter_converter` and `semver_build_filter_converter`: pass the list directly to `filter_by_semver_build` - `filter_by_semver_build`: accept a sequence of builds and use `build_number__in` / `build_code__in` for the query Fixes GH-76286 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this 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 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| try: | ||
| django_op = constants.OPERATOR_TO_DJANGO[operator] | ||
| except KeyError: | ||
| raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading error message hardcodes 'IN' for non-IN operators
Low Severity
The error message at this location hardcodes "Invalid operation 'IN'" but now that IN is supported (added to OPERATOR_TO_DJANGO), this KeyError can only be triggered by other invalid operators like "NOT IN", "LIKE", etc. The parse_semver function was correctly updated to use an f-string with the actual operator (f"Invalid operation '{operator}'") but this fix wasn't applied to semver_build_filter_converter or _semver_build_filter_converter. This results in confusing error messages when users use unsupported operators.
Additional Locations (1)
| if not versions: | ||
| versions = [SEMVER_EMPTY_RELEASE] | ||
|
|
||
| return ["release", "IN", versions] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing validation check for array parameter size limit
Medium Severity
The new IN operator handling in _semver_filter_converter is missing the validate_snuba_array_parameter(versions) check that exists in the parallel implementation semver_filter_converter in filter_aliases.py. When multiple versions are queried, each can return up to MAX_SEARCH_RELEASES results which get extended into the versions list. Without validation, this could pass an excessively large array to Snuba, potentially causing query failures or performance issues. The validation was added in filter_aliases.py at line 223 but omitted here.
| "NOT IN": "IN", | ||
| } | ||
| OPERATOR_TO_DJANGO = {">=": "gte", "<=": "lte", ">": "gt", "<": "lt", "=": "exact"} | ||
| OPERATOR_TO_DJANGO = {">=": "gte", "<=": "lte", ">": "gt", "<": "lt", "=": "exact", "IN": "in"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IN operator with non-list value causes crash instead of error
Low Severity
Adding "IN": "in" to OPERATOR_TO_DJANGO creates a regression for edge cases where operator == "IN" but the value is a single string rather than a list. Previously, this would raise a clean InvalidSearchQuery via the KeyError handler. Now, "IN" passes validation but the downstream code in filter_by_semver_build attempts qs.filter(build_number__in=int(single_value)), which causes a TypeError because Django's __in lookup requires an iterable, not an integer. While this edge case may not be reachable through normal UI usage, the type annotations explicitly allow this combination.
Additional Locations (1)
|
To be clear I'm not sure what a "correct" solution is here, so will need perspective from someone who has context here. Is this the general approach we would take to fix the bug? Ignoring the line by line details? |
When users select multiple release versions via checkboxes in the Issues page search bar, the frontend generates a query like
release.version:[1.2.3,1.2.4]with an IN operator. Previously this raised "Invalid operation 'IN' for semantic version filter" because the semver filter converters didn't handle the IN operator.This change adds support for the IN operator with list values in:
_semver_filter_converterandsemver_filter_converter: iterate through each version in the list, query matching releases for each, and combine results_semver_build_filter_converterandsemver_build_filter_converter: pass the list directly tofilter_by_semver_buildfilter_by_semver_build: accept a sequence of builds and usebuild_number__in/build_code__infor the queryFixes #76286