[State Management] State syncing helpers for query service I by Dosant · Pull Request #56128 · elastic/kibana

Dosant · 2020-01-28T12:11:55Z

Summary

Addresses Expose state container for global data updates from data plugin #56503
Partially addresses Implement data.query service observables to track state changes #57168
Partially address addresses [DISCUSS] State management plugin API #55977 - implements example with state containers / state sync and query services.

The main reason for this changes is new sub url tracking implemented in #57307 and #55818. It improves performance of new implementation.

Before this pr:

Discover, Visualise and Dashboard in setup phase create own state containers which watch for pinned filters, time and refresh interval changes. This watching and state comparisons happen for each plugin separately and not only when a specific app is mounted. So we ended up with a bunch of similar synchronous work which happens every time query services state changes.

After this pr:

Query service exposes observable to watch for changes (state$). Discover, Visualise and Dashboard use this observable for sub url tracking instead of creating its own.

Dev Docs

Query service of data plugin now has state$ observable which allows to watch for query service data changes:

interface QueryState {
  time?: TimeRange;
  refreshInterval?: RefreshInterval;
  filters?: Filter[];
}

interface QueryStateChange {
  time?: boolean; // time range has changed
  refreshInterval?: boolean; // refresh interval has changed
  filters?: boolean; // any filter has changed
  appFilters?: boolean; // specifies if app filters change
  globalFilters?: boolean; // specifies if global filters change
}

state$: Observable<{ changes: QueryStateChange; state: QueryState }>;

Checklist

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

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

…t-with-state-setup

…t-with-state-setup # Conflicts: # src/plugins/data/public/query/state_sync/index.ts # src/plugins/data/public/query/state_sync/sync_global_state_with_url.test.ts # src/plugins/data/public/query/state_sync/sync_query.ts # src/plugins/data/public/ui/search_bar/create_search_bar.tsx # src/plugins/data/public/ui/search_bar/search_bar.tsx

timefilter initial state

…ozom/kibana into dev/experiment-with-state-setup

elasticmachine · 2020-02-07T10:08:06Z

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

…ibana into dev/experiment-with-state-setup

…t-with-state-setup # Conflicts: # src/legacy/core_plugins/kibana/public/dashboard/np_ready/legacy_app.js # src/legacy/core_plugins/kibana/public/dashboard/plugin.ts # src/plugins/data/public/query/state_sync/connect_to_app_state.test.ts # src/plugins/data/public/query/state_sync/sync_app_filters.ts # src/plugins/data/public/query/state_sync/sync_query.ts # src/plugins/data/public/ui/search_bar/search_bar.tsx

…t-with-state-setup

lizozom · 2020-02-22T15:16:52Z

@elasticmachine merge upstream

lizozom · 2020-02-25T09:45:46Z

@elasticmachine merge upstream

lizozom

I think this is a great step forward in the right direction.
LGTM

flash1293

Tested and still works as expected - left some nits but nothing required and can also be addressed later.

flash1293 · 2020-02-26T10:54:57Z

src/legacy/core_plugins/kibana/public/visualize/plugin.ts

        {
          kbnUrlKey: '_g',
-          stateUpdate$: querySyncStateContainer.state$,
+          stateUpdate$: data.query.state$.pipe(


Seems like this is a common way of using state$ - what about moving into into data (either as a helper or as a second globalState$ observable?

Initially this pr had app$ global$ observables but we decided to go for more generic approach.
but I also don't like this duplications across kibana apps.
Maybe there is a place, where I could put this operator, which would be common for kibana apps?

If there is no good place for this helper, let's leave it as is. IMHO a little bit of redundancy is preferable over abstractions in the wrong places.

flash1293 · 2020-02-26T11:03:40Z

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx

+    const stopSyncingAppFilters = connectToQueryState(
+      queryService,
+      {
+        set: ({ filters }) => dashboardStateManager.setFilters(filters || []),


It looks like we can just pass the actual state container used by dashboardStateManager down here instead of emulating it like this and it would do the same thing - not required though

examples/state_containers_examples/public/with_data_services/components/app.tsx

Dosant · 2020-02-26T14:46:33Z

@elasticmachine merge upstream

Dosant · 2020-02-27T07:55:03Z

@elasticmachine merge upstream

kibanamachine · 2020-02-27T10:55:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: cc91afb

History

💚 Build #29365 succeeded e3513e0
💔 Build #29135 failed 5b96209
💔 Build #29109 failed 83e16c0
💚 Build #28732 succeeded fa61db9
💔 Build #28306 failed cc1aaff

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

…#56128) Before this pr: Discover, Visualise and Dashboard in setup phase create own state containers which watch for pinned filters, time and refresh interval changes. This watching and state comparisons happen for each plugin separately and not only when a specific app is mounted. So we ended up with a bunch of similar synchronous work which happens every time query services state changes. After this pr: Query service exposes observable to watch for changes (state$). Discover, Visualise and Dashboard use this observable for sub url tracking instead of creating its own.

Dosant · 2020-02-27T12:55:01Z

Planning to consider this as a more long term follow up: #58721

…#58711) Before this pr: Discover, Visualise and Dashboard in setup phase create own state containers which watch for pinned filters, time and refresh interval changes. This watching and state comparisons happen for each plugin separately and not only when a specific app is mounted. So we ended up with a bunch of similar synchronous work which happens every time query services state changes. After this pr: Query service exposes observable to watch for changes (state$). Discover, Visualise and Dashboard use this observable for sub url tracking instead of creating its own.

Dosant added 7 commits January 27, 2020 18:30

wip

e16961d

wip

8201cbd

improve example

e855db8

Merge branch 'master' of github.com:elastic/kibana into dev/experimen…

0ddbfe5

…t-with-state-setup

improve

56895b7

fix

224841b

improve

98703dc

This was referenced Jan 31, 2020

[State Management] State syncing utils docs #56479

Merged

Disable url tracking for dashboard #55818

Merged

Dosant and others added 6 commits February 6, 2020 14:37

merge

bc07de4

top nav ts arg support

8be16b3

timefilter initial state

Merge branch 'newplatform/data/stateful-top-nav-ts' of github.com:liz…

1c7160e

…ozom/kibana into dev/experiment-with-state-setup

Merge branch 'newplatform/data/stateful-top-nav-ts' of github.com:liz…

8b71f90

…ozom/kibana into dev/experiment-with-state-setup

wip

04fc4d5

elastic deleted a comment from kibanamachine Feb 6, 2020

Dosant added 2 commits February 6, 2020 23:36

snapshots

ac55f33

use-callback

ce59461

streamich added the Team:AppArch label Feb 7, 2020

Dosant added 7 commits February 7, 2020 13:28

wip

f4fb6b8

wip

e656578

wip

962f84f

Merge branch 'dev/experiment-with-state-setup' of github.com:Dosant/k…

f0b04da

…ibana into dev/experiment-with-state-setup

wip

29516bd

Merge branch 'master' of github.com:elastic/kibana into dev/experimen…

72bdaf5

…t-with-state-setup

Dosant changed the title ~~[State Management][Discuss] Data plugin state management utils improvements~~ [State Management] State syncing helpers for query service Feb 16, 2020

Dosant changed the title ~~[State Management] State syncing helpers for query service~~ [State Management] State syncing helpers for query service I Feb 16, 2020

fix

9b70221

Dosant mentioned this pull request Feb 20, 2020

Stateful SearchBar and Query setup #58111

Closed

Dosant requested review from flash1293 and lizozom February 20, 2020 12:08

Dosant mentioned this pull request Feb 20, 2020

Expose state container for global data updates from data plugin #56503

Closed

Merge branch 'master' into dev/experiment-with-state-setup

cc1aaff

streamich mentioned this pull request Feb 24, 2020

State Management 👉 New Platform #44151

Closed

30 tasks

Merge branch 'master' into dev/experiment-with-state-setup

fa61db9

lizozom approved these changes Feb 25, 2020

View reviewed changes

Dosant requested a review from ppisljar February 25, 2020 14:14

flash1293 approved these changes Feb 26, 2020

View reviewed changes

Liza K added 3 commits February 26, 2020 14:48

fix query state in create search bar

15bd129

removed useCallback

c816281

Enabled saved queries

83e16c0

kertal reviewed Feb 26, 2020

View reviewed changes

examples/state_containers_examples/public/with_data_services/components/app.tsx Show resolved Hide resolved

Merge branch 'master' into dev/experiment-with-state-setup

5b96209

kertal mentioned this pull request Feb 26, 2020

[Discover] Migrate AppState/GlobalState to new app state helpers #57175

Merged

2 tasks

fix setting saved search

e3513e0

elasticmachine and others added 2 commits February 27, 2020 02:55

Merge branch 'master' into dev/experiment-with-state-setup

a08ec7e

jest -u

cc91afb

Dosant merged commit 2b4a9fd into elastic:master Feb 27, 2020

This was referenced Feb 27, 2020

[7.x] [State Management] State syncing helpers for query service I (#56128) #58711

Merged

[DISCUSS] State management plugin API #55977

Closed

Comments

Conversation

Dosant commented Jan 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before this pr:

After this pr:

Dev Docs

Checklist

For maintainers

Uh oh!

elasticmachine commented Feb 7, 2020

Uh oh!

lizozom commented Feb 22, 2020

Uh oh!

lizozom commented Feb 25, 2020

Uh oh!

lizozom left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

flash1293 Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

Dosant Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flash1293 Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

flash1293 Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Dosant commented Feb 26, 2020

Uh oh!

Dosant commented Feb 27, 2020

Uh oh!

kibanamachine commented Feb 27, 2020

💚 Build Succeeded

History

Uh oh!

Dosant commented Feb 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Dosant commented Jan 28, 2020 •

edited

Loading

lizozom left a comment •

edited

Loading

Dosant Feb 26, 2020 •

edited

Loading