[Dashboard] fix custom time ranges not applied to panel until global query context changes

src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts

@@ -7,7 +7,10 @@
  */
 
 import {
+  ContactCardEmbeddable,
+  ContactCardEmbeddableFactory,
   ContactCardEmbeddableInput,
+  ContactCardEmbeddableOutput,
   CONTACT_CARD_EMBEDDABLE,
 } from '@kbn/embeddable-plugin/public/lib/test_samples';
 import {
@@ -253,3 +256,63 @@ test('creates a control group from the control group factory and waits for it to
   );
   expect(mockControlGroupContainer.untilInitialized).toHaveBeenCalled();
 });
+
+/*
+ * dashboard.getInput$() subscriptions are used to update:
+ * 1) dashboard instance searchSessionId state
+ * 2) child input on parent input changes
+ *
+ * Rxjs subscriptions are executioned in the order that they are created.
+ * This test ensures that searchSessionId update subscription is created before child input subscription
+ * to ensure child input subscription includes updated searchSessionId.
+ */
+test('searchSessionId is updated prior to child embeddable parent subscription execution', async () => {
+  const embeddableFactory = {
+    create: new ContactCardEmbeddableFactory((() => null) as any, {} as any),
+    getDefaultInput: jest.fn().mockResolvedValue({
+      timeRange: {
+        to: 'now',
+        from: 'now-15m',
+      },
+    }),
+  };
+  pluginServices.getServices().embeddable.getEmbeddableFactory = jest
+    .fn()
+    .mockReturnValue(embeddableFactory);
+  let sessionCount = 0;
+  pluginServices.getServices().data.search.session.start = () => {
+    sessionCount++;
+    return `searchSessionId${sessionCount}`;
+  };
+  const dashboard = await createDashboard(embeddableId, {
+    searchSessionSettings: {
+      getSearchSessionIdFromURL: () => undefined,
+      removeSessionIdFromUrl: () => {},
+      createSessionRestorationDataProvider: () => {},
+    } as unknown as DashboardCreationOptions['searchSessionSettings'],
+  });
+  const embeddable = await dashboard.addNewEmbeddable<
+    ContactCardEmbeddableInput,
+    ContactCardEmbeddableOutput,
+    ContactCardEmbeddable
+  >(CONTACT_CARD_EMBEDDABLE, {
+    firstName: 'Bob',
+  });
+
+  expect(embeddable.getInput().searchSessionId).toBe('searchSessionId1');
+
+  dashboard.updateInput({
+    timeRange: {
+      to: 'now',
+      from: 'now-7d',
+    },
+  });
+
+  expect(sessionCount).toBeGreaterThan(1);
+  const embeddableInput = embeddable.getInput();
+  expect((embeddableInput as any).timeRange).toEqual({
+    to: 'now',
+    from: 'now-7d',
+  });
+  expect(embeddableInput.searchSessionId).toBe(`searchSessionId${sessionCount}`);
+});

src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts

@@ -211,6 +211,7 @@ export const createDashboard = async (
   // --------------------------------------------------------------------------------------
   // Set up search sessions integration.
   // --------------------------------------------------------------------------------------
+  let initialSearchSessionId;
   if (searchSessionSettings) {
     const { sessionIdToRestore } = searchSessionSettings;
 
@@ -223,7 +224,7 @@ export const createDashboard = async (
     }
     const existingSession = session.getSessionId();
 
-    const initialSearchSessionId =
+    initialSearchSessionId =
       sessionIdToRestore ??
       (existingSession && incomingEmbeddable ? existingSession : session.start());
 
@@ -232,7 +233,6 @@ export const createDashboard = async (
         creationOptions?.searchSessionSettings
       );
     });
-    initialInput.searchSessionId = initialSearchSessionId;
   }
 
   // --------------------------------------------------------------------------------------
@@ -278,6 +278,7 @@ export const createDashboard = async (
   const dashboardContainer = new DashboardContainer(
     initialInput,
     reduxEmbeddablePackage,
+    initialSearchSessionId,
     savedObjectResult?.dashboardInput,
     dashboardCreationStartTime,
     undefined,

src/plugins/dashboard/public/dashboard_container/embeddable/create/search_sessions/start_dashboard_search_session_integration.ts

@@ -6,14 +6,13 @@
  * Side Public License, v 1.
  */
 
-import { debounceTime, pairwise, skip } from 'rxjs/operators';
+import { pairwise, skip } from 'rxjs/operators';
 
 import { noSearchSessionStorageCapabilityMessage } from '@kbn/data-plugin/public';
 
 import { DashboardContainer } from '../../dashboard_container';
 import { DashboardContainerInput } from '../../../../../common';
 import { pluginServices } from '../../../../services/plugin_services';
-import { CHANGE_CHECK_DEBOUNCE } from '../../../../dashboard_constants';
 import { DashboardCreationOptions } from '../../dashboard_container_factory';
 import { getShouldRefresh } from '../../../state/diffing/dashboard_diffing_integration';
 
@@ -57,10 +56,10 @@ export function startDashboardSearchSessionIntegration(
 
   // listen to and compare states to determine when to launch a new session.
   this.getInput$()
-    .pipe(pairwise(), debounceTime(CHANGE_CHECK_DEBOUNCE))
-    .subscribe(async (states) => {
+    .pipe(pairwise())
+    .subscribe((states) => {
       const [previous, current] = states as DashboardContainerInput[];
-      const shouldRefetch = await getShouldRefresh.bind(this)(previous, current);
+      const shouldRefetch = getShouldRefresh.bind(this)(previous, current);
       if (!shouldRefetch) return;
 
       const currentSearchSessionId = this.getState().explicitInput.searchSessionId;
@@ -83,7 +82,7 @@ export function startDashboardSearchSessionIntegration(
       })();
 
       if (updatedSearchSessionId && updatedSearchSessionId !== currentSearchSessionId) {
-        this.dispatch.setSearchSessionId(updatedSearchSessionId);
+        this.searchSessionId = updatedSearchSessionId;
       }
     });
 

src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.test.tsx

@@ -9,6 +9,7 @@
 import React from 'react';
 import { mount } from 'enzyme';
 
+import { mockedReduxEmbeddablePackage } from '@kbn/presentation-util-plugin/public/mocks';
 import { findTestSubject, nextTick } from '@kbn/test-jest-helpers';
 import { I18nProvider } from '@kbn/i18n-react';
 import {
@@ -29,9 +30,10 @@ import { applicationServiceMock, coreMock } from '@kbn/core/public/mocks';
 import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks';
 import { createEditModeActionDefinition } from '@kbn/embeddable-plugin/public/lib/test_samples';
 
-import { buildMockDashboard, getSampleDashboardPanel } from '../../mocks';
+import { buildMockDashboard, getSampleDashboardInput, getSampleDashboardPanel } from '../../mocks';
 import { pluginServices } from '../../services/plugin_services';
 import { ApplicationStart } from '@kbn/core-application-browser';
+import { DashboardContainer } from './dashboard_container';
 
 const theme = coreMock.createStart().theme;
 let application: ApplicationStart | undefined;
@@ -171,7 +173,11 @@ test('Container view mode change propagates to new children', async () => {
 
 test('searchSessionId propagates to children', async () => {
   const searchSessionId1 = 'searchSessionId1';
-  const container = buildMockDashboard({ searchSessionId: searchSessionId1 });
+  const container = new DashboardContainer(
+    getSampleDashboardInput(),
+    mockedReduxEmbeddablePackage,
+    searchSessionId1
+  );
   const embeddable = await container.addNewEmbeddable<
     ContactCardEmbeddableInput,
     ContactCardEmbeddableOutput,
@@ -181,11 +187,6 @@ test('searchSessionId propagates to children', async () => {
   });
 
   expect(embeddable.getInput().searchSessionId).toBe(searchSessionId1);
-
-  const searchSessionId2 = 'searchSessionId2';
-  container.updateInput({ searchSessionId: searchSessionId2 });
-
-  expect(embeddable.getInput().searchSessionId).toBe(searchSessionId2);
 });
 
 test('DashboardContainer in edit mode shows edit mode actions', async () => {

src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx

@@ -95,6 +95,8 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
   public subscriptions: Subscription = new Subscription();
   public controlGroup?: ControlGroupContainer;
 
+  public searchSessionId?: string;
+
   // cleanup
   public stopSyncingWithUnifiedSearch?: () => void;
   private cleanupStateTools: () => void;
@@ -117,6 +119,7 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
   constructor(
     initialInput: DashboardContainerInput,
     reduxToolsPackage: ReduxToolsPackage,
+    initialSessionId?: string,
     initialLastSavedInput?: DashboardContainerInput,
     dashboardCreationStartTime?: number,
     parent?: Container,
@@ -146,6 +149,7 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
     } = pluginServices.getServices());
 
     this.creationOptions = creationOptions;
+    this.searchSessionId = initialSessionId;
     this.dashboardCreationStartTime = dashboardCreationStartTime;
 
     // start diffing dashboard state
@@ -244,7 +248,6 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
       syncColors,
       syncTooltips,
       hidePanelTitles,
-      searchSessionId,
       refreshInterval,
       executionContext,
     } = this.input;
@@ -254,10 +257,10 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
       combinedFilters = combineDashboardFiltersWithControlGroupFilters(filters, this.controlGroup);
     }
     return {
+      searchSessionId: this.searchSessionId,
       refreshConfig: refreshInterval,
       filters: combinedFilters,
       hidePanelTitles,
-      searchSessionId,
       executionContext,
       syncTooltips,
       syncColors,

src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts

@@ -99,13 +99,6 @@ export const dashboardContainerReducers = {
     state.explicitInput.title = action.payload;
   },
 
-  setSearchSessionId: (
-    state: DashboardReduxState,
-    action: PayloadAction<DashboardContainerInput['searchSessionId']>
-  ) => {
-    state.explicitInput.searchSessionId = action.payload;
-  },
-
   // ------------------------------------------------------------------------------
   // Unsaved Changes Reducers
   // ------------------------------------------------------------------------------

src/plugins/dashboard/public/dashboard_container/state/diffing/dashboard_diffing_functions.ts

@@ -37,7 +37,7 @@ export type DashboardDiffFunctions = {
   ) => boolean | Promise<boolean>;
 };
 
-export const isKeyEqual = async (
+export const isKeyEqualAsync = async (
   key: keyof DashboardContainerInput,
   diffFunctionProps: DiffFunctionProps<typeof key>,
   diffingFunctions: DashboardDiffFunctions
@@ -52,6 +52,25 @@ export const isKeyEqual = async (
   return fastIsEqual(diffFunctionProps.currentValue, diffFunctionProps.lastValue);
 };
 
+export const isKeyEqual = (
+  key: keyof Omit<DashboardContainerInput, 'panels'>, // only Panels is async
+  diffFunctionProps: DiffFunctionProps<typeof key>,
+  diffingFunctions: DashboardDiffFunctions
+) => {
+  const propsAsNever = diffFunctionProps as never; // todo figure out why props has conflicting types in some constituents.
+  const diffingFunction = diffingFunctions[key];
+  if (!diffingFunction) {
+    return fastIsEqual(diffFunctionProps.currentValue, diffFunctionProps.lastValue);
+  }
+
+  if (diffingFunction?.prototype?.name === 'AsyncFunction') {
+    throw new Error(
+      `The function for key "${key}" is async, must use isKeyEqualAsync for asynchronous functions`
+    );
+  }
+  return diffingFunction(propsAsNever);
+};
+
 /**
  * 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.

src/plugins/dashboard/public/dashboard_container/state/diffing/dashboard_diffing_integration.test.ts

@@ -29,55 +29,55 @@ describe('getShouldRefresh', () => {
   );
 
   describe('filter changes', () => {
-    test('should return false when filters do not change', async () => {
+    test('should return false when filters do not change', () => {
       const lastInput = {
         filters: [existsFilter],
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
     });
 
-    test('should return true when pinned filters change', async () => {
+    test('should return true when pinned filters change', () => {
       const pinnedFilter = pinFilter(existsFilter);
       const lastInput = {
         filters: [pinnedFilter],
       } as unknown as DashboardContainerInput;
       const input = {
         filters: [toggleFilterNegated(pinnedFilter)],
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
     });
 
-    test('should return false when disabled filters change', async () => {
+    test('should return false when disabled filters change', () => {
       const disabledFilter = disableFilter(existsFilter);
       const lastInput = {
         filters: [disabledFilter],
       } as unknown as DashboardContainerInput;
       const input = {
         filters: [toggleFilterNegated(disabledFilter)],
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
     });
 
-    test('should return false when pinned filter changes to unpinned', async () => {
+    test('should return false when pinned filter changes to unpinned', () => {
       const lastInput = {
         filters: [existsFilter],
       } as unknown as DashboardContainerInput;
       const input = {
         filters: [pinFilter(existsFilter)],
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
     });
   });
 
   describe('timeRange changes', () => {
-    test('should return false when timeRange does not change', async () => {
+    test('should return false when timeRange does not change', () => {
       const lastInput = {
         timeRange: { from: 'now-15m', to: 'now' },
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
     });
 
-    test('should return true when timeRange changes (timeRestore is true)', async () => {
+    test('should return true when timeRange changes (timeRestore is true)', () => {
       const lastInput = {
         timeRange: { from: 'now-15m', to: 'now' },
         timeRestore: true,
@@ -86,10 +86,10 @@ describe('getShouldRefresh', () => {
         timeRange: { from: 'now-30m', to: 'now' },
         timeRestore: true,
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
     });
 
-    test('should return true when timeRange changes (timeRestore is false)', async () => {
+    test('should return true when timeRange changes (timeRestore is false)', () => {
       const lastInput = {
         timeRange: { from: 'now-15m', to: 'now' },
         timeRestore: false,
@@ -98,7 +98,26 @@ describe('getShouldRefresh', () => {
         timeRange: { from: 'now-30m', to: 'now' },
         timeRestore: false,
       } as unknown as DashboardContainerInput;
-      expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
+    });
+  });
+
+  describe('key without custom diffing function (syncColors)', () => {
+    test('should return false when syncColors do not change', () => {
+      const lastInput = {
+        syncColors: false,
+      } as unknown as DashboardContainerInput;
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
+    });
+
+    test('should return true when syncColors change', () => {
+      const lastInput = {
+        syncColors: false,
+      } as unknown as DashboardContainerInput;
+      const input = {
+        syncColors: true,
+      } as unknown as DashboardContainerInput;
+      expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
     });
   });
 });