Skip to content

Commit

Permalink
[8.x] [Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180
Browse files Browse the repository at this point in the history
) (#193429)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Embeddable Rebuild] [Controls] Clean up services + TODOs
(#193180)](#193180)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Hannah
Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-19T14:02:49Z","message":"[Embeddable
Rebuild] [Controls] Clean up services + TODOs (#193180)\n\nPart of
https://github.com/elastic/kibana/issues/192005\r\nCloses
https://github.com/elastic/kibana/issues/167438\r\n\r\n##
Summary\r\n\r\n\r\n## Summary\r\n\r\nThis PR represents the second major
cleanup task for the control group\r\nembeddable refactor. The tasks
included in this PR can be loosely\r\nsummarized as follows:\r\n1. It
removes the old, cumbersome services implementation and replaces\r\nit
with a much simpler system (which is the same one used in the
`links`\r\n+ `presentation_panel` plugins).\r\n- This it the main reason
for the decrease in lines - the old system\r\nrequired a **huge** amount
of boilerplate code, which is no longer\r\nnecessary with the new method
for storing services.\r\n2. It addresses and/or removes any remaining
TODO comments in the\r\n`controls` plugin\r\n- This includes renaming
`ControlStyle` and `DEFAULT_CONTROL_STYLE` to\r\n`ControlLabelPosition`
and `DEFAULT_CONTROL_LABEL_POSITION`\r\nrespectively, which represents a
bulk of the changes.\r\n3. It moves all compatibility checks for all
control actions to be async\r\nimported.\r\n4. It removes the ability to
register controls from the `controls`\r\nplugin setup
contract.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"db5557429979b9a0f3420a50a06c7fd69cbdf5b2","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","impact:high","v9.0.0","backport:prev-minor"],"title":"[Embeddable
Rebuild] [Controls] Clean up services +
TODOs","number":193180,"url":"https://github.com/elastic/kibana/pull/193180","mergeCommit":{"message":"[Embeddable
Rebuild] [Controls] Clean up services + TODOs (#193180)\n\nPart of
https://github.com/elastic/kibana/issues/192005\r\nCloses
https://github.com/elastic/kibana/issues/167438\r\n\r\n##
Summary\r\n\r\n\r\n## Summary\r\n\r\nThis PR represents the second major
cleanup task for the control group\r\nembeddable refactor. The tasks
included in this PR can be loosely\r\nsummarized as follows:\r\n1. It
removes the old, cumbersome services implementation and replaces\r\nit
with a much simpler system (which is the same one used in the
`links`\r\n+ `presentation_panel` plugins).\r\n- This it the main reason
for the decrease in lines - the old system\r\nrequired a **huge** amount
of boilerplate code, which is no longer\r\nnecessary with the new method
for storing services.\r\n2. It addresses and/or removes any remaining
TODO comments in the\r\n`controls` plugin\r\n- This includes renaming
`ControlStyle` and `DEFAULT_CONTROL_STYLE` to\r\n`ControlLabelPosition`
and `DEFAULT_CONTROL_LABEL_POSITION`\r\nrespectively, which represents a
bulk of the changes.\r\n3. It moves all compatibility checks for all
control actions to be async\r\nimported.\r\n4. It removes the ability to
register controls from the `controls`\r\nplugin setup
contract.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"db5557429979b9a0f3420a50a06c7fd69cbdf5b2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193180","number":193180,"mergeCommit":{"message":"[Embeddable
Rebuild] [Controls] Clean up services + TODOs (#193180)\n\nPart of
https://github.com/elastic/kibana/issues/192005\r\nCloses
https://github.com/elastic/kibana/issues/167438\r\n\r\n##
Summary\r\n\r\n\r\n## Summary\r\n\r\nThis PR represents the second major
cleanup task for the control group\r\nembeddable refactor. The tasks
included in this PR can be loosely\r\nsummarized as follows:\r\n1. It
removes the old, cumbersome services implementation and replaces\r\nit
with a much simpler system (which is the same one used in the
`links`\r\n+ `presentation_panel` plugins).\r\n- This it the main reason
for the decrease in lines - the old system\r\nrequired a **huge** amount
of boilerplate code, which is no longer\r\nnecessary with the new method
for storing services.\r\n2. It addresses and/or removes any remaining
TODO comments in the\r\n`controls` plugin\r\n- This includes renaming
`ControlStyle` and `DEFAULT_CONTROL_STYLE` to\r\n`ControlLabelPosition`
and `DEFAULT_CONTROL_LABEL_POSITION`\r\nrespectively, which represents a
bulk of the changes.\r\n3. It moves all compatibility checks for all
control actions to be async\r\nimported.\r\n4. It removes the ability to
register controls from the `controls`\r\nplugin setup
contract.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"db5557429979b9a0f3420a50a06c7fd69cbdf5b2"}}]}]
BACKPORT-->

Co-authored-by: Hannah Mudge <[email protected]>
  • Loading branch information
kibanamachine and Heenawter authored Sep 19, 2024
1 parent b0853ca commit b2e6263
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 b2e6263

Please sign in to comment.