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

[Uptime] Create state store at plugin setup time #66054

Closed
wants to merge 7 commits into from
Closed

[Uptime] Create state store at plugin setup time #66054

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c094dd9
Extract store creation to plugin start, add redux providers to alert …
justinkambic May 8, 2020
6bb5519
Update unit test.
justinkambic May 13, 2020
e5a1eb3
Move alert registration to `setup` function.
justinkambic May 14, 2020
03ec299
Allow external editing of uptime client alert types.
justinkambic May 14, 2020
bf16f6a
Move alert initialization back to `start`.
justinkambic May 14, 2020
c2ad6d0
Clean up interfaces for alert types.
justinkambic May 15, 2020
f8e3c7e
Add code that will work for settings link even outside uptime app.
justinkambic May 15, 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
37 changes: 34 additions & 3 deletions x-pack/plugins/uptime/public/apps/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { Store } from 'redux';
import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'kibana/public';
import { AppMountParameters, DEFAULT_APP_CATEGORIES } from '../../../../../src/core/public';
import { UMFrontendLibs } from '../lib/lib';
Expand All @@ -12,6 +14,9 @@ import { HomePublicPluginSetup } from '../../../../../src/plugins/home/public';
import { EmbeddableStart } from '../../../../../src/plugins/embeddable/public';
import { TriggersAndActionsUIPublicPluginSetup } from '../../../triggers_actions_ui/public';
import { DataPublicPluginSetup } from '../../../../../src/plugins/data/public';
import { alertTypeInitializers } from '../lib/alert_types';
import { initializeStore } from '../state';
import { kibanaService } from '../state/kibana_service';

export interface ClientPluginsSetup {
data: DataPublicPluginSetup;
Expand All @@ -24,12 +29,21 @@ export interface ClientPluginsStart {
}

export class UptimePlugin implements Plugin<void, void, ClientPluginsSetup, ClientPluginsStart> {
constructor(_context: PluginInitializerContext) {}
private _store: Store<any, any>;
private _data: DataPublicPluginSetup | undefined;
private _triggersActionsUI: TriggersAndActionsUIPublicPluginSetup | undefined;

constructor(_context: PluginInitializerContext) {
this._store = initializeStore();
}

public async setup(
core: CoreSetup<ClientPluginsStart, unknown>,
plugins: ClientPluginsSetup
): Promise<void> {
this._data = plugins.data;
this._triggersActionsUI = plugins.triggers_actions_ui;

if (plugins.home) {
plugins.home.featureCatalogue.register({
id: PLUGIN.ID,
Expand All @@ -42,6 +56,7 @@ export class UptimePlugin implements Plugin<void, void, ClientPluginsSetup, Clie
});
}

const self = this;
core.application.register({
appRoute: '/app/uptime#/',
id: PLUGIN.ID,
Expand All @@ -56,16 +71,32 @@ export class UptimePlugin implements Plugin<void, void, ClientPluginsSetup, Clie
);

const { element } = params;

const libs: UMFrontendLibs = {
framework: getKibanaFrameworkAdapter(coreStart, plugins, corePlugins),
framework: getKibanaFrameworkAdapter(coreStart, plugins, corePlugins, self._store),
};
libs.framework.render(element);
return () => {};
},
});
}

public start(_start: CoreStart, _plugins: {}): void {}
public start(start: CoreStart, _plugins: {}): void {
kibanaService.core = start;

alertTypeInitializers.forEach(init => {
const alertInitializer = init({
autocomplete: this._data!.autocomplete,
Copy link
Contributor

Choose a reason for hiding this comment

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

This may not work correctly in a future release. We plan to lock down the setup APIs to only be callable during the setup phase. If you just need data.autocomplete.getQuerySuggestions, that is also available on start.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I guess in general, I'm not sure why these initializers are called in start rather than setup? Seems like it all depends on setup APIs?

Copy link
Contributor

@shahzad31 shahzad31 May 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this can perhaps be moved inside component by using useKibana hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshdover can you have a look at e5a1eb3 when you've got a chance? I've moved the registration back to setup and removed instance fields for setup APIs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, that looks better 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshdover I'm actually actively working on resolving more issues with this branch, but they're related to alerting. One thing we need is core.services.application.getUrlForApp from CoreStart. I've moved this back to start, but triggers_actions_ui is available on the start plugins param, and autocomplete is available as well (like you've mentioned).

I'm not referencing anything from setup now though. Can you verify this is still ok? I think once we've ironed that out it should be the end of platform-specific concerns. bf16f6a

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should reconsider how this whole thing is designed. Why is that the Redux store needs to exist at all? Is it possible we could decouple the UI from the store to make it more portable?

store: this._store,
});
if (
this._triggersActionsUI &&
!this._triggersActionsUI.alertTypeRegistry.has(alertInitializer.id)
) {
this._triggersActionsUI.alertTypeRegistry.register(alertInitializer);
}
});
}

public stop(): void {}
}
25 changes: 15 additions & 10 deletions x-pack/plugins/uptime/public/contexts/uptime_refresh_context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
*/

import React, { createContext, useMemo, useState } from 'react';
import { store } from '../state';
import { Store } from 'redux';
import { triggerAppRefresh } from '../state/actions';

interface UptimeRefreshContext {
lastRefresh: number;
refreshApp: () => void;
}

interface RefreshContextProps {
store: Store<any>;
}

const defaultContext: UptimeRefreshContext = {
lastRefresh: 0,
refreshApp: () => {
Expand All @@ -22,19 +26,20 @@ const defaultContext: UptimeRefreshContext = {

export const UptimeRefreshContext = createContext(defaultContext);

export const UptimeRefreshContextProvider: React.FC = ({ children }) => {
export const UptimeRefreshContextProvider: React.FC<RefreshContextProps> = ({
children,
store,
}) => {
const [lastRefresh, setLastRefresh] = useState<number>(Date.now());

const refreshApp = () => {
const refreshTime = Date.now();
setLastRefresh(refreshTime);
// @ts-ignore
store.dispatch(triggerAppRefresh(refreshTime));
};

const value = useMemo(() => {
const refreshApp = () => {
const refreshTime = Date.now();
setLastRefresh(refreshTime);
store.dispatch(triggerAppRefresh(refreshTime));
};
return { lastRefresh, refreshApp };
}, [lastRefresh]);
}, [lastRefresh, store]);

return <UptimeRefreshContext.Provider value={value} children={children} />;
};
19 changes: 5 additions & 14 deletions x-pack/plugins/uptime/public/lib/adapters/framework/new_platform_adapter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import React from 'react';
import ReactDOM from 'react-dom';
import { get } from 'lodash';
import { i18n as i18nFormatter } from '@kbn/i18n';
import { alertTypeInitializers } from '../../alert_types';
import { Store } from 'redux';
import { UptimeApp, UptimeAppProps } from '../../../uptime_app';
import { getIntegratedAppAvailability } from './capabilities_adapter';
import {
Expand All @@ -20,11 +20,13 @@ import {
} from '../../../../common/constants';
import { UMFrameworkAdapter } from '../../lib';
import { ClientPluginsStart, ClientPluginsSetup } from '../../../apps/plugin';
import { AppState } from '../../../state';

export const getKibanaFrameworkAdapter = (
core: CoreStart,
plugins: ClientPluginsSetup,
startPlugins: ClientPluginsStart
startPlugins: ClientPluginsStart,
store: Store<AppState>
): UMFrameworkAdapter => {
const {
application: { capabilities },
Expand All @@ -34,18 +36,6 @@ export const getKibanaFrameworkAdapter = (
i18n,
} = core;

const {
data: { autocomplete },
triggers_actions_ui,
} = plugins;

alertTypeInitializers.forEach(init => {
const alertInitializer = init({ autocomplete });
if (!triggers_actions_ui.alertTypeRegistry.has(alertInitializer.id)) {
triggers_actions_ui.alertTypeRegistry.register(init({ autocomplete }));
}
});

let breadcrumbs: ChromeBreadcrumb[] = [];
core.chrome.getBreadcrumbs$().subscribe((nextBreadcrumbs?: ChromeBreadcrumb[]) => {
breadcrumbs = nextBreadcrumbs || [];
Expand Down Expand Up @@ -90,6 +80,7 @@ export const getKibanaFrameworkAdapter = (
routerBasename: basePath.prepend(PLUGIN.ROUTER_BASE_NAME),
setBadge,
setBreadcrumbs: core.chrome.setBreadcrumbs,
store,
};

return {
Expand Down
27 changes: 25 additions & 2 deletions x-pack/plugins/uptime/public/lib/alert_types/__tests__/monitor_status.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,38 @@ describe('monitor status alert type', () => {
});

describe('initMonitorStatusAlertType', () => {
expect(initMonitorStatusAlertType({ autocomplete: {} })).toMatchInlineSnapshot(`
expect(
initMonitorStatusAlertType({
autocomplete: {},
store: {
dispatch: jest.fn(),
getState: jest.fn(),
replaceReducer: jest.fn(),
subscribe: jest.fn(),
[Symbol.observable]: jest.fn(),
},
})
).toMatchInlineSnapshot(`
Object {
"alertParamsExpression": [Function],
"defaultActionMessage": "{{context.message}}
Last triggered at: {{state.lastTriggeredAt}}
{{context.downMonitorsWithGeo}}",
"iconClass": "uptimeApp",
"id": "xpack.uptime.alerts.monitorStatus",
"name": <MonitorStatusTitle />,
"name": <Provider
store={
Object {
"dispatch": [MockFunction],
"getState": [MockFunction],
"replaceReducer": [MockFunction],
"subscribe": [MockFunction],
Symbol(observable): [MockFunction],
}
}
>
<MonitorStatusTitle />
</Provider>,
"requiresAppContext": true,
"validate": [Function],
}
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/uptime/public/lib/alert_types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Store } from 'redux';
import { AlertTypeModel } from '../../../../triggers_actions_ui/public';
import { initMonitorStatusAlertType } from './monitor_status';
import { initTlsAlertType } from './tls';

export type AlertTypeInitializer = (dependenies: { autocomplete: any }) => AlertTypeModel;
export type AlertTypeInitializer = (dependenies: {
autocomplete: any;
store: Store<any>;
}) => AlertTypeModel;

export const alertTypeInitializers: AlertTypeInitializer[] = [
initMonitorStatusAlertType,
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/uptime/public/lib/alert_types/monitor_status.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { PathReporter } from 'io-ts/lib/PathReporter';
import { Provider as ReduxProvider } from 'react-redux';
import React from 'react';
import DateMath from '@elastic/datemath';
import { isRight } from 'fp-ts/lib/Either';
Expand Down Expand Up @@ -59,12 +60,19 @@ const { defaultActionMessage } = MonitorStatusTranslations;

export const initMonitorStatusAlertType: AlertTypeInitializer = ({
autocomplete,
store,
}): AlertTypeModel => ({
id: CLIENT_ALERT_TYPES.MONITOR_STATUS,
name: <MonitorStatusTitle />,
name: (
<ReduxProvider store={store}>
<MonitorStatusTitle />
</ReduxProvider>
),
iconClass: 'uptimeApp',
alertParamsExpression: (params: any) => (
<AlertMonitorStatus {...params} autocomplete={autocomplete} />
<ReduxProvider store={store}>
<AlertMonitorStatus {...params} autocomplete={autocomplete} />
</ReduxProvider>
),
validate,
defaultActionMessage,
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/uptime/public/lib/alert_types/tls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import React from 'react';
import { Provider as ReduxProvider } from 'react-redux';
import { AlertTypeModel } from '../../../../triggers_actions_ui/public';
import { CLIENT_ALERT_TYPES } from '../../../common/constants';
import { TlsTranslations } from './translations';
Expand All @@ -13,10 +14,14 @@ import { AlertTls } from '../../components/overview/alerts/alerts_containers/ale

const { name, defaultActionMessage } = TlsTranslations;

export const initTlsAlertType: AlertTypeInitializer = (): AlertTypeModel => ({
export const initTlsAlertType: AlertTypeInitializer = ({ store }): AlertTypeModel => ({
id: CLIENT_ALERT_TYPES.TLS,
iconClass: 'uptimeApp',
alertParamsExpression: () => <AlertTls />,
alertParamsExpression: () => (
<ReduxProvider store={store}>
<AlertTls />
</ReduxProvider>
),
name,
validate: () => ({ errors: {} }),
defaultActionMessage,
Expand Down
14 changes: 9 additions & 5 deletions x-pack/plugins/uptime/public/state/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ import createSagaMiddleware from 'redux-saga';
import { rootEffect } from './effects';
import { rootReducer } from './reducers';

const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;
export type AppState = ReturnType<typeof rootReducer>;

const sagaMW = createSagaMiddleware();
export const initializeStore = () => {
const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;

export const store = createStore(rootReducer, composeEnhancers(applyMiddleware(sagaMW)));
const sagaMW = createSagaMiddleware();

export type AppState = ReturnType<typeof rootReducer>;
const store = createStore(rootReducer, composeEnhancers(applyMiddleware(sagaMW)));

sagaMW.run(rootEffect);

sagaMW.run(rootEffect);
return store;
};
9 changes: 4 additions & 5 deletions x-pack/plugins/uptime/public/uptime_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React, { useEffect } from 'react';
import { Provider as ReduxProvider } from 'react-redux';
import { BrowserRouter as Router } from 'react-router-dom';
import { I18nStart, ChromeBreadcrumb, CoreStart } from 'src/core/public';
import { Store } from 'redux';
import { KibanaContextProvider } from '../../../../src/plugins/kibana_react/public';
import { ClientPluginsSetup, ClientPluginsStart } from './apps/plugin';
import { UMUpdateBadge } from './lib/lib';
Expand All @@ -20,14 +21,12 @@ import {
UptimeStartupPluginsContextProvider,
} from './contexts';
import { CommonlyUsedRange } from './components/common/uptime_date_picker';
import { store } from './state';
import { setBasePath } from './state/actions';
import { PageRouter } from './routes';
import {
UptimeAlertsContextProvider,
UptimeAlertsFlyoutWrapper,
} from './components/overview/alerts';
import { kibanaService } from './state/kibana_service';

export interface UptimeAppColors {
danger: string;
Expand Down Expand Up @@ -55,6 +54,7 @@ export interface UptimeAppProps {
renderGlobalHelpControls(): void;
commonlyUsedRanges: CommonlyUsedRange[];
setBreadcrumbs: (crumbs: ChromeBreadcrumb[]) => void;
store: Store<any>;
}

const Application = (props: UptimeAppProps) => {
Expand All @@ -69,6 +69,7 @@ const Application = (props: UptimeAppProps) => {
routerBasename,
setBadge,
startPlugins,
store,
} = props;

useEffect(() => {
Expand All @@ -88,8 +89,6 @@ const Application = (props: UptimeAppProps) => {
);
}, [canSave, renderGlobalHelpControls, setBadge]);

kibanaService.core = core;

store.dispatch(setBasePath(basePath));

return (
Expand All @@ -98,7 +97,7 @@ const Application = (props: UptimeAppProps) => {
<ReduxProvider store={store}>
<KibanaContextProvider services={{ ...core, ...plugins }}>
<Router basename={routerBasename}>
<UptimeRefreshContextProvider>
<UptimeRefreshContextProvider store={store}>
<UptimeSettingsContextProvider {...props}>
<UptimeThemeContextProvider darkMode={darkMode}>
<UptimeStartupPluginsContextProvider {...startPlugins}>
Expand Down