[Visualize] Remove global state in visualize #58352
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@elasticmachine merge upstream |
sulemanof
added
Feature:NP Migration
release_note:skip
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
kertal
reviewed
Feb 26, 2020
There was a problem hiding this comment.
I'm not sure if this is related to this PR, but while testing I saw that the filter entries of the query sent to ES contain duplicates, so there a 2 match_phrase entries for Zürich
another potential regression:
- select a saved search
- navigate back using browser's back button
- saved search is still selected
…l_state_visualize # Conflicts: # src/legacy/core_plugins/kibana/public/visualize/legacy_imports.ts # src/legacy/core_plugins/kibana/public/visualize/np_ready/application.ts
…anof/kibana into shim/remove_global_state_visualize
|
Hey @kertal ! That's about filter duplicates in es search, seems it's an old issue which reproduces in earlier versions:
If you don't mind, I'll try to take this issue in a separate PR. |
flash1293
reviewed
Feb 27, 2020
| @@ -334,6 +332,24 @@ function VisualizeAppController( | |||
| } | |||
| ); | |||
|
|
|||
| const updateSavedQueryFromUrl = savedQueryId => { | |||
There was a problem hiding this comment.
This function is only setting savedQuery, but it's not triggering the updateFromSavedQuery transition (together with syncing the data plugin state) even though it looks like it should.
In 7.6 I can change the saved query in the url and it will re-apply the filters, but here it's just selecting it in the saved object management popover.
flash1293
reviewed
Feb 27, 2020
There was a problem hiding this comment.
I could find a small bug in how global/local filters are handled - seems like they get mixed. Global filters are added to the saved object as local filters:
- Create new visualization
- Add filter
- Pin it
- Save visualization
- Go to listing page
- Open visualization again
- Pinned filter is added as local filter
2 tasks
…l_state_visualize # Conflicts: # src/legacy/core_plugins/kibana/public/visualize/legacy_imports.ts
…l_state_visualize
|
Hey @flash1293 @kertal @Dosant Besides that, I didn't found any other inconsistency. |
|
Great work @sulemanof ! I will have another look shortly, we can also work on the saved query topic in a separate PR to get this change in. |
sulemanof
commented
Mar 4, 2020
src/legacy/core_plugins/kibana/public/visualize/np_ready/listing/visualize_listing.js
Show resolved
Hide resolved
lizozom
reviewed
Mar 4, 2020
src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js
Show resolved
Hide resolved
lizozom
approved these changes
Mar 5, 2020
| const { | ||
| timefilter: { timefilter }, | ||
| } = query; | ||
|
|
There was a problem hiding this comment.
These two lines do nothing
You can remove them
There was a problem hiding this comment.
Usage of the timefilter is not visible in current diff,
but it's used in previous code
kertal
approved these changes
Mar 6, 2020
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / X-Pack Spaces API Integration Tests -- security_and_spaces.x-pack/test/spaces_api_integration/security_and_spaces/apis/copy_to_space·ts.spaces api with security copy to spaces dual-privileges readonly user from the space_1 space "before each" hook for "should return 200 when copying to non-existent space"Standard OutStack TraceKibana Pipeline / kibana-xpack-agent / X-Pack Spaces API Integration Tests -- security_and_spaces.x-pack/test/spaces_api_integration/security_and_spaces/apis/copy_to_space·ts.spaces api with security copy to spaces dual-privileges readonly user from the space_1 space "after each" hook for "should return 200 when copying to non-existent space"Standard OutStack TraceKibana Pipeline / kibana-xpack-agent / X-Pack Spaces API Integration Tests -- security_and_spaces.x-pack/test/spaces_api_integration/security_and_spaces/apis/copy_to_space·ts.spaces api with security copy to spaces dual-privileges readonly user from the space_1 space "before each" hook for "should return 200 when copying to non-existent space"Standard OutStack TraceHistory
To update your PR or re-run it, just comment with: |
flash1293
approved these changes
Mar 9, 2020
sulemanof
added a commit
to sulemanof/kibana
that referenced
this pull request
Mar 9, 2020
* Remove global state in visualize * Fix saved query * Update saved query handling * Resolve merge conflicts * Use new state syncing helpers * Fix state behavior * Prevent loosing the global state * Update state syncing with url Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
sulemanof
added a commit
that referenced
this pull request
Mar 9, 2020
* Remove global state in visualize * Fix saved query * Update saved query handling * Resolve merge conflicts * Use new state syncing helpers * Fix state behavior * Prevent loosing the global state * Update state syncing with url Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Mar 10, 2020
* master: (22 commits) Generate docs from data plugin (elastic#56955) Fixes elastic#59513 by hiding one of the symmetric edges rather than omiting it (elastic#59514) Harden creation of child processes (elastic#55697) [Alerting] replace watcher http APIs used by index threshold Alerting (elastic#59475) [Maps][docs] add more details to Quantitative data driven styling docs (elastic#59553) chore: 🤖 hide Drilldowns in master (elastic#59698) [Discover] Migrate AppState/GlobalState to new app state helpers (elastic#57175) Use HTTP request schemas to create types, use those types in the client (elastic#59340) [Maps] Support categorical styling for numbers and dates (elastic#57908) [ML] Functional API tests - bucket span estimation with custom search.max_buckets (elastic#59665) Fix slm_ui setting by changing camel case back to snake case. (elastic#59663) removes beta tag (elastic#59618) [DOCS] Removed spatial references (elastic#59595) fix outdated docs (elastic#58729) [ML] Fixes bucket span estimators loading of max_buckets setting (elastic#59639) [ML] Refactoring anomaly detector job types (elastic#59556) [Upgrade Assistant] Better handling of closed indices (elastic#58890) additional visualizations plugin cleanup before moving to NP (elastic#59318) In scripted fields, unable to switch the `Type` - getting a console error which says - Class constructor DecoratedFieldFormat cannot be invoked without 'new' (elastic#59285) [Visualize] Remove global state in visualize (elastic#58352) ...
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Mar 10, 2020
…s/kibana into alerting/fix-flaky-instance-test * 'alerting/fix-flaky-instance-test' of github.com:gmmorris/kibana: (176 commits) Generate docs from data plugin (elastic#56955) Fixes elastic#59513 by hiding one of the symmetric edges rather than omiting it (elastic#59514) Harden creation of child processes (elastic#55697) [Alerting] replace watcher http APIs used by index threshold Alerting (elastic#59475) [Maps][docs] add more details to Quantitative data driven styling docs (elastic#59553) chore: 🤖 hide Drilldowns in master (elastic#59698) [Discover] Migrate AppState/GlobalState to new app state helpers (elastic#57175) Use HTTP request schemas to create types, use those types in the client (elastic#59340) [Maps] Support categorical styling for numbers and dates (elastic#57908) [ML] Functional API tests - bucket span estimation with custom search.max_buckets (elastic#59665) Fix slm_ui setting by changing camel case back to snake case. (elastic#59663) removes beta tag (elastic#59618) [DOCS] Removed spatial references (elastic#59595) fix outdated docs (elastic#58729) [ML] Fixes bucket span estimators loading of max_buckets setting (elastic#59639) [ML] Refactoring anomaly detector job types (elastic#59556) [Upgrade Assistant] Better handling of closed indices (elastic#58890) additional visualizations plugin cleanup before moving to NP (elastic#59318) In scripted fields, unable to switch the `Type` - getting a console error which says - Class constructor DecoratedFieldFormat cannot be invoked without 'new' (elastic#59285) [Visualize] Remove global state in visualize (elastic#58352) ...
jkelastic
pushed a commit
to jkelastic/kibana
that referenced
this pull request
Mar 12, 2020
* Remove global state in visualize * Fix saved query * Update saved query handling * Resolve merge conflicts * Use new state syncing helpers * Fix state behavior * Prevent loosing the global state * Update state syncing with url 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Remove usage of Global State in Visualize, use
syncAppFilters&syncQueryinstead.Fixes #57811
Checklist
Delete any items that are not applicable to this PR.
For maintainers