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

Adding filter pills causes double fetch in discover and dashboards #54887

Closed
flash1293 opened this issue Jan 15, 2020 · 3 comments
Closed

Adding filter pills causes double fetch in discover and dashboards #54887

flash1293 opened this issue Jan 15, 2020 · 3 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Filters

Comments

@flash1293
Copy link
Contributor

flash1293 commented Jan 15, 2020

Kibana version: master

Elasticsearch version:

Server OS version:

Browser version:

Browser OS version:

Original install method (e.g. download page, yum, from source, etc.):

Describe the bug: src/plugins/data/public/query/filter_manager/lib/mappers/map_phrase.ts adds a formatter function to the filter object. However the filter in the appState won't have this property. This causes the filter manager to trigger subscribers to fetch because the filter_manager checks equality by a simple deep equality check: https://github.com/elastic/kibana/blob/master/src/plugins/data/public/query/filter_manager/filter_manager.ts#L92

It looks like the same thing is also happening in dashboards.

In case of discover this causes fetch to be called two times:

  • Filters are changed in the top nav component
  • Discover calls setFilters, which causes the first fetch to be dispatched
  • New filters are stored in the global state
  • The interval checker in https://github.com/elastic/kibana/blob/master/src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts#L58 picks up the change, pulls the filters from the appState and compares them to the filters stored in the filter manager - they are different because the appState filter doesn't have the formatter function property value (maybe because it gets deserialized from the url?), but the filterManager one has.
  • This is enough difference for the filter state manager to setFilters again

Untitled drawing (1)

Steps to reproduce:

  1. Go to discover
  2. Add a filter pill on a keyword field
  3. Observe two requests being dispatched

Expected behavior:

Screenshots (if relevant):

Errors in browser console (if relevant):

Provide logs and/or server output (if relevant):

Any additional context:

@flash1293 flash1293 added bug Fixes for quality problems that affect the customer experience Team:AppArch labels Jan 15, 2020
@elasticmachine
Copy link
Contributor

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

@flash1293 flash1293 changed the title Changing filter pills causes double fetch in discover Adding filter pills causes double fetch in discover Jan 15, 2020
@flash1293 flash1293 changed the title Adding filter pills causes double fetch in discover Adding filter pills causes double fetch in discover and dashboards Jan 16, 2020
lizozom pushed a commit to lizozom/kibana that referenced this issue Jan 19, 2020
 - Filters are not only fetch once on timefilter change
 - Make sure that discover doesn't fetch data when a disabled filter is changed
 - Support compareFilters on an array of filters.
 - Added tests to compare filters
 - Exctracted sortFilters and added tests to it.
lizozom pushed a commit that referenced this issue Jan 21, 2020
* Fix bug #54887
 - Filters are not only fetch once on timefilter change
 - Make sure that discover doesn't fetch data when a disabled filter is changed
 - Support compareFilters on an array of filters.
 - Added tests to compare filters
 - Exctracted sortFilters and added tests to it.

* code review + FilterCompareOptions

* Remove sort by

Co-authored-by: Elastic Machine <[email protected]>
lizozom pushed a commit to lizozom/kibana that referenced this issue Jan 21, 2020
* Fix bug elastic#54887
 - Filters are not only fetch once on timefilter change
 - Make sure that discover doesn't fetch data when a disabled filter is changed
 - Support compareFilters on an array of filters.
 - Added tests to compare filters
 - Exctracted sortFilters and added tests to it.

* code review + FilterCompareOptions

* Remove sort by

Co-authored-by: Elastic Machine <[email protected]>
lizozom pushed a commit to lizozom/kibana that referenced this issue Jan 21, 2020
* Fix bug elastic#54887
 - Filters are not only fetch once on timefilter change
 - Make sure that discover doesn't fetch data when a disabled filter is changed
 - Support compareFilters on an array of filters.
 - Added tests to compare filters
 - Exctracted sortFilters and added tests to it.

* code review + FilterCompareOptions

* Remove sort by

Co-authored-by: Elastic Machine <[email protected]>
lizozom pushed a commit that referenced this issue Jan 21, 2020
)

* Fix bug #54887
 - Filters are not only fetch once on timefilter change
 - Make sure that discover doesn't fetch data when a disabled filter is changed
 - Support compareFilters on an array of filters.
 - Added tests to compare filters
 - Exctracted sortFilters and added tests to it.

* code review + FilterCompareOptions

* Remove sort by

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
lizozom pushed a commit that referenced this issue Jan 21, 2020
)

* Fix bug #54887
 - Filters are not only fetch once on timefilter change
 - Make sure that discover doesn't fetch data when a disabled filter is changed
 - Support compareFilters on an array of filters.
 - Added tests to compare filters
 - Exctracted sortFilters and added tests to it.

* code review + FilterCompareOptions

* Remove sort by

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@lukeelmers
Copy link
Member

@lizozom @flash1293 can this be closed?

@flash1293
Copy link
Contributor Author

Yep, can be closed.

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
Projects
None yet
Development

No branches or pull requests

4 participants