Skip to content

Commit

Permalink
[embeddable] decouple FILTER_BY_MAP_EXTENT action from Embeddable fra…
Browse files Browse the repository at this point in the history
…mework (#175262)

Part of #175138 and prerequisite
for #174960

PR decouples FILTER_BY_MAP_EXTENT action from Embeddable framework by
migrating to sets of composable interfaces.

### Inheritance to composition

Replace action context type with a type definition that is as narrow as
possible. How do you know which properties are needed? Look through the
action and find all usages of `embeddabe`. Add each usage to the
narrowed type definition.
```
// original action context type.
// Notice how type requires an Embeddable instance 
// when only a few properties from the embeddable are needed
export interface FilterByMapExtentActionContext {
  embeddable: Embeddable<FilterByMapExtentInput>;
}

// new action context type. 
// Notice how type explicitly states what is used and 
// is no longer tied to an Embeddable (even though an Embeddable will pass the type guard)
// Another point to highlight is use of 'Partial'. This indicates that API interfaces are not required
export type FilterByMapExtentActionApi = HasType &
  Partial<HasDisableTriggers & HasParentApi<HasType> & HasVisualizeConfig>;

// This is the new action context rewritten to inline types like HasType to make it clearer 
export type FilterByMapExtentActionApi = {
  type: string;
  disableTriggers?: boolean;
  parentApi?: {
    type: string
  };
  getVis?: () => Vis<VisParam>;
}
```

### Migrating embeddable.parent?.type
Action uses `embeddable.parent?.type` to customize the display name. For
dashboards the display name would be "Filter dashboards by map bounds",
otherwise the display name would be "Filter page by map bounds".

To access `parentApi`, we must:

1) `HasParentApi` interface and `apiHasParentApi` type guard already
exist at
`packages/presentation/presentation_publishing/interfaces/has_parent_api.ts`,
so we can just use those.
2) No compatibility changes are required since Embeddable already
exposes `parentApi`.
3) Add `Partial<HasParentApi<HasType>>` to action compatibility
definition. Using `Partial` because accessing `parentApi` is not
required.
4) Access value like `api.parentApi?.type`

### Migrating embeddable.getInput().disableTriggers
Action fails compatibility check when
`embeddable.getInput().disableTriggers` returns `true`.

To access `disableTriggers`, we must:

1) Define `HasDisableTriggers` interface and `apiHasDisableTriggers`
type guard in the package
`packages/presentation/presentation_publishing`. Adding interface and
type guard to generic package folder since `input.disableTriggers` is
not tied to any specific Embeddable implementation. *Naming convention:
Use prefix `has` since `disableTriggers` is static. Use prefix
`publishes` for dynamic values since value will be accesses via a
subscription.*
2) Add compatibility changes to
`src/plugins/embeddable/public/lib/embeddables/embeddable.tsx` so that
an Embeddable instance exposes `disableTriggers`.
3) Add `Partial<HasDisableTriggers>` to action compatibility definition.
Using `Partial` because accessing `disableTriggers` is not required.
4) Access value like `api.disableTriggers`

### Migrating embeddable.getVis()
Action uses `embeddable.getVis()` to pass compatibility check when
visualize embeddable is a region_map or tile_map

To access `getVis`, we must:

1) Define `HasVisualizeConfig` interface and `apiHasVisualizeConfig`
type guard in `src/plugins/visualizations/public/embeddable/interfaces`.
Adding interface to the visualize plugin since `getVis` is only
available for Visualization Embeddable.
2) No compatibility changes are required since Visualization Embeddable
already exposes `getVis` method.
3) Add `Partial<HasVisualizeConfig>` to action compatibility definition.
Using `Partial` because accessing `getVis` is not required.
4) Access value like `api.getVis()`

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
nreese and kibanamachine authored Jan 25, 2024
1 parent 2a27b83 commit 4565b1b
Show file tree
Hide file tree
Showing 13 changed files with 88 additions and 45 deletions.
1 change: 1 addition & 0 deletions packages/presentation/presentation_publishing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export {
type PublishesSavedObjectId,
} from './interfaces/publishes_saved_object_id';
export { apiHasUniqueId, type HasUniqueId } from './interfaces/has_uuid';
export { apiHasDisableTriggers, type HasDisableTriggers } from './interfaces/has_disable_triggers';
export {
apiPublishesViewMode,
apiPublishesWritableViewMode,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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 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 or the Server
* Side Public License, v 1.
*/

export interface HasDisableTriggers {
disableTriggers: boolean;
}

export const apiHasDisableTriggers = (api: unknown | null): api is HasDisableTriggers => {
return Boolean(api && typeof (api as HasDisableTriggers).disableTriggers === 'boolean');
};
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export const legacyEmbeddableToApi = (

const uuid = embeddable.id;
const parentApi = embeddable.parent;
const disableTriggers = embeddable.getInput().disableTriggers;

/**
* We treat all legacy embeddable types as if they can support local unified search state, because there is no programmatic way
Expand Down Expand Up @@ -201,6 +202,7 @@ export const legacyEmbeddableToApi = (
api: {
parentApi: parentApi as LegacyEmbeddableAPI['parentApi'],
uuid,
disableTriggers: disableTriggers ?? false,
viewMode,
dataLoading,
blockingError,
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export abstract class Embeddable<
this.destroyAPI = destroyAPI;
({
uuid: this.uuid,
disableTriggers: this.disableTriggers,
onEdit: this.onEdit,
viewMode: this.viewMode,
dataViews: this.dataViews,
Expand Down Expand Up @@ -157,6 +158,7 @@ export abstract class Embeddable<
*/
private destroyAPI;
public uuid: LegacyEmbeddableAPI['uuid'];
public disableTriggers: LegacyEmbeddableAPI['disableTriggers'];
public onEdit: LegacyEmbeddableAPI['onEdit'];
public viewMode: LegacyEmbeddableAPI['viewMode'];
public parentApi: LegacyEmbeddableAPI['parentApi'];
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { DefaultPresentationPanelApi } from '@kbn/presentation-panel-plugin/publ
import {
HasEditCapabilities,
HasType,
HasDisableTriggers,
PublishesBlockingError,
PublishesDataLoading,
PublishesDataViews,
Expand Down Expand Up @@ -39,6 +40,7 @@ export type { EmbeddableInput };
*/
export type LegacyEmbeddableAPI = HasType &
HasUniqueId &
HasDisableTriggers &
PublishesPhaseEvents &
PublishesViewMode &
PublishesDataViews &
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/embeddable/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ export { VIS_EVENT_TO_TRIGGER } from './events';
export { createVisEmbeddableFromObject } from './create_vis_embeddable_from_object';

export type { VisualizeEmbeddable, VisualizeInput } from './visualize_embeddable';

export { type HasVisualizeConfig, apiHasVisualizeConfig } from './interfaces/has_visualize_config';
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* 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 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 or the Server
* Side Public License, v 1.
*/

import { VisParams } from '../../types';
import Vis from '../../vis';

export interface HasVisualizeConfig {
getVis: () => Vis<VisParams>;
}

export const apiHasVisualizeConfig = (api: unknown): api is HasVisualizeConfig => {
return Boolean(api && typeof (api as HasVisualizeConfig).getVis === 'function');
};
9 changes: 6 additions & 3 deletions src/plugins/visualizations/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ export function plugin(initializerContext: PluginInitializerContext) {

/** @public static code */
export { TypesService } from './vis_types/types_service';
export { VISUALIZE_EMBEDDABLE_TYPE, VIS_EVENT_TO_TRIGGER } from './embeddable';
export {
apiHasVisualizeConfig,
VISUALIZE_EMBEDDABLE_TYPE,
VIS_EVENT_TO_TRIGGER,
} from './embeddable';
export { VisualizationContainer } from './components';
export { getVisSchemas } from './vis_schemas';

Expand All @@ -36,8 +40,7 @@ export type {
export type { Vis, SerializedVis, SerializedVisData, VisData } from './vis';
export type VisualizeEmbeddableFactoryContract = PublicContract<VisualizeEmbeddableFactory>;
export type VisualizeEmbeddableContract = PublicContract<VisualizeEmbeddable>;
export type { VisualizeInput } from './embeddable';
export type { VisualizeEmbeddable } from './embeddable';
export type { VisualizeInput, VisualizeEmbeddable, HasVisualizeConfig } from './embeddable';
export type { SchemaConfig } from '../common/types';
export { updateOldState } from './legacy/vis_update_state';
export type { PersistedState } from './persisted_state';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
* 2.0.
*/

import type { HasType } from '@kbn/presentation-publishing';
import { Embeddable } from '@kbn/embeddable-plugin/public';
import type { VisualizeEmbeddable } from '@kbn/visualizations-plugin/public';
import type { HasVisualizeConfig, VisualizeEmbeddable } from '@kbn/visualizations-plugin/public';
import { apiHasVisualizeConfig } from '@kbn/visualizations-plugin/public';

export function isLegacyMap(embeddable: Embeddable) {
return (
Expand All @@ -15,3 +17,12 @@ export function isLegacyMap(embeddable: Embeddable) {
['region_map', 'tile_map'].includes((embeddable as VisualizeEmbeddable).getVis()?.type?.name)
);
}

type LegacyMapApi = HasType & Partial<HasVisualizeConfig>;

export function isLegacyMapApi(api: LegacyMapApi) {
if (api.type !== 'visualization' || !apiHasVisualizeConfig(api)) {
return false;
}
return ['region_map', 'tile_map'].includes(api.getVis().type?.name);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@
*/

import { i18n } from '@kbn/i18n';
import type { Embeddable } from '@kbn/embeddable-plugin/public';
import { type EmbeddableApiContext, apiHasType } from '@kbn/presentation-publishing';
import { createAction } from '@kbn/ui-actions-plugin/public';
import type { FilterByMapExtentActionContext, FilterByMapExtentInput } from './types';
import { type FilterByMapExtentActionApi } from './types';
import { MAP_SAVED_OBJECT_TYPE } from '../../../common/constants';
import { isLegacyMapApi } from '../../legacy_visualizations/is_legacy_map';

export const FILTER_BY_MAP_EXTENT = 'FILTER_BY_MAP_EXTENT';

function getContainerLabel(embeddable: Embeddable<FilterByMapExtentInput>) {
return embeddable.parent?.type === 'dashboard'
export const isApiCompatible = (api: unknown | null): api is FilterByMapExtentActionApi =>
Boolean(apiHasType(api));

function getContainerLabel(api: FilterByMapExtentActionApi | unknown) {
return isApiCompatible(api) && api.parentApi?.type === 'dashboard'
? i18n.translate('xpack.maps.filterByMapExtentMenuItem.dashboardLabel', {
defaultMessage: 'dashboard',
})
Expand All @@ -22,36 +27,38 @@ function getContainerLabel(embeddable: Embeddable<FilterByMapExtentInput>) {
});
}

function getDisplayName(embeddable: Embeddable<FilterByMapExtentInput>) {
function getDisplayName(api: FilterByMapExtentActionApi | unknown) {
return i18n.translate('xpack.maps.filterByMapExtentMenuItem.displayName', {
defaultMessage: 'Filter {containerLabel} by map bounds',
values: { containerLabel: getContainerLabel(embeddable) },
values: { containerLabel: getContainerLabel(api) },
});
}

export const filterByMapExtentAction = createAction<FilterByMapExtentActionContext>({
export const filterByMapExtentAction = createAction<EmbeddableApiContext>({
id: FILTER_BY_MAP_EXTENT,
type: FILTER_BY_MAP_EXTENT,
order: 20,
getDisplayName: (context: FilterByMapExtentActionContext) => {
return getDisplayName(context.embeddable);
getDisplayName: ({ embeddable }: EmbeddableApiContext) => {
return getDisplayName(embeddable);
},
getDisplayNameTooltip: (context: FilterByMapExtentActionContext) => {
getDisplayNameTooltip: ({ embeddable }: EmbeddableApiContext) => {
return i18n.translate('xpack.maps.filterByMapExtentMenuItem.displayNameTooltip', {
defaultMessage:
'As you zoom and pan the map, the {containerLabel} updates to display only the data visible in the map bounds.',
values: { containerLabel: getContainerLabel(context.embeddable) },
values: { containerLabel: getContainerLabel(embeddable) },
});
},
getIconType: () => {
return 'filter';
},
isCompatible: async (context: FilterByMapExtentActionContext) => {
const { isCompatible } = await import('./is_compatible');
return isCompatible(context);
isCompatible: async ({ embeddable }: EmbeddableApiContext) => {
if (!isApiCompatible(embeddable)) return false;
return embeddable.disableTriggers
? false
: embeddable.type === MAP_SAVED_OBJECT_TYPE || isLegacyMapApi(embeddable);
},
execute: async (context: FilterByMapExtentActionContext) => {
execute: async ({ embeddable }: EmbeddableApiContext) => {
const { openModal } = await import('./modal');
openModal(getDisplayName(context.embeddable));
openModal(getDisplayName(embeddable));
},
});

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,8 @@
* 2.0.
*/

import type { Embeddable, EmbeddableInput } from '@kbn/embeddable-plugin/public';
import type { HasDisableTriggers, HasParentApi, HasType } from '@kbn/presentation-publishing';
import type { HasVisualizeConfig } from '@kbn/visualizations-plugin/public';

export interface FilterByMapExtentInput extends EmbeddableInput {
filterByMapExtent: boolean;
}

export interface FilterByMapExtentActionContext {
embeddable: Embeddable<FilterByMapExtentInput>;
}
export type FilterByMapExtentActionApi = HasType &
Partial<HasDisableTriggers & HasParentApi<HasType> & HasVisualizeConfig>;
1 change: 1 addition & 0 deletions x-pack/plugins/maps/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
"@kbn/es-types",
"@kbn/data-service",
"@kbn/code-editor",
"@kbn/presentation-publishing",
],
"exclude": [
"target/**/*",
Expand Down

0 comments on commit 4565b1b

Please sign in to comment.