-
Notifications
You must be signed in to change notification settings - Fork 14k
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
fix(database): allow filtering by UUID #26469
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26469 +/- ##
==========================================
+ Coverage 67.04% 69.16% +2.12%
==========================================
Files 1948 1948
Lines 76062 76062
Branches 8493 8493
==========================================
+ Hits 50995 52609 +1614
+ Misses 22887 21273 -1614
Partials 2180 2180
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Tagging the PR that caused the regression: #25961 For future reference, it's probably a good idea to always explicitly specify all searchable columns when creating new API classes to avoid regressions like this. |
(cherry picked from commit e36c014)
SUMMARY
I found a regression where we can longer filter databases by the UUID. While this functionality is not used by Superset, the Preset CLI relies on it (and is used not just with Preset workspaces, but with standalone Superset deployments for multiple reasons).
This PR brings back the functionality, and adds a unit test to prevent future breakage.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Run something like:
http://localhost:8089/api/v1/database/?q=(filters:!((col:uuid,opr:eq,value:%276ae566ae36f64a19a210621253414d03%27),(col:changed_by,opr:rel_o_m,value:1)),order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)
And verify that it works.
ADDITIONAL INFORMATION