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.

Sign up for GitHub

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

[State Management] State syncing utilities - remove AppState from Dashboard app #54105

Merged
merged 14 commits into from
Jan 21, 2020
Merged

[State Management] State syncing utilities - remove AppState from Dashboard app #54105

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5073715
Remove AppState from np/dashboard
Dosant Jan 14, 2020
5b042db
fix hard reseting time filters
Dosant Jan 15, 2020
fb47fc8
make sure url ('_a') matches initial state
Dosant Jan 15, 2020
9fced16
Merge branch 'master' of github.com:elastic/kibana into dev/state-man…
Dosant Jan 15, 2020
262912e
Merge branch 'master' of github.com:elastic/kibana into dev/state-man…
Dosant Jan 17, 2020
d15cd9b
improvements according to review
Dosant Jan 17, 2020
10fe777
clean up AppState imports
Dosant Jan 17, 2020
873aa19
Merge branch 'master' of github.com:elastic/kibana into dev/state-man…
Dosant Jan 20, 2020
1257fb1
fix used before declare
Dosant Jan 20, 2020
6e71ffb
fixing test
Dosant Jan 20, 2020
23845a5
improve stability of dashboard clone test
Dosant Jan 20, 2020
3f7a2b8
Merge branch 'master' of github.com:elastic/kibana into dev/state-man…
Dosant Jan 20, 2020
45bd639
Merge branch 'master' into dev/state-managment/state-sync-dashboard
elasticmachine Jan 21, 2020
446c744
Merge branch 'master' of github.com:elastic/kibana into dev/state-man…
Dosant Jan 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,24 @@
*/

import _ from 'lodash';
import { Subscription } from 'rxjs';
import { State } from 'ui/state_management/state';
import { FilterManager, esFilters } from '../../../../../../plugins/data/public';

type GetAppStateFunc = () => State | undefined | null;
type GetAppStateFunc = () => { filters?: esFilters.Filter[]; save?: () => void } | undefined | null;

/**
* FilterStateManager is responsible for watching for filter changes
* and syncing with FilterManager, as well as syncing FilterManager changes
* back to the URL.
**/
export class FilterStateManager {
private filterManagerUpdatesSubscription: Subscription;

filterManager: FilterManager;
globalState: State;
getAppState: GetAppStateFunc;
interval: NodeJS.Timeout | undefined;
interval: number | undefined;

constructor(globalState: State, getAppState: GetAppStateFunc, filterManager: FilterManager) {
this.getAppState = getAppState;
Expand All @@ -41,7 +44,7 @@ export class FilterStateManager {

this.watchFilterState();

this.filterManager.getUpdates$().subscribe(() => {
this.filterManagerUpdatesSubscription = this.filterManager.getUpdates$().subscribe(() => {
this.updateAppState();
});
}
Expand All @@ -50,12 +53,13 @@ export class FilterStateManager {
if (this.interval) {
clearInterval(this.interval);
}
this.filterManagerUpdatesSubscription.unsubscribe();
}

private watchFilterState() {
// This is a temporary solution to remove rootscope.
// Moving forward, state should provide observable subscriptions.
this.interval = setInterval(() => {
this.interval = window.setInterval(() => {
const appState = this.getAppState();
const stateUndefined = !appState || !this.globalState;
if (stateUndefined) return;
Expand All @@ -80,7 +84,7 @@ export class FilterStateManager {

private saveState() {
const appState = this.getAppState();
if (appState) appState.save();
if (appState && appState.save) appState.save();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we get an appState without a save method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This filter_state_management is still used in Dashboard (until next pr, where we will remove global state also)
So, as we removed AppState from Dashboard, I change the interface of filter_state_manager to :
https://github.com/elastic/kibana/pull/54105/files/10fe7772a3297f9e5b03591dd9d47a5b8ca9be02#diff-e99af5c3c776f37f6e37ab25d094e3cfR25

And save function is for legacy AppState case, but new state sync utils don't have explicit save() method.

this.globalState.save();
}

Expand Down
1 change: 0 additions & 1 deletion src/legacy/core_plugins/kibana/public/dashboard/__tests__/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@
* under the License.
*/

export { getAppStateMock } from './get_app_state_mock';
export { getSavedDashboardMock } from './get_saved_dashboard_mock';
export { getEmbeddableFactoryMock } from './get_embeddable_factories_mock';
6 changes: 0 additions & 6 deletions src/legacy/core_plugins/kibana/public/dashboard/legacy_imports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import chrome from 'ui/chrome';

export const legacyChrome = chrome;
export { State } from 'ui/state_management/state';
export { AppState } from 'ui/state_management/app_state';
export { AppStateClass } from 'ui/state_management/app_state';
export { SavedObjectSaveOpts } from 'ui/saved_objects/types';
export { npSetup, npStart } from 'ui/new_platform';
export { IPrivate } from 'ui/private';
Expand All @@ -45,8 +43,6 @@ export { GlobalStateProvider } from 'ui/state_management/global_state';
// @ts-ignore
export { StateManagementConfigProvider } from 'ui/state_management/config_provider';
// @ts-ignore
export { AppStateProvider } from 'ui/state_management/app_state';
// @ts-ignore
export { PrivateProvider } from 'ui/private/private';
// @ts-ignore
export { EventsProvider } from 'ui/events';
Expand All @@ -60,9 +56,7 @@ export { KbnUrlProvider, RedirectWhenMissingProvider } from 'ui/url/index';
// @ts-ignore
export { confirmModalFactory } from 'ui/modals/confirm_modal';
export { configureAppAngularModule } from 'ui/legacy_compat';
export { stateMonitorFactory, StateMonitor } from 'ui/state_management/state_monitor_factory';
export { ensureDefaultIndexPattern } from 'ui/legacy_compat';
export { unhashUrl } from '../../../../../plugins/kibana_utils/public';
export { IInjector } from 'ui/chrome';
export { SavedObjectLoader } from 'ui/saved_objects';
export { VISUALIZE_EMBEDDABLE_TYPE } from '../../../visualizations/public/embeddable';
Expand Down
7 changes: 0 additions & 7 deletions src/legacy/core_plugins/kibana/public/dashboard/np_ready/application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { Storage } from '../../../../../../plugins/kibana_utils/public';
import {
GlobalStateProvider,
StateManagementConfigProvider,
AppStateProvider,
PrivateProvider,
EventsProvider,
PersistedState,
Expand Down Expand Up @@ -155,12 +154,6 @@ function createLocalStateModule() {
'app/dashboard/Promise',
'app/dashboard/PersistedState',
])
.factory('AppState', function(Private: any) {
return Private(AppStateProvider);
})
.service('getAppState', function(Private: any) {
return Private(AppStateProvider).getAppState;
})
.service('globalState', function(Private: any) {
return Private(GlobalStateProvider);
});
Expand Down
16 changes: 2 additions & 14 deletions src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,7 @@
import moment from 'moment';
import { Subscription } from 'rxjs';

import {
AppStateClass as TAppStateClass,
AppState as TAppState,
IInjector,
KbnUrl,
} from '../legacy_imports';
import { IInjector } from '../legacy_imports';

import { ViewMode } from '../../../../embeddable_api/public/np_ready/public';
import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard';
Expand All @@ -43,7 +38,7 @@ import { RenderDeps } from './application';

export interface DashboardAppScope extends ng.IScope {
dash: SavedObjectDashboard;
appState: TAppState;
appState: DashboardAppState;
screenTitle: string;
model: {
query: Query;
Expand All @@ -60,7 +55,6 @@ export interface DashboardAppScope extends ng.IScope {
refreshInterval: any;
panels: SavedDashboardPanel[];
indexPatterns: IIndexPattern[];
$evalAsync: any;
dashboardViewMode: ViewMode;
expandedPanel?: string;
getShouldShowEditHelp: () => boolean;
Expand Down Expand Up @@ -91,8 +85,6 @@ export interface DashboardAppScope extends ng.IScope {

export function initDashboardAppDirective(app: any, deps: RenderDeps) {
app.directive('dashboardApp', function($injector: IInjector) {
const AppState = $injector.get<TAppStateClass<DashboardAppState>>('AppState');
const kbnUrl = $injector.get<KbnUrl>('kbnUrl');
const confirmModal = $injector.get<ConfirmModalFn>('confirmModal');
const config = deps.uiSettings;

Expand All @@ -105,17 +97,13 @@ export function initDashboardAppDirective(app: any, deps: RenderDeps) {
$routeParams: {
id?: string;
},
getAppState: any,
globalState: any
) =>
new DashboardAppController({
$route,
$scope,
$routeParams,
getAppState,
globalState,
kbnUrl,
AppStateClass: AppState,
config,
confirmModal,
indexPatterns: deps.npDataStart.indexPatterns,
Expand Down
Loading