diff --git a/packages/kbn-es-query/src/filters/helpers/only_disabled.ts b/packages/kbn-es-query/src/filters/helpers/only_disabled.ts index d605d8b0be628..5a9ca76646eb4 100644 --- a/packages/kbn-es-query/src/filters/helpers/only_disabled.ts +++ b/packages/kbn-es-query/src/filters/helpers/only_disabled.ts @@ -8,6 +8,7 @@ import { filter } from 'lodash'; import type { Filter } from '..'; +import type { FilterCompareOptions } from './compare_filters'; import { compareFilters, COMPARE_ALL_OPTIONS } from './compare_filters'; const isEnabled = (f: Filter) => f && f.meta && !f.meta.disabled; @@ -18,10 +19,15 @@ const isEnabled = (f: Filter) => f && f.meta && !f.meta.disabled; * * @public */ -export const onlyDisabledFiltersChanged = (newFilters?: Filter[], oldFilters?: Filter[]) => { +export const onlyDisabledFiltersChanged = ( + newFilters?: Filter[], + oldFilters?: Filter[], + comparatorOptions?: FilterCompareOptions +) => { // If it's the same - compare only enabled filters const newEnabledFilters = filter(newFilters || [], isEnabled); const oldEnabledFilters = filter(oldFilters || [], isEnabled); + const options = comparatorOptions ?? COMPARE_ALL_OPTIONS; - return compareFilters(oldEnabledFilters, newEnabledFilters, COMPARE_ALL_OPTIONS); + return compareFilters(oldEnabledFilters, newEnabledFilters, options); }; 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 e4cc338f15dd6..c4886c55d976c 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 @@ -9,7 +9,12 @@ import fastIsEqual from 'fast-deep-equal'; import { persistableControlGroupInputIsEqual } from '@kbn/controls-plugin/common'; -import { compareFilters, COMPARE_ALL_OPTIONS, isFilterPinned } from '@kbn/es-query'; +import { + compareFilters, + COMPARE_ALL_OPTIONS, + isFilterPinned, + onlyDisabledFiltersChanged, +} from '@kbn/es-query'; import { DashboardContainer } from '../../dashboard_container'; import { DashboardContainerByValueInput } from '../../../../../common'; @@ -32,10 +37,11 @@ export type DashboardDiffFunctions = { export const isKeyEqual = async ( key: keyof DashboardContainerByValueInput, - diffFunctionProps: DiffFunctionProps + diffFunctionProps: DiffFunctionProps, + diffingFunctions: DashboardDiffFunctions ) => { const propsAsNever = diffFunctionProps as never; // todo figure out why props has conflicting types in some constituents. - const diffingFunction = dashboardDiffingFunctions[key]; + const diffingFunction = diffingFunctions[key]; if (diffingFunction) { return diffingFunction?.prototype?.name === 'AsyncFunction' ? await diffingFunction(propsAsNever) @@ -48,7 +54,7 @@ export const isKeyEqual = async ( * A collection of functions which diff individual keys of dashboard state. If a key is missing from this list it is * diffed by the default diffing function, fastIsEqual. */ -export const dashboardDiffingFunctions: DashboardDiffFunctions = { +export const unsavedChangesDiffingFunctions: DashboardDiffFunctions = { panels: async ({ currentValue, lastValue, container }) => { if (!getPanelLayoutsAreEqual(currentValue, lastValue)) return false; @@ -81,6 +87,7 @@ export const dashboardDiffingFunctions: DashboardDiffFunctions = { return await Promise.any(explicitInputComparePromises).catch(() => true); }, + // exclude pinned filters from comparision because pinned filters are not part of application state filters: ({ currentValue, lastValue }) => compareFilters( currentValue.filter((f) => !isFilterPinned(f)), @@ -109,3 +116,15 @@ export const dashboardDiffingFunctions: 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 }) => + onlyDisabledFiltersChanged(lastValue, currentValue, shouldRefreshFilterCompareOptions), +}; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.test.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.test.ts new file mode 100644 index 0000000000000..6c4e9c3989cd7 --- /dev/null +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.test.ts @@ -0,0 +1,63 @@ +/* + * 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 { buildExistsFilter, disableFilter, pinFilter, toggleFilterNegated } from '@kbn/es-query'; +import type { DataViewFieldBase, DataViewBase } from '@kbn/es-query'; +import { getShouldRefresh } from './dashboard_diffing_integration'; +import { DashboardContainer } from '../../dashboard_container'; +import { DashboardContainerByValueInput } from '../../../../../common'; + +describe('getShouldRefresh', () => { + const dashboardContainerMock = { + untilInitialized: () => {}, + } as unknown as DashboardContainer; + + const existsFilter = buildExistsFilter( + { + name: 'myFieldName', + } as DataViewFieldBase, + { + id: 'myDataViewId', + } as DataViewBase + ); + + test('should return true when pinned filters change', async () => { + const pinnedFilter = pinFilter(existsFilter); + const lastInput = { + filters: [pinnedFilter], + } as unknown as DashboardContainerByValueInput; + const input = { + filters: [toggleFilterNegated(pinnedFilter)], + } as unknown as DashboardContainerByValueInput; + expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, { ...lastInput })).toBe( + false + ); + expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true); + }); + + test('should return false when disabled filters change', async () => { + const disabledFilter = disableFilter(existsFilter); + const lastInput = { + filters: [disabledFilter], + } as unknown as DashboardContainerByValueInput; + const input = { + filters: [toggleFilterNegated(disabledFilter)], + } as unknown as DashboardContainerByValueInput; + expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false); + }); + + test('should return false when pinned filter changes to unpinned', async () => { + const lastInput = { + filters: [existsFilter], + } as unknown as DashboardContainerByValueInput; + const input = { + filters: [pinFilter(existsFilter)], + } as unknown as DashboardContainerByValueInput; + expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false); + }); +}); diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.ts index 32956cea18d14..2062ac3a4b620 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.ts @@ -13,7 +13,12 @@ import { DashboardContainer } from '../../dashboard_container'; import { pluginServices } from '../../../../services/plugin_services'; import { DashboardContainerByValueInput } from '../../../../../common'; import { CHANGE_CHECK_DEBOUNCE } from '../../../../dashboard_constants'; -import { isKeyEqual } from './dashboard_diffing_functions'; +import type { DashboardDiffFunctions } from './dashboard_diffing_functions'; +import { + isKeyEqual, + shouldRefreshDiffingFunctions, + unsavedChangesDiffingFunctions, +} from './dashboard_diffing_functions'; import { dashboardContainerReducers } from '../../../state/dashboard_container_reducers'; /** @@ -54,6 +59,23 @@ export const keysNotConsideredUnsavedChanges: Array = [ + 'query', + 'filters', + 'timeRange', + 'timeslice', + 'timeRestore', + 'lastReloadRequestTime', + + // also refetch when chart settings change + 'syncColors', + 'syncCursor', + 'syncTooltips', +]; + /** * Does an initial diff between @param initialInput and @param initialLastSavedInput, and created a middleware * which listens to the redux store and checks for & publishes the unsaved changes on dispatches. @@ -139,44 +161,67 @@ function backupUnsavedChanges( } /** - * Does a shallow diff between @param lastExplicitInput and @param currentExplicitInput and + * Does a shallow diff between @param lastInput and @param input and * @returns an object out of the keys which are different. */ export async function getUnsavedChanges( this: DashboardContainer, lastInput: DashboardContainerByValueInput, + input: DashboardContainerByValueInput +): Promise> { + const allKeys = [...new Set([...Object.keys(lastInput), ...Object.keys(input)])] as Array< + keyof DashboardContainerByValueInput + >; + return await getInputChanges(this, lastInput, input, allKeys, unsavedChangesDiffingFunctions); +} + +export async function getShouldRefresh( + this: DashboardContainer, + lastInput: DashboardContainerByValueInput, + input: DashboardContainerByValueInput +): Promise { + const inputChanges = await getInputChanges( + this, + lastInput, + input, + refetchKeys, + shouldRefreshDiffingFunctions + ); + return Object.keys(inputChanges).length > 0; +} + +async function getInputChanges( + container: DashboardContainer, + lastInput: DashboardContainerByValueInput, input: DashboardContainerByValueInput, - keys?: Array + keys: Array, + diffingFunctions: DashboardDiffFunctions ): Promise> { - await this.untilInitialized(); - const allKeys = - keys ?? - ([...new Set([...Object.keys(lastInput), ...Object.keys(input)])] as Array< - keyof DashboardContainerByValueInput - >); - const keyComparePromises = allKeys.map( + await container.untilInitialized(); + const keyComparePromises = keys.map( (key) => new Promise<{ key: keyof DashboardContainerByValueInput; isEqual: boolean }>((resolve) => - isKeyEqual(key, { - container: this, - - currentValue: input[key], - currentInput: input, - - lastValue: lastInput[key], - lastInput, - }).then((isEqual) => resolve({ key, isEqual })) + isKeyEqual( + key, + { + container, + + currentValue: input[key], + currentInput: input, + + lastValue: lastInput[key], + lastInput, + }, + diffingFunctions + ).then((isEqual) => resolve({ key, isEqual })) ) ); - const unsavedChanges = (await Promise.allSettled(keyComparePromises)).reduce( - (changes, current) => { - if (current.status === 'fulfilled') { - const { key, isEqual } = current.value; - if (!isEqual) (changes as { [key: string]: unknown })[key] = input[key]; - } - return changes; - }, - {} as Partial - ); - return unsavedChanges; + const inputChanges = (await Promise.allSettled(keyComparePromises)).reduce((changes, current) => { + if (current.status === 'fulfilled') { + const { key, isEqual } = current.value; + if (!isEqual) (changes as { [key: string]: unknown })[key] = input[key]; + } + return changes; + }, {} as Partial); + return inputChanges; } diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/search_sessions/start_dashboard_search_session_integration.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/search_sessions/start_dashboard_search_session_integration.ts index d8e1aadf0ed4a..986a53a022c6a 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/search_sessions/start_dashboard_search_session_integration.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/search_sessions/start_dashboard_search_session_integration.ts @@ -15,24 +15,7 @@ import { pluginServices } from '../../../../services/plugin_services'; import { DashboardContainerByValueInput } from '../../../../../common'; import { CHANGE_CHECK_DEBOUNCE } from '../../../../dashboard_constants'; import { DashboardCreationOptions } from '../../dashboard_container_factory'; -import { getUnsavedChanges } from '../diff_state/dashboard_diffing_integration'; - -/** - * input keys that will cause a new session to be created. - */ -const refetchKeys: Array = [ - 'query', - 'filters', - 'timeRange', - 'timeslice', - 'timeRestore', - 'lastReloadRequestTime', - - // also refetch when chart settings change - 'syncColors', - 'syncCursor', - 'syncTooltips', -]; +import { getShouldRefresh } from '../diff_state/dashboard_diffing_integration'; /** * Enables dashboard search sessions. @@ -96,8 +79,7 @@ export function startDashboardSearchSessionIntegration( .pipe(pairwise(), debounceTime(CHANGE_CHECK_DEBOUNCE)) .subscribe(async (states) => { const [previous, current] = states as DashboardContainerByValueInput[]; - const changes = await getUnsavedChanges.bind(this)(previous, current, refetchKeys); - const shouldRefetch = Object.keys(changes).length > 0; + const shouldRefetch = await getShouldRefresh.bind(this)(previous, current); if (!shouldRefetch) return; const {