diff --git a/changelogs/fragments/8806.yml b/changelogs/fragments/8806.yml new file mode 100644 index 000000000000..201209101aa8 --- /dev/null +++ b/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)) \ No newline at end of file diff --git a/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts b/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts index 54f98870a658..afc4a18ac89a 100644 --- a/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts +++ b/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts @@ -38,10 +38,10 @@ const createSetupContractMock = () => { isAutoRefreshSelectorEnabled: jest.fn(), isTimeRangeSelectorEnabled: jest.fn(), getEnabledUpdated$: jest.fn(), - getTimeUpdate$: jest.fn(), - getRefreshIntervalUpdate$: jest.fn(), + getTimeUpdate$: jest.fn(() => new Observable()), + getRefreshIntervalUpdate$: jest.fn(() => new Observable()), getAutoRefreshFetch$: jest.fn(() => new Observable()), - getFetch$: jest.fn(), + getFetch$: jest.fn(() => new Observable()), getTime: jest.fn(), setTime: jest.fn(), setRefreshInterval: jest.fn(), diff --git a/src/plugins/discover/public/application/view_components/utils/use_search.test.tsx b/src/plugins/discover/public/application/view_components/utils/use_search.test.tsx new file mode 100644 index 000000000000..7f0d95296373 --- /dev/null +++ b/src/plugins/discover/public/application/view_components/utils/use_search.test.tsx @@ -0,0 +1,170 @@ +/* + * 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 ( + + {children} + + ); +}; + +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, waitForNextUpdate } = renderHook(() => useSearch(services), { wrapper }); + + expect(result.current.data$.getValue()).toEqual( + expect.objectContaining({ status: ResultStatus.LOADING }) + ); + + // useSearch updates state async in useEffect, wait for it to finish to + // avoid warning + await act(async () => { + await waitForNextUpdate(); + }); + }); + + 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, waitForNextUpdate } = renderHook(() => useSearch(services), { wrapper }); + expect(result.current.data$.getValue()).toEqual( + expect.objectContaining({ status: ResultStatus.UNINITIALIZED }) + ); + + await act(async () => { + await waitForNextUpdate(); + }); + }); + + it('should update startTime when hook rerenders', async () => { + const services = createMockServices(); + + const { result, rerender, waitForNextUpdate } = 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); + + await act(async () => { + await waitForNextUpdate(); + }); + }); + + 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 }) + ); + }); +}); diff --git a/src/plugins/discover/public/application/view_components/utils/use_search.ts b/src/plugins/discover/public/application/view_components/utils/use_search.ts index aff68c36726c..b8480bb03245 100644 --- a/src/plugins/discover/public/application/view_components/utils/use_search.ts +++ b/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 @@ export const useSearch = (services: DiscoverViewServices) => { : 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(), []); const fetch = useCallback(async () => { diff --git a/src/plugins/discover/public/mocks.ts b/src/plugins/discover/public/mocks.ts index 4724ced290ff..27c0043b8c94 100644 --- a/src/plugins/discover/public/mocks.ts +++ b/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; export type Start = jest.Mocked; @@ -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, };