Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix bug in Filters aggregation and Terms aggregation with Other bucket #40698

Merged
merged 1 commit into from
Jul 10, 2019
Merged

Fix bug in Filters aggregation and Terms aggregation with Other bucket #40698

merged 1 commit into from
Jul 10, 2019

Conversation

wylieconlon
Copy link
Contributor

@wylieconlon wylieconlon commented Jul 9, 2019 •
edited
Loading

Summary

The bug is reproduced when you run a Filters aggregation with an empty string in the filter bar, and results in a user-visible toast saying "Can't convert undefined to object". It also threw a console error saying malformed filter agg params, missing "input" query:

Screenshot 2019-07-09 18 05 45

The bug is that the empty string was causing the query DSL not to be generated for the default Filters aggregation. This is related to the Terms aggregation with Other bucket, which uses a Filters aggregation to calculate how many documents are not included in the top terms.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@wylieconlon wylieconlon added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:KQL KQL Feature:Vis Editor Visualization editor issues labels Jul 9, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

wylieconlon
wylieconlon commented Jul 9, 2019
View reviewed changes
src/legacy/ui/public/agg_types/__tests__/buckets/_terms_other_bucket_helper.js
@@ -165,20 +165,16 @@ describe('Terms Agg Other bucket helper', () => {
filters: {
filters: {
'': {
'bool': {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just reformatting

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Bargs
Bargs approved these changes Jul 10, 2019
View reviewed changes
Copy link
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

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

LGTM

@wylieconlon wylieconlon merged commit 95049f3 into elastic:master Jul 10, 2019
@wylieconlon wylieconlon deleted the fix-terms-other-bucket branch July 10, 2019 15:58
@wylieconlon wylieconlon mentioned this pull request Jul 10, 2019
Merged
TinaHeiligers
TinaHeiligers reviewed Jul 10, 2019
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I was too slow!

wylieconlon pushed a commit that referenced this pull request Jul 10, 2019
Fix usage of Filters aggregation by Terms' Other bucket (#40698) (#40778
1a6ff16 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TinaHeiligers TinaHeiligers TinaHeiligers left review comments

@Bargs Bargs Bargs approved these changes

Assignees
No one assigned
Labels
Feature:KQL KQL Feature:Vis Editor Visualization editor issues release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.2.1 v7.3.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wylieconlon @elasticmachine @Bargs @TinaHeiligers