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

[Dashboard] [Controls] Refactor controls services #138668

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
2 changes: 1 addition & 1 deletion src/plugins/controls/jest_setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

// Start the services with stubs
import { pluginServices } from './public/services';
import { registry } from './public/services/stub';
import { registry } from './public/services/plugin_services.story';

registry.start({});
pluginServices.setRegistry(registry);
8 changes: 4 additions & 4 deletions src/plugins/controls/public/__stories__/controls.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ import {
import { decorators } from './decorators';
import { ControlsPanels } from '../control_group/types';
import { ControlGroupContainer } from '../control_group';
import { pluginServices, registry } from '../services/storybook';
import { injectStorybookDataView } from '../services/storybook/data_views';
import { replaceOptionsListMethod } from '../services/storybook/options_list';
import { pluginServices, registry } from '../services/plugin_services.story';
import { injectStorybookDataView } from '../services/data_views/data_views.story';
import { replaceOptionsListMethod } from '../services/options_list/options_list.story';
import { populateStorybookControlFactories } from './storybook_control_factories';
import { replaceValueSuggestionMethod } from '../services/storybook/unified_search';
import { replaceValueSuggestionMethod } from '../services/unified_search/unified_search.story';
import { OptionsListResponse, OptionsListRequest } from '../options_list/types';

export default {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

import { OptionsListEmbeddableFactory } from '../options_list';
import { RangeSliderEmbeddableFactory } from '../range_slider';
import { ControlsService } from '../services/controls';
import { ControlsServiceType } from '../services/controls/types';
import { ControlFactory } from '..';

export const populateStorybookControlFactories = (controlsServiceStub: ControlsService) => {
export const populateStorybookControlFactories = (controlsServiceStub: ControlsServiceType) => {
const optionsListFactoryStub = new OptionsListEmbeddableFactory();

// cast to unknown because the stub cannot use the embeddable start contract to transform the EmbeddableFactoryDefinition into an EmbeddableFactory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ export const ControlFrame = ({
const controlStyle = select((state) => state.explicitInput.controlStyle);

// Controls Services Context
const { overlays } = pluginServices.getHooks();
const { openConfirm } = overlays.useService();
const {
overlays: { openConfirm },
} = pluginServices.getServices();

const embeddable = useChildEmbeddable({ untilEmbeddableLoaded, embeddableId, embeddableType });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ export const ControlEditor = ({
getRelevantDataViewId,
setLastUsedDataViewId,
}: EditControlProps) => {
const { dataViews, controls } = pluginServices.getHooks();
const { getIdsWithTitle, getDefaultId, get } = dataViews.useService();
const { getControlTypes, getControlFactory } = controls.useService();
const {
dataViews: { getIdsWithTitle, getDefaultId, get },
controls: { getControlTypes, getControlFactory },
} = pluginServices.getServices();
const [state, setState] = useState<ControlEditorState>({
dataViewListItems: [],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ export const CreateControlButton = ({
updateDefaultGrow,
}: CreateControlButtonProps) => {
// Controls Services Context
const { overlays, controls, theme } = pluginServices.getHooks();
const { getControlTypes, getControlFactory } = controls.useService();
const { openFlyout, openConfirm } = overlays.useService();
const themeService = theme.useService();
const {
overlays: { openFlyout, openConfirm },
controls: { getControlTypes, getControlFactory },
theme: { theme$ },
} = pluginServices.getServices();

const createNewControl = async () => {
const ControlsServicesProvider = pluginServices.getContextProvider();
Expand Down Expand Up @@ -115,7 +116,7 @@ export const CreateControlButton = ({
}
/>
</ControlsServicesProvider>,
{ theme$: themeService.theme$ }
{ theme$ }
),
{
'aria-label': ControlGroupStrings.manageControl.getFlyoutCreateTitle(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ interface EditControlResult {

export const EditControlButton = ({ embeddableId }: { embeddableId: string }) => {
// Controls Services Context
const { overlays, controls, theme } = pluginServices.getHooks();
const { getControlFactory } = controls.useService();
const { openFlyout, openConfirm } = overlays.useService();
const themeService = theme.useService();

const {
overlays: { openFlyout, openConfirm },
controls: { getControlFactory },
theme: { theme$ },
} = pluginServices.getServices();
// Redux embeddable container Context
const reduxContainerContext = useReduxContainerContext<
ControlGroupReduxState,
Expand Down Expand Up @@ -161,7 +161,7 @@ export const EditControlButton = ({ embeddableId }: { embeddableId: string }) =>
}}
/>
</ControlsServicesProvider>,
{ theme$: themeService.theme$ }
{ theme$ }
),
{
'aria-label': ControlGroupStrings.manageControl.getFlyoutEditTitle(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ export const EditControlGroup = ({
controlGroupContainer,
closePopover,
}: EditControlGroupButtonProps) => {
const { overlays, theme } = pluginServices.getHooks();
const { openConfirm, openFlyout } = overlays.useService();
const themeService = theme.useService();
const {
overlays: { openConfirm, openFlyout },
theme: { theme$ },
} = pluginServices.getServices();

const editControlGroup = () => {
const onDeleteAll = (ref: OverlayRef) => {
Expand All @@ -55,7 +56,7 @@ export const EditControlGroup = ({
onDeleteAll={() => onDeleteAll(flyoutInstance)}
onClose={() => flyoutInstance.close()}
/>,
{ theme$: themeService.theme$ }
{ theme$ }
),
{
outsideClickCloses: false,
Expand Down
31 changes: 0 additions & 31 deletions src/plugins/controls/public/controls_service.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ import {
import { pluginServices } from '../../services';
import { ControlInput, ControlOutput } from '../..';
import { optionsListReducers } from '../options_list_reducers';
import { ControlsDataViewsService } from '../../services/data_views';
import { OptionsListControl } from '../components/options_list_control';
import { ControlsOptionsListService } from '../../services/options_list';
import { ControlsDataViewsService } from '../../services/data_views/types';
import { ControlsOptionsListService } from '../../services/options_list/types';

const diffDataFetchProps = (
last?: OptionsListDataFetchProps,
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/controls/public/options_list/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,9 @@
*/

import { ReduxEmbeddableState } from '@kbn/presentation-util-plugin/public';

import { ControlOutput } from '../types';
import { OptionsListEmbeddableInput, OptionsListField } from '../../common/options_list/types';

export * from '../../common/options_list/types';

// Component state is only used by public components.
export interface OptionsListComponentState {
field?: OptionsListField;
Expand All @@ -29,3 +26,5 @@ export type OptionsListReduxState = ReduxEmbeddableState<
ControlOutput,
OptionsListComponentState
>;

export * from '../../common/options_list/types';
Copy link
Contributor

@cqliu1 cqliu1 Aug 25, 2022

Choose a reason for hiding this comment

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

Why was this line moved to the bottom of the file? Just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was making it match the range_slider types file, which also has it at the bottom. But I could move both to the top instead? Don't really have a strong preference - what are your thoughts?

Copy link
Contributor

@cqliu1 cqliu1 Aug 29, 2022

Choose a reason for hiding this comment

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

Hmmmmm I guess you can leave it at the bottom. It's an export so it's not like we need to group all of them at the top. We have a clear pattern for imports, but I don't think we've discussed exports. I don't know that we need to organize exports in any particular way besides just having them below imports? What do you think @ThomThomson

Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these exports being used? Can we not just import from common in those places rather than reexporting here? I think it's usually best not to re-export if it can be avoided, because it can make things more difficult to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very fair point - the common export actually seems to be used more frequently than the re-exported public types (at least for options list - haven't checked range slider yet), so I think it makes the most sense to just avoid re-exporting stuff even if it results in shorter paths 👍 I'll make the change to this PR since it should be only a few changes

5 changes: 2 additions & 3 deletions src/plugins/controls/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import {
} from '.';
import { OptionsListEmbeddableFactory, OptionsListEmbeddableInput } from './options_list';
import { RangeSliderEmbeddableFactory, RangeSliderEmbeddableInput } from './range_slider';
import { pluginServices } from './services';
import { controlsService } from './services/kibana/controls';
import { controlsService } from './services/controls/controls_service';
import {
ControlsPluginSetup,
ControlsPluginStart,
Expand All @@ -41,7 +40,7 @@ export class ControlsPlugin
coreStart: CoreStart,
startPlugins: ControlsPluginStartDeps
) {
const { registry } = await import('./services/kibana');
const { registry, pluginServices } = await import('./services/plugin_services');
pluginServices.setRegistry(registry.start({ coreStart, startPlugins }));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ export const RangeSliderPopover: FC = () => {
const rangeRef = useRef<EuiDualRange | null>(null);

// Controls Services Context
const { dataViews } = pluginServices.getHooks();
const { get: getDataViewById } = dataViews.useService();
const {
dataViews: { get: getDataViewById },
} = pluginServices.getServices();
const {
useEmbeddableDispatch,
useEmbeddableSelector: select,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ import { ReduxEmbeddableTools, ReduxEmbeddablePackage } from '@kbn/presentation-

import { ControlInput, ControlOutput } from '../..';
import { pluginServices } from '../../services';
import { ControlsDataService } from '../../services/data';
import { ControlsDataViewsService } from '../../services/data_views';
import { RangeSliderControl } from '../components/range_slider_control';
import { getDefaultComponentState, rangeSliderReducers } from '../range_slider_reducers';
import { RangeSliderEmbeddableInput, RangeSliderReduxState, RANGE_SLIDER_CONTROL } from '../types';
import { ControlsDataService } from '../../services/data/types';
import { ControlsDataViewsService } from '../../services/data_views/types';

const diffDataFetchProps = (
current?: RangeSliderDataFetchProps,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import { EmbeddableFactory } from '@kbn/embeddable-plugin/public';
import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { ControlEmbeddable, ControlFactory, ControlInput, ControlOutput } from '../..';
import { ControlsService, ControlTypeRegistry } from '../controls';
import { ControlsServiceType, ControlTypeRegistry } from './types';

export type ControlsServiceFactory = PluginServiceFactory<ControlsService>;
export type ControlsServiceFactory = PluginServiceFactory<ControlsServiceType>;
export const controlsServiceFactory = () => getStubControlsService();

export const getStubControlsService = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
import { EmbeddableFactory } from '@kbn/embeddable-plugin/public';
import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { ControlEmbeddable, ControlFactory, ControlInput, ControlOutput } from '../..';
import { ControlsService, ControlTypeRegistry } from '../controls';
import { ControlsServiceType, ControlTypeRegistry } from './types';

export type ControlsServiceFactory = PluginServiceFactory<ControlsService>;
export type ControlsServiceFactory = PluginServiceFactory<ControlsServiceType>;
export const controlsServiceFactory = () => controlsService;

const controlsFactoriesMap: ControlTypeRegistry = {};

// export controls service directly for use in plugin setup lifecycle
export const controlsService: ControlsService = {
export const controlsService: ControlsServiceType = {
registerControlType: (factory: ControlFactory) => {
controlsFactoriesMap[factory.type] = factory;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
*/

import { EmbeddableFactory } from '@kbn/embeddable-plugin/public';
import { ControlEmbeddable, ControlFactory, ControlOutput, ControlInput } from '../types';
import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { ControlEmbeddable, ControlFactory, ControlOutput, ControlInput } from '../../types';

export type ControlsServiceFactory = PluginServiceFactory<ControlsServiceType>;

export interface ControlTypeRegistry {
[key: string]: ControlFactory;
}

export interface ControlsService {
export interface ControlsServiceType {
registerControlType: (factory: ControlFactory) => void;

getControlFactory: <
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
* Side Public License, v 1.
*/

import { of, Observable } from 'rxjs';
import { of } from 'rxjs';
import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { DataPublicPluginStart } from '@kbn/data-plugin/public';
import { DataView } from '@kbn/data-views-plugin/common';
import { ControlsDataService } from '../data';
import { ControlsDataService } from './types';

export type DataServiceFactory = PluginServiceFactory<ControlsDataService>;
export const dataServiceFactory: DataServiceFactory = () => ({
Expand All @@ -30,7 +29,4 @@ export const dataServiceFactory: DataServiceFactory = () => ({
createFilter: () => {},
} as unknown as DataPublicPluginStart['query']['timefilter']['timefilter'],
fetchFieldRange: () => Promise.resolve({ min: 0, max: 100 }),
fetchFieldRange$: () => new Observable<{ min: number; max: number }>(),
getDataView: () => Promise.resolve({} as DataView),
getDataView$: () => new Observable({} as any),
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

import { DataViewField } from '@kbn/data-views-plugin/common';
import { get } from 'lodash';
import { from, lastValueFrom } from 'rxjs';
import { lastValueFrom } from 'rxjs';
import { KibanaPluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { ControlsDataService } from '../data';
import { ControlsDataService } from './types';
import { ControlsPluginStartDeps } from '../../types';

export type DataServiceFactory = KibanaPluginServiceFactory<
Expand Down Expand Up @@ -91,10 +91,6 @@ export const dataServiceFactory: DataServiceFactory = ({ startPlugins }) => {

return {
fetchFieldRange,
fetchFieldRange$: (dataView, fieldName, input) =>
from(fetchFieldRange(dataView, fieldName, input)),
getDataView: data.dataViews.get,
getDataView$: (id: string) => from(data.dataViews.get(id)),
query: queryPlugin,
searchSource: search.searchSource,
timefilter: queryPlugin.timefilter.timefilter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,16 @@
* Side Public License, v 1.
*/

import { Observable } from 'rxjs';
import { DataPublicPluginStart } from '@kbn/data-plugin/public';
import { DataView } from '@kbn/data-views-plugin/public';
import { ControlInput } from '../types';
import { ControlInput } from '../../types';

export interface ControlsDataService {
fetchFieldRange: (
dataView: DataView,
fieldName: string,
input: ControlInput
) => Promise<{ min: number; max: number }>;
fetchFieldRange$: (
dataView: DataView,
fieldName: string,
input: ControlInput
) => Observable<{ min?: number; max?: number }>;
getDataView: DataPublicPluginStart['dataViews']['get'];
getDataView$: (id: string) => Observable<DataView>;
query: DataPublicPluginStart['query'];
searchSource: DataPublicPluginStart['search']['searchSource'];
timefilter: DataPublicPluginStart['query']['timefilter']['timefilter'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
import { DataView } from '@kbn/data-views-plugin/common';
import { ControlsDataViewsService } from '../data_views';
import { ControlsDataViewsService } from './types';

export type DataViewsServiceFactory = PluginServiceFactory<ControlsDataViewsService>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { KibanaPluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { ControlsPluginStartDeps } from '../../types';
import { ControlsDataViewsService } from '../data_views';
import { ControlsDataViewsService } from './types';

export type DataViewsServiceFactory = KibanaPluginServiceFactory<
ControlsDataViewsService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { HttpResponse } from '@kbn/core/public';
import { PluginServiceFactory } from '@kbn/presentation-util-plugin/public';
import { ControlsHTTPService } from '../http';
import { ControlsHTTPService } from './types';

type HttpServiceFactory = PluginServiceFactory<ControlsHTTPService>;

Expand Down
Loading