Skip to content

Commit

Permalink
[Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180)
Browse files Browse the repository at this point in the history
Part of #192005
Closes #167438

## Summary


## Summary

This PR represents the second major cleanup task for the control group
embeddable refactor. The tasks included in this PR can be loosely
summarized as follows:
1. It removes the old, cumbersome services implementation and replaces
it with a much simpler system (which is the same one used in the `links`
+ `presentation_panel` plugins).
- This it the main reason for the decrease in lines - the old system
required a **huge** amount of boilerplate code, which is no longer
necessary with the new method for storing services.
2. It addresses and/or removes any remaining TODO comments in the
`controls` plugin
- This includes renaming `ControlStyle` and `DEFAULT_CONTROL_STYLE` to
`ControlLabelPosition` and `DEFAULT_CONTROL_LABEL_POSITION`
respectively, which represents a bulk of the changes.
3. It moves all compatibility checks for all control actions to be async
imported.
4. It removes the ability to register controls from the `controls`
plugin setup contract.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Heenawter and kibanamachine committed Sep 19, 2024
1 parent af0582c commit db55574
Show file tree
Hide file tree
Showing 91 changed files with 399 additions and 1,408 deletions.
4 changes: 2 additions & 2 deletions src/plugins/controls/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { ControlStyle, ControlWidth } from './types';
import { ControlLabelPosition, ControlWidth } from './types';

export const DEFAULT_CONTROL_WIDTH: ControlWidth = 'medium';
export const DEFAULT_CONTROL_GROW: boolean = true;
export const DEFAULT_CONTROL_STYLE: ControlStyle = 'oneLine';
export const DEFAULT_CONTROL_LABEL_POSITION: ControlLabelPosition = 'oneLine';

export const TIME_SLIDER_CONTROL = 'timeSlider';
export const RANGE_SLIDER_CONTROL = 'rangeSliderControl';
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/controls/common/control_group/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import { DataViewField } from '@kbn/data-views-plugin/common';
import { ControlStyle, DefaultControlState, ParentIgnoreSettings } from '../types';
import { ControlLabelPosition, DefaultControlState, ParentIgnoreSettings } from '../types';

export const CONTROL_GROUP_TYPE = 'control_group';

Expand All @@ -31,7 +31,7 @@ export interface ControlGroupEditorConfig {

export interface ControlGroupRuntimeState<State extends DefaultControlState = DefaultControlState> {
chainingSystem: ControlGroupChainingSystem;
labelPosition: ControlStyle; // TODO: Rename this type to ControlLabelPosition
labelPosition: ControlLabelPosition;
autoApplySelections: boolean;
ignoreParentSettings?: ParentIgnoreSettings;

Expand All @@ -50,7 +50,7 @@ export interface ControlGroupSerializedState
ignoreParentSettingsJSON: string;
// In runtime state, we refer to this property as `labelPosition`;
// to avoid migrations, we will continue to refer to this property as `controlStyle` in the serialized state
controlStyle: ControlStyle;
controlStyle: ControlLabelPosition;
// In runtime state, we refer to the inverse of this property as `autoApplySelections`
// to avoid migrations, we will continue to refer to this property as `showApplySelections` in the serialized state
showApplySelections?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/controls/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

export type {
ControlStyle,
ControlLabelPosition,
ControlWidth,
DefaultControlState,
DefaultDataControlState,
Expand All @@ -18,7 +18,7 @@ export type {

export {
DEFAULT_CONTROL_GROW,
DEFAULT_CONTROL_STYLE,
DEFAULT_CONTROL_LABEL_POSITION,
DEFAULT_CONTROL_WIDTH,
OPTIONS_LIST_CONTROL,
RANGE_SLIDER_CONTROL,
Expand Down
2 changes: 0 additions & 2 deletions src/plugins/controls/common/options_list/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { OptionsListSortingType } from './suggestions_sorting';
import { DefaultDataControlState } from '../types';
import { OptionsListSearchTechnique } from './suggestions_searching';

export const OPTIONS_LIST_CONTROL = 'optionsListControl'; // TODO: Replace with OPTIONS_LIST_CONTROL_TYPE

/**
* ----------------------------------------------------------------
* Options list state types
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/controls/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

export type ControlWidth = 'small' | 'medium' | 'large';
export type ControlStyle = 'twoLine' | 'oneLine';
export type ControlLabelPosition = 'twoLine' | 'oneLine';

export type TimeSlice = [number, number];

Expand Down
7 changes: 2 additions & 5 deletions src/plugins/controls/jest_setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,5 @@
*/

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

registry.start({});
pluginServices.setRegistry(registry);
import { setStubKibanaServices } from './public/services/mocks';
setStubKibanaServices();
4 changes: 1 addition & 3 deletions src/plugins/controls/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@
"browser": true,
"requiredPlugins": [
"presentationUtil",
"kibanaReact",
"expressions",
"embeddable",
"dataViews",
"data",
"unifiedSearch",
"uiActions"
],
"extraPublicDirs": ["common"],
"requiredBundles": ["kibanaUtils"]
"requiredBundles": []
}
}
49 changes: 8 additions & 41 deletions src/plugins/controls/public/actions/clear_control_action.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,42 +11,10 @@ import React, { SyntheticEvent } from 'react';

import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import {
apiIsPresentationContainer,
type PresentationContainer,
} from '@kbn/presentation-containers';
import {
apiCanAccessViewMode,
apiHasParentApi,
apiHasType,
apiHasUniqueId,
apiIsOfType,
type EmbeddableApiContext,
type HasParentApi,
type HasType,
type HasUniqueId,
} from '@kbn/presentation-publishing';
import { type Action, IncompatibleActionError } from '@kbn/ui-actions-plugin/public';
import type { EmbeddableApiContext, HasUniqueId } from '@kbn/presentation-publishing';
import { IncompatibleActionError, type Action } from '@kbn/ui-actions-plugin/public';

import { ACTION_CLEAR_CONTROL } from '.';
import { CONTROL_GROUP_TYPE } from '..';
import { isClearableControl, type CanClearSelections } from '../types';

export type ClearControlActionApi = HasType &
HasUniqueId &
CanClearSelections &
HasParentApi<PresentationContainer & HasType>;

const isApiCompatible = (api: unknown | null): api is ClearControlActionApi =>
Boolean(
apiHasType(api) &&
apiHasUniqueId(api) &&
isClearableControl(api) &&
apiHasParentApi(api) &&
apiCanAccessViewMode(api.parentApi) &&
apiIsOfType(api.parentApi, CONTROL_GROUP_TYPE) &&
apiIsPresentationContainer(api.parentApi)
);

export class ClearControlAction implements Action<EmbeddableApiContext> {
public readonly type = ACTION_CLEAR_CONTROL;
Expand All @@ -56,12 +24,10 @@ export class ClearControlAction implements Action<EmbeddableApiContext> {
constructor() {}

public readonly MenuItem = ({ context }: { context: EmbeddableApiContext }) => {
if (!isApiCompatible(context.embeddable)) throw new IncompatibleActionError();

return (
<EuiToolTip content={this.getDisplayName(context)}>
<EuiButtonIcon
data-test-subj={`control-action-${context.embeddable.uuid}-erase`}
data-test-subj={`control-action-${(context.embeddable as HasUniqueId).uuid}-erase`}
aria-label={this.getDisplayName(context)}
iconType={this.getIconType(context)}
onClick={(event: SyntheticEvent<HTMLButtonElement>) => {
Expand All @@ -75,23 +41,24 @@ export class ClearControlAction implements Action<EmbeddableApiContext> {
};

public getDisplayName({ embeddable }: EmbeddableApiContext) {
if (!isApiCompatible(embeddable)) throw new IncompatibleActionError();
return i18n.translate('controls.controlGroup.floatingActions.clearTitle', {
defaultMessage: 'Clear',
});
}

public getIconType({ embeddable }: EmbeddableApiContext) {
if (!isApiCompatible(embeddable)) throw new IncompatibleActionError();
return 'eraser';
}

public async isCompatible({ embeddable }: EmbeddableApiContext) {
return isApiCompatible(embeddable);
const { isCompatible } = await import('./clear_control_action_compatibility_check');
return isCompatible(embeddable);
}

public async execute({ embeddable }: EmbeddableApiContext) {
if (!isApiCompatible(embeddable)) throw new IncompatibleActionError();
const { compatibilityCheck } = await import('./clear_control_action_compatibility_check');
if (!compatibilityCheck(embeddable)) throw new IncompatibleActionError();

embeddable.clearSelections();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { PresentationContainer, apiIsPresentationContainer } from '@kbn/presentation-containers';
import {
HasParentApi,
HasType,
HasUniqueId,
apiCanAccessViewMode,
apiHasParentApi,
apiHasType,
apiHasUniqueId,
apiIsOfType,
} from '@kbn/presentation-publishing';
import { CONTROL_GROUP_TYPE } from '../../common';
import { isClearableControl, type CanClearSelections } from '../types';

type ClearControlActionApi = HasType &
HasUniqueId &
CanClearSelections &
HasParentApi<PresentationContainer & HasType>;

export const compatibilityCheck = (api: unknown | null): api is ClearControlActionApi =>
Boolean(
apiHasType(api) &&
apiHasUniqueId(api) &&
isClearableControl(api) &&
apiHasParentApi(api) &&
apiCanAccessViewMode(api.parentApi) &&
apiIsOfType(api.parentApi, CONTROL_GROUP_TYPE) &&
apiIsPresentationContainer(api.parentApi)
);

export function isCompatible(api: unknown) {
return compatibilityCheck(api);
}
19 changes: 4 additions & 15 deletions src/plugins/controls/public/actions/delete_control_action.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,15 @@

import { BehaviorSubject } from 'rxjs';

import { coreMock } from '@kbn/core/public/mocks';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import { ViewMode } from '@kbn/presentation-publishing';

import { getOptionsListControlFactory } from '../react_controls/controls/data_controls/options_list_control/get_options_list_control_factory';
import { OptionsListControlApi } from '../react_controls/controls/data_controls/options_list_control/types';
import {
getMockedBuildApi,
getMockedControlGroupApi,
} from '../react_controls/controls/mocks/control_mocks';
import { pluginServices } from '../services';
import { DeleteControlAction } from './delete_control_action';

const mockDataViews = dataViewPluginMocks.createStartContract();
const mockCore = coreMock.createStart();
import { coreServices } from '../services/kibana_services';

const dashboardApi = {
viewMode: new BehaviorSubject<ViewMode>('view'),
Expand All @@ -38,11 +31,7 @@ const controlGroupApi = getMockedControlGroupApi(dashboardApi, {

let controlApi: OptionsListControlApi;
beforeAll(async () => {
const controlFactory = getOptionsListControlFactory({
core: mockCore,
data: dataPluginMock.createStartContract(),
dataViews: mockDataViews,
});
const controlFactory = getOptionsListControlFactory();

const uuid = 'testControl';
const control = await controlFactory.buildControl(
Expand Down Expand Up @@ -72,7 +61,7 @@ test('Execute throws an error when called with an embeddable not in a parent', a
describe('Execute should open a confirm modal', () => {
test('Canceling modal will keep control', async () => {
const spyOn = jest.fn().mockResolvedValue(false);
pluginServices.getServices().overlays.openConfirm = spyOn;
coreServices.overlays.openConfirm = spyOn;

const deleteControlAction = new DeleteControlAction();
await deleteControlAction.execute({ embeddable: controlApi });
Expand All @@ -83,7 +72,7 @@ describe('Execute should open a confirm modal', () => {

test('Confirming modal will delete control', async () => {
const spyOn = jest.fn().mockResolvedValue(true);
pluginServices.getServices().overlays.openConfirm = spyOn;
coreServices.overlays.openConfirm = spyOn;

const deleteControlAction = new DeleteControlAction();
await deleteControlAction.execute({ embeddable: controlApi });
Expand Down
Loading

0 comments on commit db55574

Please sign in to comment.