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.

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

Allow creating filters from fields with null values in discover #70936

Merged
merged 11 commits into from Jul 9, 2020

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Jul 7, 2020

Summary

In discover, allow creating filters from fields with null \ undefined values.

The created filter is a exists filter.

  • For an inclusive filter - it creates a does not exist - hence including only items where the field is null or undefined.
  • For an exclude filter - it creates an exists filter - hence excluding items where the field is null or undefined.

This fix also includes fixing an issue with flattenHits where empty array values are inserted into the field incorrectly.

Functional test added as well.

Dev docs

Bug fix for #7189: Allow creating filters from fields with null \ undefined values.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@lizozom lizozom added bug Fixes for quality problems that affect the customer experience Feature:Filters release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v8.0.0 Team:AppArch v7.9.0 labels Jul 7, 2020
@lizozom lizozom requested a review from a team as a code owner July 7, 2020 10:32
@lizozom lizozom self-assigned this Jul 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@lizozom lizozom changed the title Fix bug #7189 Allow creating filters from fields with null values in discover #7189 Jul 7, 2020
@lizozom lizozom changed the title Allow creating filters from fields with null values in discover #7189 Allow creating filters from fields with null values in discover Jul 7, 2020
Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

In playing around with this PR, I noticed that we aren't properly handling the case where the value is an array of multiple null values:

p4bLZXX5bi

I also noticed that for missing fields, we don't show the add inclusive/exclusive filter buttons. We should probably make the behavior the same (show the filter buttons for adding exists/not exists filters).

Liza K and others added 2 commits July 8, 2020 11:17
…ers.ts

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
@lizozom
Copy link
Contributor Author

lizozom commented Jul 8, 2020

@elasticmachine merge upstream

@lizozom lizozom requested review from a team and lukasolson July 8, 2020 12:09
@lizozom
Copy link
Contributor Author

lizozom commented Jul 8, 2020

@lukasolson resolved both issue you had suggested 🙇‍♀️

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@lizozom lizozom requested a review from kertal July 8, 2020 18:31
Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code Owner code LGTM 👍 , tested locally in Chrome, Firefox, Safari, MacOS 10.14.6. That a Filter For action for null/undefined value creates a "NOT exists" made me dizzy 💫 at the start, but it makes sense.

@lizozom lizozom merged commit 52b42a8 into elastic:master Jul 9, 2020
lizozom added a commit to lizozom/kibana that referenced this pull request Jul 9, 2020
…tic#70936)

* Fix bug elastic#7189

* typo

* Test adjustments

* wait for load complete

* Fine tune test

* Update src/plugins/data/public/query/filter_manager/lib/generate_filters.ts

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>

* Fix filtering by an array of nulls
Allow filtering by a non existing field in the doc
simplify flatten hit logic

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Jul 13, 2020
lizozom added a commit that referenced this pull request Jul 13, 2020
…#70936) (#71211)

* Allow creating filters from fields with null values in discover (#70936)

* Fix bug #7189

* typo

* Test adjustments

* wait for load complete

* Fine tune test

* Update src/plugins/data/public/query/filter_manager/lib/generate_filters.ts

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>

* Fix filtering by an array of nulls
Allow filtering by a non existing field in the doc
simplify flatten hit logic

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* improve test stability

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Filters release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants