[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)](#151390)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Nathan Reese <[email protected]>
elastic · Feb 21, 2023 · 748dfcb · 748dfcb
1 parent 5aa2bdf
commit 748dfcb
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 56 deletions.
diff --git 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/...lic/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts b/...lic/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),
+};
diff --git a/...hboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.test.ts b/...hboard_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/...c/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_integration.ts b/...c/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
+): Promise<Partial<DashboardContainerByValueInput>> {
+  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>
+  keys: Array<keyof DashboardContainerByValueInput>,
+  diffingFunctions: DashboardDiffFunctions
 ): 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(
+  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<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;
 }
diff --git a/...ner/embeddable/integrations/search_sessions/start_dashboard_search_session_integration.ts b/...ner/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 {