[8.7] [Dashboard] fix searchSessionId not updated when pinned filter changes (#151390) (#151742)

# Backport This will backport the following commits from `main` to `8.7`: - [[Dashboard] fix searchSessionId not updated when pinned filter changes (#151390)](https://github.com/elastic/kibana/pull/151390)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
2025-04-23 17:28:26 -04:00 · 2023-02-21 12:39:00 -05:00 · 2023-02-21 12:39:00 -05:00 · 748dfcb927
commit 748dfcb927
parent 5aa2bdf16c
5 changed files with 170 additions and 55 deletions
--- 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);
 };
--- 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<typeof key>
+  diffFunctionProps: DiffFunctionProps<typeof key>,
+  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),
+};
--- 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
@ -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);
+  });
+});
--- 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<keyof DashboardContainerByVa
  'viewMode',
 ];

+/**
+ * input keys that will cause a new session to be created.
+ */
+const refetchKeys: Array<keyof DashboardContainerByValueInput> = [
+  '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,
-  keys?: Array<keyof DashboardContainerByValueInput>
+  input: DashboardContainerByValueInput
 ): Promise<Partial<DashboardContainerByValueInput>> {
-  await this.untilInitialized();
-  const allKeys =
-    keys ??
-    ([...new Set([...Object.keys(lastInput), ...Object.keys(input)])] as Array<
-      keyof DashboardContainerByValueInput
-    >);
-  const keyComparePromises = allKeys.map(
+  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<boolean> {
+  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<keyof DashboardContainerByValueInput>,
+  diffingFunctions: DashboardDiffFunctions
+): Promise<Partial<DashboardContainerByValueInput>> {
+  await container.untilInitialized();
+  const keyComparePromises = keys.map(
    (key) =>
      new Promise<{ key: keyof DashboardContainerByValueInput; isEqual: boolean }>((resolve) =>
-        isKeyEqual(key, {
-          container: this,
+        isKeyEqual(
+          key,
+          {
+            container,

-          currentValue: input[key],
-          currentInput: input,
+            currentValue: input[key],
+            currentInput: input,

-          lastValue: lastInput[key],
-          lastInput,
-        }).then((isEqual) => resolve({ key, isEqual }))
+            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<DashboardContainerByValueInput>
-  );
-  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<DashboardContainerByValueInput>);
+  return inputChanges;
 }
--- 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<keyof DashboardContainerByValueInput> = [
-  '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 {