[8.x] [Embeddable] Avoid rerendering loop due to search context reload (

#203150) (#203818) # Backport This will backport the following commits from `main` to `8.x`: - [[Embeddable] Avoid rerendering loop due to search context reload (#203150)](#203150)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
elastic · Dec 11, 2024 · 8eb5049 · 8eb5049
1 parent 35488c3
commit 8eb5049
Show file tree

Hide file tree

Showing 10 changed files with 192 additions and 55 deletions.
diff --git a/packages/presentation/presentation_publishing/interfaces/fetch/publishes_unified_search.ts b/packages/presentation/presentation_publishing/interfaces/fetch/publishes_unified_search.ts
@@ -7,7 +7,15 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-import { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query';
+import {
+  AggregateQuery,
+  COMPARE_ALL_OPTIONS,
+  Filter,
+  Query,
+  TimeRange,
+  onlyDisabledFiltersChanged,
+} from '@kbn/es-query';
+import fastIsEqual from 'fast-deep-equal';
 import { useEffect, useMemo } from 'react';
 import { BehaviorSubject } from 'rxjs';
 import { PublishingSubject } from '../../publishing_subject';
@@ -112,15 +120,27 @@ export function useSearchApi({
   }, []);
 
   useEffect(() => {
-    searchApi.filters$.next(filters);
+    if (
+      !onlyDisabledFiltersChanged(searchApi.filters$.getValue(), filters, {
+        ...COMPARE_ALL_OPTIONS,
+        // do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results)
+        state: false,
+      })
+    ) {
+      searchApi.filters$.next(filters);
+    }
   }, [filters, searchApi.filters$]);
 
   useEffect(() => {
-    searchApi.query$.next(query);
+    if (!fastIsEqual(searchApi.query$.getValue(), query)) {
+      searchApi.query$.next(query);
+    }
   }, [query, searchApi.query$]);
 
   useEffect(() => {
-    searchApi.timeRange$.next(timeRange);
+    if (!fastIsEqual(searchApi.timeRange$.getValue(), timeRange)) {
+      searchApi.timeRange$.next(timeRange);
+    }
   }, [timeRange, searchApi.timeRange$]);
 
   return searchApi;

diff --git a/x-pack/plugins/lens/public/react_embeddable/data_loader.ts b/x-pack/plugins/lens/public/react_embeddable/data_loader.ts
@@ -6,8 +6,7 @@
  */
 
 import type { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common';
-import { fetch$, type FetchContext } from '@kbn/presentation-publishing';
-import { apiPublishesSearchSession } from '@kbn/presentation-publishing/interfaces/fetch/publishes_search_session';
+import { apiPublishesUnifiedSearch, fetch$ } from '@kbn/presentation-publishing';
 import { type KibanaExecutionContext } from '@kbn/core/public';
 import {
   BehaviorSubject,
@@ -21,6 +20,7 @@ import {
   map,
 } from 'rxjs';
 import fastIsEqual from 'fast-deep-equal';
+import { pick } from 'lodash';
 import { getEditPath } from '../../common/constants';
 import type {
   GetStateType,
@@ -54,6 +54,24 @@ type ReloadReason =
   | 'viewMode'
   | 'searchContext';
 
+function getSearchContext(parentApi: unknown) {
+  const unifiedSearch$ = apiPublishesUnifiedSearch(parentApi)
+    ? pick(parentApi, 'filters$', 'query$', 'timeslice$', 'timeRange$')
+    : {
+        filters$: new BehaviorSubject(undefined),
+        query$: new BehaviorSubject(undefined),
+        timeslice$: new BehaviorSubject(undefined),
+        timeRange$: new BehaviorSubject(undefined),
+      };
+
+  return {
+    filters: unifiedSearch$.filters$.getValue(),
+    query: unifiedSearch$.query$.getValue(),
+    timeRange: unifiedSearch$.timeRange$.getValue(),
+    timeslice: unifiedSearch$.timeslice$?.getValue(),
+  };
+}
+
 /**
  * The function computes the expression used to render the panel and produces the necessary props
  * for the ExpressionWrapper component, binding any outer context to them.
@@ -112,16 +130,6 @@ export function loadEmbeddableData(
     }
   };
 
-  const unifiedSearch$ = new BehaviorSubject<
-    Pick<FetchContext, 'query' | 'filters' | 'timeRange' | 'timeslice' | 'searchSessionId'>
-  >({
-    query: undefined,
-    filters: undefined,
-    timeRange: undefined,
-    timeslice: undefined,
-    searchSessionId: undefined,
-  });
-
   async function reload(
     // make reload easier to debug
     sourceId: ReloadReason
@@ -142,8 +150,6 @@ export function loadEmbeddableData(
 
     const currentState = getState();
 
-    const { searchSessionId, ...unifiedSearch } = unifiedSearch$.getValue();
-
     const getExecutionContext = () => {
       const parentContext = getParentContext(parentApi);
       const lastState = getState();
@@ -198,7 +204,7 @@ export function loadEmbeddableData(
 
     const searchContext = getMergedSearchContext(
       currentState,
-      unifiedSearch,
+      getSearchContext(parentApi),
       api.timeRange$,
       parentApi,
       services
@@ -216,7 +222,7 @@ export function loadEmbeddableData(
         },
         renderMode: getRenderMode(parentApi),
         services,
-        searchSessionId,
+        searchSessionId: api.searchSessionId$.getValue(),
         abortController: internalApi.expressionAbortController$.getValue(),
         getExecutionContext,
         logError: getLogError(getExecutionContext),
@@ -259,20 +265,8 @@ export function loadEmbeddableData(
   }
 
   const mergedSubscriptions = merge(
-    // on data change from the parentApi, reload
-    fetch$(api).pipe(
-      tap((data) => {
-        const searchSessionId = apiPublishesSearchSession(parentApi) ? data.searchSessionId : '';
-        unifiedSearch$.next({
-          query: data.query,
-          filters: data.filters,
-          timeRange: data.timeRange,
-          timeslice: data.timeslice,
-          searchSessionId,
-        });
-      }),
-      map(() => 'searchContext' as ReloadReason)
-    ),
+    // on search context change, reload
+    fetch$(api).pipe(map(() => 'searchContext' as ReloadReason)),
     // On state change, reload
     // this is used to refresh the chart on inline editing
     // just make sure to avoid to rerender if there's no substantial change

diff --git a/x-pack/plugins/lens/public/react_embeddable/mocks/index.tsx b/x-pack/plugins/lens/public/react_embeddable/mocks/index.tsx
@@ -116,6 +116,7 @@ const LensApiMock: LensApi = {
   disabledActionIds: new BehaviorSubject<string[] | undefined>(undefined),
   setDisabledActionIds: jest.fn(),
   rendered$: new BehaviorSubject<boolean>(false),
+  searchSessionId$: new BehaviorSubject<string | undefined>(undefined),
 };
 
 const LensSerializedStateMock: LensSerializedState = createEmptyLensState(

diff --git a/x-pack/plugins/lens/public/react_embeddable/renderer/lens_custom_renderer_component.tsx b/x-pack/plugins/lens/public/react_embeddable/renderer/lens_custom_renderer_component.tsx
@@ -59,6 +59,7 @@ export function LensRenderer({
   filters,
   timeRange,
   disabledActions,
+  searchSessionId,
   hidePanelTitles,
   ...props
 }: LensRendererProps) {
@@ -72,6 +73,7 @@ export function LensRenderer({
   }, []);
   const disabledActionIds$ = useObservableVariable(disabledActions);
   const viewMode$ = useObservableVariable(viewMode);
+  const searchSessionId$ = useObservableVariable(searchSessionId);
   const hidePanelTitles$ = useObservableVariable(hidePanelTitles);
 
   // Lens API will be set once, but when set trigger a reflow to adopt the latest attributes
@@ -136,6 +138,7 @@ export function LensRenderer({
         ...props,
         // forward the unified search context
         ...searchApi,
+        searchSessionId$,
         disabledActionIds: disabledActionIds$,
         setDisabledActionIds: (ids: string[] | undefined) => disabledActionIds$.next(ids),
         viewMode: viewMode$,

diff --git a/x-pack/plugins/lens/public/react_embeddable/types.ts b/x-pack/plugins/lens/public/react_embeddable/types.ts
@@ -65,6 +65,7 @@ import type { AllowedPartitionOverrides } from '@kbn/expression-partition-vis-pl
 import type { AllowedXYOverrides } from '@kbn/expression-xy-plugin/common';
 import type { Action } from '@kbn/ui-actions-plugin/public';
 import { PresentationContainer } from '@kbn/presentation-containers';
+import { PublishesSearchSession } from '@kbn/presentation-publishing/interfaces/fetch/publishes_search_session';
 import type { LegacyMetricState } from '../../common';
 import type { LensDocument } from '../persistence';
 import type { LensInspector } from '../lens_inspector_service';
@@ -364,6 +365,8 @@ export type LensApi = Simplify<
     PublishesBlockingError &
     // This is used by dashboard/container to show filters/queries on the panel
     PublishesUnifiedSearch &
+    // Forward the search session id
+    PublishesSearchSession &
     // Let the container know the loading state
     PublishesDataLoading &
     // Let the container know when the rendering has completed rendering

diff --git a/...ins/observability_solution/infra/public/components/asset_details/hooks/use_date_picker.ts b/...ins/observability_solution/infra/public/components/asset_details/hooks/use_date_picker.ts
@@ -52,8 +52,9 @@ export function useDatePicker({
     (newDateRange: TimeRange) => {
       setUrlState({ dateRange: newDateRange });
       setParsedDateRange(parseDateRange(newDateRange));
+      updateSearchSessionId();
     },
-    [setUrlState]
+    [setUrlState, updateSearchSessionId]
   );
 
   const onRefresh = useCallback(
@@ -62,12 +63,10 @@ export function useDatePicker({
       if (autoRefreshEnabled) {
         autoRefreshTick$.next(null);
       } else {
-        updateSearchSessionId();
+        setDateRange(newDateRange);
       }
-
-      setDateRange(newDateRange);
     },
-    [autoRefreshEnabled, autoRefreshTick$, setDateRange, updateSearchSessionId]
+    [autoRefreshEnabled, autoRefreshTick$, setDateRange]
   );
 
   const setAutoRefresh = useCallback(

diff --git a/...ugins/observability_solution/infra/public/pages/metrics/hosts/hooks/use_unified_search.ts b/...ugins/observability_solution/infra/public/pages/metrics/hosts/hooks/use_unified_search.ts
@@ -53,7 +53,11 @@ export const useUnifiedSearch = () => {
 
   const {
     data: {
-      query: { filterManager: filterManagerService, queryString: queryStringService },
+      query: {
+        filterManager: filterManagerService,
+        queryString: queryStringService,
+        timefilter: timeFilterService,
+      },
     },
     telemetry,
   } = services;
@@ -68,29 +72,33 @@ export const useUnifiedSearch = () => {
   const onFiltersChange = useCallback(
     (filters: Filter[]) => {
       setSearch({ type: 'SET_FILTERS', filters });
+      updateSearchSessionId();
     },
-    [setSearch]
+    [setSearch, updateSearchSessionId]
   );
 
   const onPanelFiltersChange = useCallback(
     (panelFilters: Filter[]) => {
       setSearch({ type: 'SET_PANEL_FILTERS', panelFilters });
+      updateSearchSessionId();
     },
-    [setSearch]
+    [setSearch, updateSearchSessionId]
   );
 
   const onLimitChange = useCallback(
     (limit: number) => {
       setSearch({ type: 'SET_LIMIT', limit });
+      updateSearchSessionId();
     },
-    [setSearch]
+    [setSearch, updateSearchSessionId]
   );
 
   const onDateRangeChange = useCallback(
     (dateRange: StringDateRange) => {
       setSearch({ type: 'SET_DATE_RANGE', dateRange });
+      updateSearchSessionId();
     },
-    [setSearch]
+    [setSearch, updateSearchSessionId]
   );
 
   const onQueryChange = useCallback(
@@ -99,19 +107,19 @@ export const useUnifiedSearch = () => {
         setError(null);
         validateQuery(query);
         setSearch({ type: 'SET_QUERY', query });
+        updateSearchSessionId();
       } catch (err) {
         setError(err);
       }
     },
-    [validateQuery, setSearch]
+    [validateQuery, setSearch, updateSearchSessionId]
   );
 
   const onSubmit = useCallback(
     ({ dateRange }: { dateRange: TimeRange }) => {
       onDateRangeChange(dateRange);
-      updateSearchSessionId();
     },
-    [onDateRangeChange, updateSearchSessionId]
+    [onDateRangeChange]
   );
 
   const getDateRangeAsTimestamp = useCallback(() => {
@@ -168,6 +176,16 @@ export const useUnifiedSearch = () => {
         .subscribe()
     );
 
+    subscription.add(
+      timeFilterService.timefilter
+        .getTimeUpdate$()
+        .pipe(
+          map(() => timeFilterService.timefilter.getTime()),
+          tap((dateRange) => onDateRangeChange(dateRange))
+        )
+        .subscribe()
+    );
+
     subscription.add(
       queryStringService
         .getUpdates$()
@@ -181,7 +199,14 @@ export const useUnifiedSearch = () => {
     return () => {
       subscription.unsubscribe();
     };
-  }, [filterManagerService, queryStringService, onQueryChange, onFiltersChange]);
+  }, [
+    filterManagerService,
+    queryStringService,
+    onQueryChange,
+    onFiltersChange,
+    timeFilterService.timefilter,
+    onDateRangeChange,
+  ]);
 
   // Track telemetry event on query/filter/date changes
   useEffect(() => {

diff --git a/...ty_solution/observability/public/components/rule_condition_chart/rule_condition_chart.tsx b/...ty_solution/observability/public/components/rule_condition_chart/rule_condition_chart.tsx
@@ -138,19 +138,21 @@ export function RuleConditionChart({
       const errorDiv = document.querySelector('.lnsEmbeddedError');
       if (errorDiv) {
         const paragraphElements = errorDiv.querySelectorAll('p');
-        if (!paragraphElements || paragraphElements.length < 2) return;
+        if (!paragraphElements) return;
         paragraphElements[0].innerText = i18n.translate(
           'xpack.observability.ruleCondition.chart.error_equation.title',
           {
             defaultMessage: 'An error occurred while rendering the chart',
           }
         );
-        paragraphElements[1].innerText = i18n.translate(
-          'xpack.observability.ruleCondition.chart.error_equation.description',
-          {
-            defaultMessage: 'Check the rule equation.',
-          }
-        );
+        if (paragraphElements.length > 1) {
+          paragraphElements[1].innerText = i18n.translate(
+            'xpack.observability.ruleCondition.chart.error_equation.description',
+            {
+              defaultMessage: 'Check the rule equation.',
+            }
+          );
+        }
       }
     });
   }, [chartLoading, attributes]);

diff --git a/x-pack/test/observability_functional/apps/observability/index.ts b/x-pack/test/observability_functional/apps/observability/index.ts
@@ -17,6 +17,7 @@ export default function ({ loadTestFile }: FtrProviderContext) {
     loadTestFile(require.resolve('./pages/alerts/rule_stats'));
     loadTestFile(require.resolve('./pages/alerts/state_synchronization'));
     loadTestFile(require.resolve('./pages/alerts/table_storage'));
+    loadTestFile(require.resolve('./pages/alerts/custom_threshold_preview_chart'));
     loadTestFile(require.resolve('./pages/alerts/custom_threshold'));
     loadTestFile(require.resolve('./pages/cases/case_details'));
     loadTestFile(require.resolve('./pages/overview/alert_table'));