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

Remove indexPatterns dependency from filter service #47471

Merged

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Oct 7, 2019

Summary

Part of #44377

Refactored mappers and filter service to have no dependency on indexPatterns by generating and using the filter disaplyName only in the relevant components (FilterItem, FilterView, ApplyFiltersPopover).

  • Stop passing indexPatterns into FilterManager code.
  • filter.meta.value can now either be a string or a function that receives a formatter. I wanted to rename it to displayValue, but wasn't sure if this would't require further changes.
  • Remove async from all filter mappers and related code
  • Make setFilters, addFilters and removeAll synchronous

Checklist

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

For maintainers

@lizozom lizozom self-assigned this Oct 7, 2019
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom lizozom force-pushed the newplatform/filters/remove-index-patterns-2 branch from a2fef81 to a3bb287 Compare October 8, 2019 07:50
@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom lizozom marked this pull request as ready for review October 8, 2019 09:36
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lizozom
Copy link
Contributor Author

lizozom commented Oct 10, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom lizozom merged commit 338851f into elastic:master Oct 15, 2019
lizozom pushed a commit to lizozom/kibana that referenced this pull request Oct 15, 2019
* Get rid of addFiltersAndChangeTimeFilter

* ts fix

* remove timefilter dependency from filter manager

* code review change

* Fixed bug in tests

* changeTimeFilter

* Refactored mappers and filter service to have no dependency on indexPatterns by generating the filter disaplyName in the relevant components.

* Fix map and flatten test

* Fixed filter state manager test

* Remove async from addFIlters and setFilters

* Fixed saved objects test - removed (display)value from url

* Make removeAll sync

* defer setFilters and removeAll in dashboard controller - temp hack

* fixed translation in filter view

* update strings

* Fixed range rendering

* map range converter
lizozom pushed a commit that referenced this pull request Oct 15, 2019
* Get rid of addFiltersAndChangeTimeFilter

* ts fix

* remove timefilter dependency from filter manager

* code review change

* Fixed bug in tests

* changeTimeFilter

* Refactored mappers and filter service to have no dependency on indexPatterns by generating the filter disaplyName in the relevant components.

* Fix map and flatten test

* Fixed filter state manager test

* Remove async from addFIlters and setFilters

* Fixed saved objects test - removed (display)value from url

* Make removeAll sync

* defer setFilters and removeAll in dashboard controller - temp hack

* fixed translation in filter view

* update strings

* Fixed range rendering

* map range converter
chrisdavies added a commit to chrisdavies/kibana that referenced this pull request Oct 15, 2019
@tylersmalley
Copy link
Contributor

There are some stackoverflow issues which were causing failures in master/7.x. Looking to revert this PR here in the meantime.

@lizozom
Copy link
Contributor Author

lizozom commented Oct 15, 2019

@tylersmalley @chrisdavies
Are these the errors you're seeing?
image
If so, this screenshot is from master, 4-5 days ago and @Avinar-24 was looking into them. They started about a week or so ago.
So I am not sure they are related to whatever failures you're seeing.

chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Oct 16, 2019
* Get rid of addFiltersAndChangeTimeFilter

* ts fix

* remove timefilter dependency from filter manager

* code review change

* Fixed bug in tests

* changeTimeFilter

* Refactored mappers and filter service to have no dependency on indexPatterns by generating the filter disaplyName in the relevant components.

* Fix map and flatten test

* Fixed filter state manager test

* Remove async from addFIlters and setFilters

* Fixed saved objects test - removed (display)value from url

* Make removeAll sync

* defer setFilters and removeAll in dashboard controller - temp hack

* fixed translation in filter view

* update strings

* Fixed range rendering

* map range converter
@lizozom lizozom deleted the newplatform/filters/remove-index-patterns-2 branch November 14, 2019 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants