diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts index c4886c55d976c..74f852df484e4 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts @@ -15,6 +15,7 @@ import { isFilterPinned, onlyDisabledFiltersChanged, } from '@kbn/es-query'; +import { shouldRefreshFilterCompareOptions } from '@kbn/embeddable-plugin/public'; import { DashboardContainer } from '../../dashboard_container'; import { DashboardContainerByValueInput } from '../../../../../common'; @@ -117,12 +118,6 @@ export const unsavedChangesDiffingFunctions: DashboardDiffFunctions = { viewMode: () => false, // When compared view mode is always considered unequal so that it gets backed up. }; -const shouldRefreshFilterCompareOptions = { - ...COMPARE_ALL_OPTIONS, - // do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results) - state: false, -}; - export const shouldRefreshDiffingFunctions: DashboardDiffFunctions = { ...unsavedChangesDiffingFunctions, filters: ({ currentValue, lastValue }) => diff --git a/src/plugins/embeddable/public/index.ts b/src/plugins/embeddable/public/index.ts index ff41aa5503e33..18e5484b8fa4d 100644 --- a/src/plugins/embeddable/public/index.ts +++ b/src/plugins/embeddable/public/index.ts @@ -87,6 +87,8 @@ export { EmbeddableRenderer, useEmbeddableFactory, isFilterableEmbeddable, + shouldFetch$, + shouldRefreshFilterCompareOptions, } from './lib'; export { AttributeService, ATTRIBUTE_SERVICE_KEY } from './lib/attribute_service'; diff --git a/src/plugins/embeddable/public/lib/filterable_embeddable/index.ts b/src/plugins/embeddable/public/lib/filterable_embeddable/index.ts index ffa9470a34af4..0a01b6cab39df 100644 --- a/src/plugins/embeddable/public/lib/filterable_embeddable/index.ts +++ b/src/plugins/embeddable/public/lib/filterable_embeddable/index.ts @@ -8,3 +8,4 @@ export type { FilterableEmbeddable } from './types'; export { isFilterableEmbeddable } from './types'; +export { shouldFetch$, shouldRefreshFilterCompareOptions } from './should_fetch'; diff --git a/src/plugins/embeddable/public/lib/filterable_embeddable/should_fetch.tsx b/src/plugins/embeddable/public/lib/filterable_embeddable/should_fetch.tsx new file mode 100644 index 0000000000000..cf9cba103e011 --- /dev/null +++ b/src/plugins/embeddable/public/lib/filterable_embeddable/should_fetch.tsx @@ -0,0 +1,44 @@ +/* + * 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 fastIsEqual from 'fast-deep-equal'; +import { Observable } from 'rxjs'; +import { map, distinctUntilChanged, skip, startWith } from 'rxjs/operators'; +import { COMPARE_ALL_OPTIONS, onlyDisabledFiltersChanged } from '@kbn/es-query'; +import type { FilterableEmbeddableInput } from './types'; + +export const shouldRefreshFilterCompareOptions = { + ...COMPARE_ALL_OPTIONS, + // do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results) + state: false, +}; + +export function shouldFetch$< + TFilterableEmbeddableInput extends FilterableEmbeddableInput = FilterableEmbeddableInput +>( + updated$: Observable, + getInput: () => TFilterableEmbeddableInput +): Observable { + return updated$.pipe(map(() => getInput())).pipe( + // wrapping distinctUntilChanged with startWith and skip to prime distinctUntilChanged with an initial input value. + startWith(getInput()), + distinctUntilChanged((a: TFilterableEmbeddableInput, b: TFilterableEmbeddableInput) => { + if ( + !fastIsEqual( + [a.searchSessionId, a.query, a.timeRange, a.timeslice], + [b.searchSessionId, b.query, b.timeRange, b.timeslice] + ) + ) { + return false; + } + + return onlyDisabledFiltersChanged(a.filters, b.filters, shouldRefreshFilterCompareOptions); + }), + skip(1) + ); +} diff --git a/src/plugins/embeddable/public/lib/filterable_embeddable/types.ts b/src/plugins/embeddable/public/lib/filterable_embeddable/types.ts index 8fe2b85e02ada..2b6182b1b95db 100644 --- a/src/plugins/embeddable/public/lib/filterable_embeddable/types.ts +++ b/src/plugins/embeddable/public/lib/filterable_embeddable/types.ts @@ -6,7 +6,15 @@ * Side Public License, v 1. */ -import { type AggregateQuery, type Filter, type Query } from '@kbn/es-query'; +import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; +import { EmbeddableInput } from '../embeddables'; + +export type FilterableEmbeddableInput = EmbeddableInput & { + filters?: Filter[]; + query?: Query; + timeRange?: TimeRange; + timeslice?: [number, number]; +}; /** * All embeddables that implement this interface should support being filtered diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 39bc64adef207..edb3d82b09e13 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -55,6 +55,7 @@ import { cellValueTrigger, CELL_VALUE_TRIGGER, type CellValueContext, + shouldFetch$, } from '@kbn/embeddable-plugin/public'; import type { Action, UiActionsStart } from '@kbn/ui-actions-plugin/public'; import type { DataViewsContract, DataView } from '@kbn/data-views-plugin/public'; @@ -501,20 +502,11 @@ export class Embeddable // Update search context and reload on changes related to search this.inputReloadSubscriptions.push( - this.getUpdated$() - .pipe(map(() => this.getInput())) - .pipe( - distinctUntilChanged((a, b) => - fastIsEqual( - [a.filters, a.query, a.timeRange, a.searchSessionId], - [b.filters, b.query, b.timeRange, b.searchSessionId] - ) - ), - skip(1) - ) - .subscribe(async (input) => { + shouldFetch$(this.getUpdated$(), () => this.getInput()).subscribe( + (input) => { this.onContainerStateChanged(input); - }) + } + ) ); } diff --git a/x-pack/plugins/maps/public/embeddable/map_embeddable.test.tsx b/x-pack/plugins/maps/public/embeddable/map_embeddable.test.tsx new file mode 100644 index 0000000000000..bcb5aea3cca85 --- /dev/null +++ b/x-pack/plugins/maps/public/embeddable/map_embeddable.test.tsx @@ -0,0 +1,290 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { v4 as uuidv4 } from 'uuid'; +import { getControlledBy, MapEmbeddable } from './map_embeddable'; +import { buildExistsFilter, disableFilter, pinFilter, toggleFilterNegated } from '@kbn/es-query'; +import type { DataViewFieldBase, DataViewBase } from '@kbn/es-query'; +import { MapEmbeddableConfig, MapEmbeddableInput } from './types'; +import { MapSavedObjectAttributes } from '../../common/map_saved_object_type'; + +jest.mock('../kibana_services', () => { + return { + getHttp() { + return { + basePath: { + prepend: (url: string) => url, + }, + }; + }, + getMapsCapabilities() { + return { save: true }; + }, + getSearchService() { + return { + session: { + getSearchOptions() { + return undefined; + }, + }, + }; + }, + getShowMapsInspectorAdapter() { + return false; + }, + getTimeFilter() { + return { + getTime() { + return { from: 'now-7d', to: 'now' }; + }, + }; + }, + }; +}); + +jest.mock('../connected_components/map_container', () => { + return { + MapContainer: () => { + return
mockLayerTOC
; + }, + }; +}); + +jest.mock('../routes/map_page', () => { + class MockSavedMap { + // eslint-disable-next-line @typescript-eslint/no-var-requires + private _store = require('../reducers/store').createMapStore(); + private _attributes: MapSavedObjectAttributes = { + title: 'myMap', + }; + + whenReady = async function () {}; + + getStore() { + return this._store; + } + getAttributes() { + return this._attributes; + } + getAutoFitToBounds() { + return true; + } + getSharingSavedObjectProps() { + return null; + } + } + return { SavedMap: MockSavedMap }; +}); + +function untilInitialized(mapEmbeddable: MapEmbeddable): Promise { + return new Promise((resolve) => { + // @ts-expect-error setInitializationFinished is protected but we are overriding it to know when embeddable is initialized + mapEmbeddable.setInitializationFinished = () => { + resolve(); + }; + }); +} + +function onNextTick(): Promise { + // wait one tick to give observables time to fire + return new Promise((resolve) => setTimeout(resolve, 0)); +} + +describe('shouldFetch$', () => { + test('should not fetch when search context does not change', async () => { + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: 'map1', + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + title: 'updated map title', + }); + + await onNextTick(); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + describe('on filters change', () => { + test('should fetch on filter change', async () => { + const existsFilter = buildExistsFilter( + { + name: 'myFieldName', + } as DataViewFieldBase, + { + id: 'myDataViewId', + } as DataViewBase + ); + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: 'map1', + filters: [existsFilter], + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + filters: [toggleFilterNegated(existsFilter)], + }); + + await onNextTick(); + + expect(fetchSpy).toHaveBeenCalled(); + }); + + test('should not fetch on disabled filter change', async () => { + const disabledFilter = disableFilter( + buildExistsFilter( + { + name: 'myFieldName', + } as DataViewFieldBase, + { + id: 'myDataViewId', + } as DataViewBase + ) + ); + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: 'map1', + filters: [disabledFilter], + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + filters: [toggleFilterNegated(disabledFilter)], + }); + + await onNextTick(); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + test('should not fetch when unpinned filter is pinned', async () => { + const unpinnedFilter = buildExistsFilter( + { + name: 'myFieldName', + } as DataViewFieldBase, + { + id: 'myDataViewId', + } as DataViewBase + ); + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: 'map1', + filters: [unpinnedFilter], + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + filters: [pinFilter(unpinnedFilter)], + }); + + await onNextTick(); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + test('should not fetch on filter controlled by map embeddable change', async () => { + const embeddableId = 'map1'; + const filter = buildExistsFilter( + { + name: 'myFieldName', + } as DataViewFieldBase, + { + id: 'myDataViewId', + } as DataViewBase + ); + const controlledByFilter = { + ...filter, + meta: { + ...filter.meta, + controlledBy: getControlledBy(embeddableId), + }, + }; + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: embeddableId, + filters: [controlledByFilter], + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + filters: [toggleFilterNegated(controlledByFilter)], + }); + + await onNextTick(); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + }); + + describe('on searchSessionId change', () => { + test('should fetch when filterByMapExtent is false', async () => { + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: 'map1', + filterByMapExtent: false, + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + searchSessionId: uuidv4(), + }); + + await onNextTick(); + + expect(fetchSpy).toHaveBeenCalled(); + }); + + test('should not fetch when filterByMapExtent is true', async () => { + const mapEmbeddable = new MapEmbeddable( + {} as unknown as MapEmbeddableConfig, + { + id: 'map1', + filterByMapExtent: true, + } as unknown as MapEmbeddableInput + ); + await untilInitialized(mapEmbeddable); + + const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery'); + + mapEmbeddable.updateInput({ + searchSessionId: uuidv4(), + }); + + await onNextTick(); + + expect(fetchSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx b/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx index 51b806cec5dce..41579d4f5375d 100644 --- a/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx +++ b/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx @@ -14,7 +14,7 @@ import { render, unmountComponentAtNode } from 'react-dom'; import { Subscription } from 'rxjs'; import { Unsubscribe } from 'redux'; import { EuiEmptyPrompt } from '@elastic/eui'; -import { type Filter, compareFilters, type TimeRange, type Query } from '@kbn/es-query'; +import { type Filter } from '@kbn/es-query'; import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; import { Embeddable, @@ -24,6 +24,7 @@ import { VALUE_CLICK_TRIGGER, omitGenericEmbeddableInput, FilterableEmbeddable, + shouldFetch$, } from '@kbn/embeddable-plugin/public'; import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public'; import { APPLY_FILTER_TRIGGER } from '@kbn/data-plugin/public'; @@ -104,6 +105,10 @@ function getIsRestore(searchSessionId?: string) { return searchSessionOptions ? searchSessionOptions.isRestore : false; } +export function getControlledBy(id: string) { + return `mapEmbeddablePanel${id}`; +} + export class MapEmbeddable extends Embeddable implements ReferenceOrValueEmbeddable, FilterableEmbeddable @@ -114,15 +119,10 @@ export class MapEmbeddable private _isActive: boolean; private _savedMap: SavedMap; private _renderTooltipContent?: RenderToolTipContent; - private _subscription: Subscription; + private _subscriptions: Subscription[] = []; private _prevIsRestore: boolean = false; private _prevMapExtent?: MapExtent; - private _prevTimeRange?: TimeRange; - private _prevTimeslice?: [number, number]; - private _prevQuery?: Query; - private _prevFilters: Filter[] = []; private _prevSyncColors?: boolean; - private _prevSearchSessionId?: string; private _domNode?: HTMLElement; private _unsubscribeFromStore?: Unsubscribe; private _isInitialized = false; @@ -145,8 +145,8 @@ export class MapEmbeddable this._isActive = true; this._savedMap = new SavedMap({ mapEmbeddableInput: initialInput }); this._initializeSaveMap(); - this._subscription = this.getUpdated$().subscribe(() => this.onUpdate()); - this._controlledBy = `mapEmbeddablePanel${this.id}`; + this._subscriptions.push(this.getUpdated$().subscribe(() => this.onUpdate())); + this._controlledBy = getControlledBy(this.id); } public reportsEmbeddableLoad() { @@ -193,9 +193,20 @@ export class MapEmbeddable // Passing callback into redux store instead of regular pattern of getting redux state changes for performance reasons store.dispatch(setOnMapMove(this._propogateMapMovement)); - this._dispatchSetQuery({ - forceRefresh: false, - }); + this._dispatchSetQuery({ forceRefresh: false }); + this._subscriptions.push( + shouldFetch$(this.getUpdated$(), () => { + return { + ...this.getInput(), + filters: this._getInputFilters(), + searchSessionId: this._getSearchSessionId(), + }; + }).subscribe(() => { + this._dispatchSetQuery({ + forceRefresh: false, + }); + }) + ); const mapStateJSON = this._savedMap.getAttributes().mapStateJSON; if (mapStateJSON) { @@ -309,18 +320,6 @@ export class MapEmbeddable } onUpdate() { - if ( - !_.isEqual(this.input.timeRange, this._prevTimeRange) || - !_.isEqual(this.input.timeslice, this._prevTimeslice) || - !_.isEqual(this.input.query, this._prevQuery) || - !compareFilters(this._getFilters(), this._prevFilters) || - this._getSearchSessionId() !== this._prevSearchSessionId - ) { - this._dispatchSetQuery({ - forceRefresh: false, - }); - } - if (this.input.syncColors !== this._prevSyncColors) { this._dispatchSetChartsPaletteServiceGetColor(this.input.syncColors); } @@ -382,7 +381,7 @@ export class MapEmbeddable } }; - _getFilters() { + _getInputFilters() { return this.input.filters ? this.input.filters.filter( (filter) => !filter.meta.disabled && filter.meta.controlledBy !== this._controlledBy @@ -401,15 +400,9 @@ export class MapEmbeddable } _dispatchSetQuery({ forceRefresh }: { forceRefresh: boolean }) { - const filters = this._getFilters(); - this._prevTimeRange = this.input.timeRange; - this._prevTimeslice = this.input.timeslice; - this._prevQuery = this.input.query; - this._prevFilters = filters; - this._prevSearchSessionId = this._getSearchSessionId(); this._savedMap.getStore().dispatch( setQuery({ - filters, + filters: this._getInputFilters(), query: this.input.query, timeFilters: this.input.timeRange, timeslice: this.input.timeslice @@ -675,9 +668,9 @@ export class MapEmbeddable unmountComponentAtNode(this._domNode); } - if (this._subscription) { - this._subscription.unsubscribe(); - } + this._subscriptions.forEach((subscription) => { + subscription.unsubscribe(); + }); } reload() { diff --git a/x-pack/plugins/maps/public/embeddable/types.ts b/x-pack/plugins/maps/public/embeddable/types.ts index d024eee88ae6d..1d29a23ec44c5 100644 --- a/x-pack/plugins/maps/public/embeddable/types.ts +++ b/x-pack/plugins/maps/public/embeddable/types.ts @@ -5,7 +5,6 @@ * 2.0. */ -import type { Filter } from '@kbn/es-query'; import type { DataView } from '@kbn/data-plugin/common'; import { Embeddable, @@ -13,7 +12,7 @@ import { EmbeddableOutput, SavedObjectEmbeddableInput, } from '@kbn/embeddable-plugin/public'; -import type { Query, TimeRange } from '@kbn/es-query'; +import type { Filter, Query, TimeRange } from '@kbn/es-query'; import { MapCenterAndZoom, MapExtent, MapSettings } from '../../common/descriptor_types'; import { MapSavedObjectAttributes } from '../../common/map_saved_object_type';