-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
feat(charts): modify custom api filter to include more fields #11054
Conversation
a171849
to
1ae8278
Compare
1ae8278
to
bb23780
Compare
Codecov Report
@@ Coverage Diff @@
## master #11054 +/- ##
==========================================
+ Coverage 65.79% 73.04% +7.25%
==========================================
Files 816 433 -383
Lines 38422 14236 -24186
Branches 3621 3621
==========================================
- Hits 25280 10399 -14881
+ Misses 13034 3728 -9306
- Partials 108 109 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -364,7 +364,7 @@ function ChartList(props: ChartListProps) { | |||
Header: t('Search'), | |||
id: 'slice_name', | |||
input: 'search', | |||
operator: 'name_or_description', | |||
operator: 'chart_all_text', |
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.
Looks like arg_names share a global namespace? I got a test failure on the previously added all_text
filter for saved queries. If so, we should add the the resource name to other custom filters so there's no confusion/collision.
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.
Yes, good point!
SUMMARY
similar to #11031 this PR expends the 'search' fields to include slice_name, description, viz_type, or datasource_name
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TEST PLAN
CI, unit tests
ADDITIONAL INFORMATION