[Discover] Decouple data$ updates to prevent rows clearing on hook re-render

changelogs/fragments/8806.yml

@@ -0,0 +1,2 @@
+fix:
+- Decouple data$ updates to prevent rows clearing on hook re-render ([#8806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8806))

src/plugins/discover/public/application/view_components/utils/use_search.test.tsx

@@ -0,0 +1,156 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { act, renderHook } from '@testing-library/react-hooks';
+import { createMemoryHistory } from 'history';
+import React from 'react';
+import { Provider } from 'react-redux';
+import { Router } from 'react-router-dom';
+import { Subject } from 'rxjs';
+import { createDataExplorerServicesMock } from '../../../../../data_explorer/public/utils/mocks';
+import { DiscoverViewServices } from '../../../build_services';
+import { discoverPluginMock } from '../../../mocks';
+import { ResultStatus, useSearch } from './use_search';
+
+jest.mock('./use_index_pattern', () => ({
+  useIndexPattern: jest.fn(),
+}));
+
+const mockSavedSearch = {
+  id: 'test-saved-search',
+  title: 'Test Saved Search',
+  searchSource: {
+    setField: jest.fn(),
+    getField: jest.fn(),
+    fetch: jest.fn(),
+    getSearchRequestBody: jest.fn().mockResolvedValue({}),
+    getOwnField: jest.fn(),
+    getDataFrame: jest.fn(() => ({ name: 'test-pattern' })),
+  },
+  getFullPath: jest.fn(),
+  getOpenSearchType: jest.fn(),
+};
+
+const createMockServices = (): DiscoverViewServices => {
+  const dataExplorerServicesMock = createDataExplorerServicesMock();
+  const discoverServicesMock = discoverPluginMock.createDiscoverServicesMock();
+  const services: DiscoverViewServices = {
+    ...dataExplorerServicesMock,
+    ...discoverServicesMock,
+  };
+
+  (services.data.query.timefilter.timefilter.getRefreshInterval as jest.Mock).mockReturnValue({
+    pause: false,
+    value: 10,
+  });
+  services.getSavedSearchById = jest.fn().mockResolvedValue(mockSavedSearch);
+  return services;
+};
+
+const history = createMemoryHistory();
+const mockStore = {
+  getState: () => ({
+    discover: {
+      savedSearch: 'test-saved-search',
+      sort: [],
+      interval: 'auto',
+      savedQuery: undefined,
+    },
+  }),
+  subscribe: jest.fn(),
+  dispatch: jest.fn(),
+};
+const wrapper: React.FC = ({ children }) => {
+  return (
+    <Provider store={mockStore}>
+      <Router history={history}>{children}</Router>
+    </Provider>
+  );
+};
+
+describe('useSearch', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should initialize with loading state when search on page load is enabled', async () => {
+    const services = createMockServices();
+    (services.uiSettings.get as jest.Mock).mockReturnValueOnce(true);
+
+    const { result } = renderHook(() => useSearch(services), { wrapper });
+
+    expect(result.current.data$.getValue()).toEqual(
+      expect.objectContaining({ status: ResultStatus.LOADING })
+    );
+  });
+
+  it('should initialize with uninitialized state when search on page load is disabled', async () => {
+    const services = createMockServices();
+    (services.uiSettings.get as jest.Mock).mockReturnValueOnce(false);
+    (services.data.query.timefilter.timefilter.getRefreshInterval as jest.Mock).mockReturnValue({
+      pause: true,
+      value: 10,
+    });
+
+    const { result } = renderHook(() => useSearch(services), { wrapper });
+    expect(result.current.data$.getValue()).toEqual(
+      expect.objectContaining({ status: ResultStatus.UNINITIALIZED })
+    );
+  });
+
+  it('should update startTime when hook rerenders', async () => {
+    const services = createMockServices();
+
+    const { result, rerender } = renderHook(() => useSearch(services), {
+      wrapper,
+    });
+
+    const initialStartTime = result.current.data$.getValue().queryStatus?.startTime;
+    expect(initialStartTime).toBeDefined();
+
+    rerender();
+    const newStartTime = result.current.data$.getValue().queryStatus?.startTime;
+    expect(newStartTime).toBeDefined();
+    expect(newStartTime).not.toEqual(initialStartTime);
+  });
+
+  it('should reset data observable when dataset changes', async () => {
+    const services = createMockServices();
+    const mockDatasetUpdates$ = new Subject();
+    services.data.query.queryString.getUpdates$ = jest.fn().mockReturnValue(mockDatasetUpdates$);
+
+    const { result, waitForNextUpdate } = renderHook(() => useSearch(services), {
+      wrapper,
+    });
+
+    act(() => {
+      result.current.data$.next({ status: ResultStatus.READY });
+    });
+
+    expect(result.current.data$.getValue()).toEqual(
+      expect.objectContaining({ status: ResultStatus.READY })
+    );
+
+    act(() => {
+      mockDatasetUpdates$.next({
+        dataset: { id: 'new-dataset-id', title: 'New Dataset', type: 'INDEX_PATTERN' },
+      });
+      mockDatasetUpdates$.next({
+        dataset: { id: 'new-dataset-id', title: 'New Dataset', type: 'INDEX_PATTERN' },
+      });
+      mockDatasetUpdates$.next({
+        dataset: { id: 'new-dataset-id2', title: 'New Dataset', type: 'INDEX_PATTERN' },
+      });
+    });
+
+    await act(async () => {
+      await waitForNextUpdate();
+    });
+
+    expect(result.current.data$.getValue()).toEqual(
+      expect.objectContaining({ status: ResultStatus.LOADING })
+    );
+  });
+});

src/plugins/discover/public/application/view_components/utils/use_search.ts

@@ -5,11 +5,12 @@
 
 import { useCallback, useMemo, useRef, useState } from 'react';
 import { BehaviorSubject, Subject, merge } from 'rxjs';
-import { debounceTime } from 'rxjs/operators';
+import { debounceTime, filter, pairwise } from 'rxjs/operators';
 import { i18n } from '@osd/i18n';
 import { useEffect } from 'react';
 import { cloneDeep } from 'lodash';
 import { useLocation } from 'react-router-dom';
+import { useEffectOnce } from 'react-use';
 import { RequestAdapter } from '../../../../../inspector/public';
 import { DiscoverViewServices } from '../../../build_services';
 import { search } from '../../../../../data/public';
@@ -143,8 +144,45 @@
             : ResultStatus.UNINITIALIZED,
         queryStatus: { startTime },
       }),
-    [shouldSearchOnPageLoad, startTime, skipInitialFetch]
+    // we only want data$ observable to be created once, updates will be done through useEffect
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    []
   );
+
+  // re-initialize data$ when the selected dataset changes
+  useEffectOnce(() => {
+    const subscription = data.query.queryString
+      .getUpdates$()
+      .pipe(
+        pairwise(),
+        filter(([prev, curr]) => prev.dataset?.id === curr.dataset?.id)
+      )
+      .subscribe(() => {
+        data$.next({
+          status:
+            shouldSearchOnPageLoad() && !skipInitialFetch.current
+              ? ResultStatus.LOADING
+              : ResultStatus.UNINITIALIZED,
+          queryStatus: { startTime },
+        });
+      });
+    return () => subscription.unsubscribe();
+  });
+
+  useEffect(() => {
+    data$.next({ ...data$.value, queryStatus: { startTime } });
+  }, [data$, startTime]);
+
+  useEffect(() => {
+    data$.next({
+      ...data$.value,
+      status:
+        shouldSearchOnPageLoad() && !skipInitialFetch.current
+          ? ResultStatus.LOADING
+          : ResultStatus.UNINITIALIZED,
+    });
+  }, [data$, shouldSearchOnPageLoad, skipInitialFetch]);
+
   const refetch$ = useMemo(() => new Subject<SearchRefetch>(), []);
 
   const fetch = useCallback(async () => {

src/plugins/discover/public/mocks.ts

@@ -29,6 +29,17 @@
  */
 
 import { DiscoverSetup, DiscoverStart } from '.';
+import { coreMock } from '../../../core/public/mocks';
+import { chartPluginMock } from '../../charts/public/mocks';
+import { dataPluginMock } from '../../data/public/mocks';
+import { embeddablePluginMock } from '../../embeddable/public/mocks';
+import { inspectorPluginMock } from '../../inspector/public/mocks';
+import { navigationPluginMock } from '../../navigation/public/mocks';
+import { opensearchDashboardsLegacyPluginMock } from '../../opensearch_dashboards_legacy/public/mocks';
+import { uiActionsPluginMock } from '../../ui_actions/public/mocks';
+import { urlForwardingPluginMock } from '../../url_forwarding/public/mocks';
+import { visualizationsPluginMock } from '../../visualizations/public/mocks';
+import { buildServices, DiscoverServices } from './build_services';
 
 export type Setup = jest.Mocked<DiscoverSetup>;
 export type Start = jest.Mocked<DiscoverStart>;
@@ -55,7 +66,25 @@ const createStartContract = (): Start => {
   return startContract;
 };
 
+const createDiscoverServicesMock = (): DiscoverServices =>
+  buildServices(
+    coreMock.createStart(),
+    {
+      data: dataPluginMock.createStartContract(),
+      charts: chartPluginMock.createStartContract(),
+      embeddable: embeddablePluginMock.createStartContract(),
+      inspector: inspectorPluginMock.createStartContract(),
+      navigation: navigationPluginMock.createStartContract(),
+      uiActions: uiActionsPluginMock.createStartContract(),
+      urlForwarding: urlForwardingPluginMock.createStartContract(),
+      visualizations: visualizationsPluginMock.createStartContract(),
+      opensearchDashboardsLegacy: opensearchDashboardsLegacyPluginMock.createStartContract(),
+    },
+    coreMock.createPluginInitializerContext()
+  );
+
 export const discoverPluginMock = {
+  createDiscoverServicesMock,
   createSetupContract,
   createStartContract,
 };