[Search] Fix dashboard embeddables don't refetch on searchSessionId c…

…hange (#84261)

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md

@@ -0,0 +1,17 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [Embeddable](./kibana-plugin-plugins-embeddable-public.embeddable.md) &gt; [getUpdated$](./kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md)
+
+## Embeddable.getUpdated$() method
+
+Merges input$ and output$ streams and denounces emit till next macro-task Could be useful to batch reactions to input$ and output$ updates that happen separately but synchronously In case corresponding state change triggered `reload` this stream is guarantied to emit later which allows to skip any state handling in case `reload` already handled it
+
+<b>Signature:</b>
+
+```typescript
+getUpdated$(): Readonly<Rx.Observable<void>>;
+```
+<b>Returns:</b>
+
+`Readonly<Rx.Observable<void>>`
+

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.md

@@ -44,8 +44,9 @@ export declare abstract class Embeddable<TEmbeddableInput extends EmbeddableInpu
 |  [getOutput$()](./kibana-plugin-plugins-embeddable-public.embeddable.getoutput_.md) |  |  |
 |  [getRoot()](./kibana-plugin-plugins-embeddable-public.embeddable.getroot.md) |  | Returns the top most parent embeddable, or itself if this embeddable is not within a parent. |
 |  [getTitle()](./kibana-plugin-plugins-embeddable-public.embeddable.gettitle.md) |  |  |
+|  [getUpdated$()](./kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md) |  | Merges input$ and output$ streams and denounces emit till next macro-task Could be useful to batch reactions to input$ and output$ updates that happen separately but synchronously In case corresponding state change triggered <code>reload</code> this stream is guarantied to emit later which allows to skip any state handling in case <code>reload</code> already handled it |
 |  [onFatalError(e)](./kibana-plugin-plugins-embeddable-public.embeddable.onfatalerror.md) |  |  |
-|  [reload()](./kibana-plugin-plugins-embeddable-public.embeddable.reload.md) |  | Reload will be called when there is a request to refresh the data or view, even if the input data did not change. |
+|  [reload()](./kibana-plugin-plugins-embeddable-public.embeddable.reload.md) |  | Reload will be called when there is a request to refresh the data or view, even if the input data did not change.<!-- -->In case if input data did change and reload is requested input$ and output$ would still emit before <code>reload</code> is called<!-- -->The order would be as follows: input$ output$ reload() \-\-\-- updated$ |
 |  [render(el)](./kibana-plugin-plugins-embeddable-public.embeddable.render.md) |  |  |
 |  [supportedTriggers()](./kibana-plugin-plugins-embeddable-public.embeddable.supportedtriggers.md) |  |  |
 |  [updateInput(changes)](./kibana-plugin-plugins-embeddable-public.embeddable.updateinput.md) |  |  |

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.reload.md

@@ -6,6 +6,10 @@
 
 Reload will be called when there is a request to refresh the data or view, even if the input data did not change.
 
+In case if input data did change and reload is requested input$ and output$ would still emit before `reload` is called
+
+The order would be as follows: input$ output$ reload() \-\-\-- updated$
+
 <b>Signature:</b>
 
 ```typescript

src/plugins/dashboard/public/application/dashboard_app_controller.tsx

@@ -633,9 +633,26 @@ export class DashboardAppController {
           removeQueryParam(history, DashboardConstants.SEARCH_SESSION_ID, true);
         }
 
+        // state keys change in which likely won't need a data fetch
+        const noRefetchKeys: Array<keyof DashboardContainerInput> = [
+          'viewMode',
+          'title',
+          'description',
+          'expandedPanelId',
+          'useMargins',
+          'isEmbeddedExternally',
+          'isFullScreenMode',
+          'isEmptyState',
+        ];
+
+        const shouldRefetch = Object.keys(changes).some(
+          (changeKey) => !noRefetchKeys.includes(changeKey as keyof DashboardContainerInput)
+        );
+
         dashboardContainer.updateInput({
           ...changes,
-          searchSessionId: searchService.session.start(),
+          // do not start a new session if this is irrelevant state change to prevent excessive searches
+          ...(shouldRefetch && { searchSessionId: searchService.session.start() }),
         });
       }
     };

src/plugins/discover/public/application/embeddable/search_embeddable.ts

@@ -19,7 +19,6 @@
 import './search_embeddable.scss';
 import angular from 'angular';
 import _ from 'lodash';
-import * as Rx from 'rxjs';
 import { Subscription } from 'rxjs';
 import { i18n } from '@kbn/i18n';
 import { UiActionsStart, APPLY_FILTER_TRIGGER } from '../../../../ui_actions/public';
@@ -98,6 +97,7 @@ export class SearchEmbeddable
   private prevTimeRange?: TimeRange;
   private prevFilters?: Filter[];
   private prevQuery?: Query;
+  private prevSearchSessionId?: string;
 
   constructor(
     {
@@ -140,7 +140,7 @@ export class SearchEmbeddable
       .timefilter.getAutoRefreshFetch$()
       .subscribe(this.fetch);
 
-    this.subscription = Rx.merge(this.getOutput$(), this.getInput$()).subscribe(() => {
+    this.subscription = this.getUpdated$().subscribe(() => {
       this.panelTitle = this.output.title || '';
 
       if (this.searchScope) {
@@ -262,7 +262,8 @@ export class SearchEmbeddable
   }
 
   public reload() {
-    this.fetch();
+    if (this.searchScope)
+      this.pushContainerStateParamsToScope(this.searchScope, { forceFetch: true });
   }
 
   private fetch = async () => {
@@ -326,26 +327,31 @@ export class SearchEmbeddable
     }
   };
 
-  private pushContainerStateParamsToScope(searchScope: SearchScope) {
+  private pushContainerStateParamsToScope(
+    searchScope: SearchScope,
+    { forceFetch = false }: { forceFetch: boolean } = { forceFetch: false }
+  ) {
     const isFetchRequired =
       !esFilters.onlyDisabledFiltersChanged(this.input.filters, this.prevFilters) ||
       !_.isEqual(this.prevQuery, this.input.query) ||
       !_.isEqual(this.prevTimeRange, this.input.timeRange) ||
-      !_.isEqual(searchScope.sort, this.input.sort || this.savedSearch.sort);
+      !_.isEqual(searchScope.sort, this.input.sort || this.savedSearch.sort) ||
+      this.prevSearchSessionId !== this.input.searchSessionId;
 
     // If there is column or sort data on the panel, that means the original columns or sort settings have
     // been overridden in a dashboard.
     searchScope.columns = this.input.columns || this.savedSearch.columns;
     searchScope.sort = this.input.sort || this.savedSearch.sort;
     searchScope.sharedItemTitle = this.panelTitle;
 
-    if (isFetchRequired) {
+    if (forceFetch || isFetchRequired) {
       this.filtersSearchSource!.setField('filter', this.input.filters);
       this.filtersSearchSource!.setField('query', this.input.query);
+
       this.prevFilters = this.input.filters;
       this.prevQuery = this.input.query;
       this.prevTimeRange = this.input.timeRange;
-
+      this.prevSearchSessionId = this.input.searchSessionId;
       this.fetch();
     } else if (this.searchScope) {
       // trigger a digest cycle to make sure non-fetch relevant changes are propagated

src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx

@@ -19,7 +19,7 @@
 
 /* eslint-disable max-classes-per-file */
 
-import { skip } from 'rxjs/operators';
+import { skip, take } from 'rxjs/operators';
 import { Embeddable } from './embeddable';
 import { EmbeddableOutput, EmbeddableInput } from './i_embeddable';
 import { ViewMode } from '../types';
@@ -112,3 +112,34 @@ test('updating output state retains instance information', async () => {
   expect(outputTest.getOutput().inputUpdatedTimes).toBe(2);
   expect(outputTest.getOutput().testClass).toBeInstanceOf(TestClass);
 });
+
+test('updated$ called after reload and batches input/output changes', async () => {
+  const hello = new ContactCardEmbeddable(
+    { id: '123', firstName: 'Brienne', lastName: 'Tarth' },
+    { execAction: (() => null) as any }
+  );
+
+  const reloadSpy = jest.spyOn(hello, 'reload');
+
+  const input$ = hello.getInput$().pipe(skip(1));
+  let inputEmittedTimes = 0;
+  input$.subscribe(() => {
+    inputEmittedTimes++;
+  });
+
+  const updated$ = hello.getUpdated$();
+  let updatedEmittedTimes = 0;
+  updated$.subscribe(() => {
+    updatedEmittedTimes++;
+  });
+  const updatedPromise = updated$.pipe(take(1)).toPromise();
+
+  hello.updateInput({ nameTitle: 'Sir', lastReloadRequestTime: Date.now() });
+  expect(reloadSpy).toHaveBeenCalledTimes(1);
+  expect(inputEmittedTimes).toBe(1);
+  expect(updatedEmittedTimes).toBe(0);
+
+  await updatedPromise;
+
+  expect(updatedEmittedTimes).toBe(1);
+});

src/plugins/embeddable/public/lib/embeddables/embeddable.tsx

@@ -19,7 +19,8 @@
 
 import { cloneDeep, isEqual } from 'lodash';
 import * as Rx from 'rxjs';
-import { distinctUntilChanged, map } from 'rxjs/operators';
+import { merge } from 'rxjs';
+import { debounceTime, distinctUntilChanged, map, mapTo, skip } from 'rxjs/operators';
 import { RenderCompleteDispatcher } from '../../../../kibana_utils/public';
 import { Adapters } from '../types';
 import { IContainer } from '../containers';
@@ -104,9 +105,31 @@ export abstract class Embeddable<
   /**
    * Reload will be called when there is a request to refresh the data or view, even if the
    * input data did not change.
+   *
+   * In case if input data did change and reload is requested input$ and output$ would still emit before `reload` is called
+   *
+   * The order would be as follows:
+   * input$
+   * output$
+   * reload()
+   * ----
+   * updated$
    */
   public abstract reload(): void;
 
+  /**
+   * Merges input$ and output$ streams and debounces emit till next macro-task.
+   * Could be useful to batch reactions to input$ and output$ updates that happen separately but synchronously.
+   * In case corresponding state change triggered `reload` this stream is guarantied to emit later,
+   * which allows to skip any state handling in case `reload` already handled it.
+   */
+  public getUpdated$(): Readonly<Rx.Observable<void>> {
+    return merge(this.getInput$().pipe(skip(1)), this.getOutput$().pipe(skip(1))).pipe(
+      debounceTime(0),
+      mapTo(undefined)
+    );
+  }
+
   public getInput$(): Readonly<Rx.Observable<TEmbeddableInput>> {
     return this.input$.asObservable();
   }

src/plugins/embeddable/public/public.api.md

@@ -312,6 +312,7 @@ export abstract class Embeddable<TEmbeddableInput extends EmbeddableInput = Embe
     getRoot(): IEmbeddable | IContainer;
     // (undocumented)
     getTitle(): string;
+    getUpdated$(): Readonly<Rx.Observable<void>>;
     // (undocumented)
     readonly id: string;
     // (undocumented)

src/plugins/visualizations/public/embeddable/visualize_embeddable.ts

@@ -19,7 +19,6 @@
 
 import _, { get } from 'lodash';
 import { Subscription } from 'rxjs';
-import * as Rx from 'rxjs';
 import { i18n } from '@kbn/i18n';
 import { VISUALIZE_EMBEDDABLE_TYPE } from './constants';
 import {
@@ -100,6 +99,7 @@ export class VisualizeEmbeddable
   private timeRange?: TimeRange;
   private query?: Query;
   private filters?: Filter[];
+  private searchSessionId?: string;
   private visCustomizations?: Pick<VisualizeInput, 'vis' | 'table'>;
   private subscriptions: Subscription[] = [];
   private expression: string = '';
@@ -155,8 +155,11 @@ export class VisualizeEmbeddable
       .subscribe(this.updateHandler.bind(this));
 
     this.subscriptions.push(
-      Rx.merge(this.getOutput$(), this.getInput$()).subscribe(() => {
-        this.handleChanges();
+      this.getUpdated$().subscribe(() => {
+        const isDirty = this.handleChanges();
+        if (isDirty && this.handler) {
+          this.updateHandler();
+        }
       })
     );
 
@@ -220,7 +223,7 @@ export class VisualizeEmbeddable
     }
   }
 
-  public async handleChanges() {
+  private handleChanges(): boolean {
     this.transferCustomizationsToUiState();
 
     let dirty = false;
@@ -243,13 +246,16 @@ export class VisualizeEmbeddable
       dirty = true;
     }
 
+    if (this.searchSessionId !== this.input.searchSessionId) {
+      this.searchSessionId = this.input.searchSessionId;
+      dirty = true;
+    }
+
     if (this.vis.description && this.domNode) {
       this.domNode.setAttribute('data-description', this.vis.description);
     }
 
-    if (this.handler && dirty) {
-      this.updateHandler();
-    }
+    return dirty;
   }
 
   // this is a hack to make editor still work, will be removed once we clean up editor
@@ -395,6 +401,7 @@ export class VisualizeEmbeddable
   }
 
   private handleVisUpdate = async () => {
+    this.handleChanges();
     this.updateHandler();
   };
 

x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx

@@ -205,13 +205,19 @@ describe('embeddable', () => {
     await embeddable.initializeSavedVis({ id: '123' } as LensEmbeddableInput);
     embeddable.render(mountpoint);
 
+    expect(expressionRenderer).toHaveBeenCalledTimes(1);
+
     embeddable.updateInput({
       timeRange,
       query,
       filters,
       searchSessionId: 'searchSessionId',
     });
 
+    expect(expressionRenderer).toHaveBeenCalledTimes(1);
+
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
     expect(expressionRenderer).toHaveBeenCalledTimes(2);
   });