Skip to content

Commit

Permalink
[Discover] Migrate expandedDoc to internalStateContainer (#153369)
Browse files Browse the repository at this point in the history
## Summary
Refactor stateful expandedDoc prop to internalStateContainer & rename AppState, DataStateContainer, and InternalStateContainer for better consistency.
  • Loading branch information
kertal authored Mar 22, 2023
1 parent 84890fb commit fba936c
Show file tree
Hide file tree
Showing 25 changed files with 97 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import React, { useState } from 'react';
import { storiesOf } from '@storybook/react';
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
import { DiscoverMainProvider } from '../../../services/discover_state_provider';
import { AppState } from '../../../services/discover_app_state_container';
import { DiscoverAppState } from '../../../services/discover_app_state_container';
import { getDataViewMock } from '../../../../../__mocks__/__storybook_mocks__/get_data_view_mock';
import { withDiscoverServices } from '../../../../../__mocks__/__storybook_mocks__/with_discover_services';
import { getDocumentsLayoutProps, getPlainRecordLayoutProps } from './get_layout_props';
Expand All @@ -23,7 +23,7 @@ setHeaderActionMenuMounter(() => void 0);
const DiscoverLayoutStory = (layoutProps: DiscoverLayoutProps) => {
const [state, setState] = useState({});

const update = (newState: Partial<AppState>) => {
const update = (newState: Partial<DiscoverAppState>) => {
setState((prevState) => ({ ...prevState, ...newState }));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { buildDataTableRecord } from '../../../../utils/build_data_record';
import { EsHitRecord } from '../../../../types';
import { DiscoverMainProvider } from '../../services/discover_state_provider';
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
import { AppState } from '../../services/discover_app_state_container';
import { DiscoverAppState } from '../../services/discover_app_state_container';

setHeaderActionMenuMounter(jest.fn());

Expand Down Expand Up @@ -84,7 +84,7 @@ describe('Discover documents layout', () => {
test('should set rounded width to state on resize column', () => {
const state = {
grid: { columns: { timestamp: { width: 173 }, someField: { width: 197 } } },
} as AppState;
} as DiscoverAppState;
const container = getDiscoverStateMock({});
container.appState.update(state);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
import { FormattedMessage } from '@kbn/i18n-react';
import { DataView } from '@kbn/data-views-plugin/public';
import { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public';
import { DataTableRecord } from '../../../../types';
import { useInternalStateSelector } from '../../services/discover_internal_state_container';
import { useAppStateSelector } from '../../services/discover_app_state_container';
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
import { DocViewFilterFn } from '../../../../services/doc_views/doc_views_types';
Expand All @@ -36,7 +38,6 @@ import { DocTableInfinite } from '../../../../components/doc_table/doc_table_inf
import { DocumentExplorerCallout } from '../document_explorer_callout';
import { DocumentExplorerUpdateCallout } from '../document_explorer_callout/document_explorer_update_callout';
import { DiscoverTourProvider } from '../../../../components/discover_tour';
import { DataTableRecord } from '../../../../types';
import { getRawRecordType } from '../../utils/get_raw_record_type';
import { DiscoverGridFlyout } from '../../../../components/discover_grid/discover_grid_flyout';
import { DocViewer } from '../../../../services/doc_views/components/doc_viewer';
Expand All @@ -60,20 +61,16 @@ export const onResize = (
};

function DiscoverDocumentsComponent({
expandedDoc,
dataView,
onAddFilter,
savedSearch,
setExpandedDoc,
stateContainer,
onFieldEdited,
}: {
expandedDoc?: DataTableRecord;
dataView: DataView;
navigateTo: (url: string) => void;
onAddFilter?: DocViewFilterFn;
savedSearch: SavedSearch;
setExpandedDoc: (doc?: DataTableRecord) => void;
stateContainer: DiscoverStateContainer;
onFieldEdited?: () => void;
}) {
Expand All @@ -93,6 +90,14 @@ function DiscoverDocumentsComponent({
];
}
);
const setExpandedDoc = useCallback(
(doc: DataTableRecord | undefined) => {
stateContainer.internalState.transitions.setExpandedDoc(doc);
},
[stateContainer]
);

const expandedDoc = useInternalStateSelector((state) => state.expandedDoc);

const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]);
const hideAnnouncements = useMemo(() => uiSettings.get(HIDE_ANNOUNCEMENTS), [uiSettings]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ const mountComponent = async ({
isPlainRecord,
dataView: dataViewMock,
navigateTo: jest.fn(),
setExpandedDoc: jest.fn(),
savedSearch,
stateContainer,
onFieldEdited: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,9 @@ const TopNavMemoized = React.memo(DiscoverTopNav);

export function DiscoverLayout({
inspectorAdapters,
expandedDoc,
navigateTo,
onChangeDataView,
onUpdateQuery,
setExpandedDoc,
resetSavedSearch,
savedSearch,
searchSource,
Expand Down Expand Up @@ -122,8 +120,8 @@ export function DiscoverLayout({
);

const onOpenInspector = useInspector({
setExpandedDoc,
inspector,
stateContainer,
inspectorAdapters,
savedSearch,
});
Expand Down Expand Up @@ -239,8 +237,6 @@ export function DiscoverLayout({
dataView={dataView}
navigateTo={navigateTo}
resetSavedSearch={resetSavedSearch}
expandedDoc={expandedDoc}
setExpandedDoc={setExpandedDoc}
savedSearch={savedSearch}
stateContainer={stateContainer}
columns={currentColumns}
Expand All @@ -257,7 +253,6 @@ export function DiscoverLayout({
currentColumns,
data,
dataView,
expandedDoc,
inspectorAdapters,
isPlainRecord,
isTimeBased,
Expand All @@ -268,7 +263,6 @@ export function DiscoverLayout({
resetSavedSearch,
resultState,
savedSearch,
setExpandedDoc,
stateContainer,
viewMode,
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ const mountComponent = ({
isPlainRecord,
dataView: dataViewMock,
navigateTo: jest.fn(),
setExpandedDoc: jest.fn(),
savedSearch,
stateContainer,
onFieldEdited: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { METRIC_TYPE } from '@kbn/analytics';
import { i18n } from '@kbn/i18n';
import { VIEW_MODE } from '../../../../../common/constants';
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
import { DataTableRecord } from '../../../../types';
import { DocumentViewModeToggle } from '../../../../components/view_mode_toggle';
import { DocViewFilterFn } from '../../../../services/doc_views/doc_views_types';
import { DiscoverStateContainer } from '../../services/discover_state';
Expand All @@ -30,8 +29,6 @@ export interface DiscoverMainContentProps {
isPlainRecord: boolean;
navigateTo: (url: string) => void;
stateContainer: DiscoverStateContainer;
expandedDoc?: DataTableRecord;
setExpandedDoc: (doc?: DataTableRecord) => void;
viewMode: VIEW_MODE;
onAddFilter: DocViewFilterFn | undefined;
onFieldEdited: () => Promise<void>;
Expand All @@ -42,8 +39,6 @@ export const DiscoverMainContent = ({
dataView,
isPlainRecord,
navigateTo,
expandedDoc,
setExpandedDoc,
viewMode,
onAddFilter,
onFieldEdited,
Expand Down Expand Up @@ -95,12 +90,10 @@ export const DiscoverMainContent = ({
)}
{viewMode === VIEW_MODE.DOCUMENT_LEVEL ? (
<DiscoverDocuments
expandedDoc={expandedDoc}
dataView={dataView}
navigateTo={navigateTo}
onAddFilter={!isPlainRecord ? onAddFilter : undefined}
savedSearch={savedSearch}
setExpandedDoc={setExpandedDoc}
stateContainer={stateContainer}
onFieldEdited={!isPlainRecord ? onFieldEdited : undefined}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query';
import type { DataView } from '@kbn/data-views-plugin/public';
import type { ISearchSource } from '@kbn/data-plugin/public';
import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { DataTableRecord } from '../../../../types';
import { DiscoverStateContainer } from '../../services/discover_state';
import type { InspectorAdapters } from '../../hooks/use_inspector';

Expand All @@ -23,8 +22,6 @@ export interface DiscoverLayoutProps {
isUpdate?: boolean
) => void;
resetSavedSearch: () => void;
expandedDoc?: DataTableRecord;
setExpandedDoc: (doc?: DataTableRecord) => void;
savedSearch: SavedSearch;
searchSource: ISearchSource;
stateContainer: DiscoverStateContainer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import React, { useCallback, useEffect, useState } from 'react';
import React, { useCallback, useEffect } from 'react';
import { useHistory } from 'react-router-dom';
import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { DataViewListItem } from '@kbn/data-views-plugin/public';
Expand All @@ -15,7 +15,6 @@ import { addHelpMenuToAppChrome } from '../../components/help_menu/help_menu_uti
import { useDiscoverState } from './hooks/use_discover_state';
import { useUrl } from './hooks/use_url';
import { useDiscoverServices } from '../../hooks/use_discover_services';
import { DataTableRecord } from '../../types';
import { useSavedSearchAliasMatchRedirect } from '../../hooks/saved_search_alias_match_redirect';
import { DiscoverMainProvider } from './services/discover_state_provider';

Expand All @@ -37,7 +36,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) {
const services = useDiscoverServices();
const { chrome, docLinks, data, spaces, history } = services;
const usedHistory = useHistory();
const [expandedDoc, setExpandedDoc] = useState<DataTableRecord | undefined>(undefined);
const navigateTo = useCallback(
(path: string) => {
usedHistory.push(path);
Expand All @@ -62,7 +60,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) {
services,
history: usedHistory,
savedSearch,
setExpandedDoc,
});

/**
Expand Down Expand Up @@ -107,11 +104,9 @@ export function DiscoverMainApp(props: DiscoverMainProps) {
<DiscoverMainProvider value={stateContainer}>
<DiscoverLayoutMemoized
inspectorAdapters={inspectorAdapters}
expandedDoc={expandedDoc}
onChangeDataView={onChangeDataView}
onUpdateQuery={onUpdateQuery}
resetSavedSearch={resetCurrentSavedSearch}
setExpandedDoc={setExpandedDoc}
navigateTo={navigateTo}
savedSearch={savedSearch}
searchSource={searchSource}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe('test useDiscoverState', () => {
services: discoverServiceMock,
history,
savedSearch: savedSearchMock,
setExpandedDoc: jest.fn(),
});
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,17 @@ import { useUrlTracking } from './use_url_tracking';
import { getDiscoverStateContainer } from '../services/discover_state';
import { getStateDefaults } from '../utils/get_state_defaults';
import { DiscoverServices } from '../../../build_services';
import { DataTableRecord } from '../../../types';
import { restoreStateFromSavedSearch } from '../../../services/saved_searches/restore_from_saved_search';
import { useAdHocDataViews } from './use_adhoc_data_views';

export function useDiscoverState({
services,
history,
savedSearch,
setExpandedDoc,
}: {
services: DiscoverServices;
savedSearch: SavedSearch;
history: History;
setExpandedDoc: (doc?: DataTableRecord) => void;
}) {
const { data, filterManager, dataViews, toastNotifications, trackUiMetric } = services;

Expand Down Expand Up @@ -150,9 +147,9 @@ export function useDiscoverState({
const onChangeDataView = useCallback(
async (id: string) => {
await changeDataView(id, { services, discoverState: stateContainer, setUrlTracking });
setExpandedDoc(undefined);
stateContainer.internalState.transitions.setExpandedDoc(undefined);
},
[services, setExpandedDoc, setUrlTracking, stateContainer]
[services, setUrlTracking, stateContainer]
);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,44 @@
* Side Public License, v 1.
*/

import { renderHook } from '@testing-library/react-hooks';
import { act, renderHook } from '@testing-library/react-hooks';
import { discoverServiceMock } from '../../../__mocks__/services';
import { savedSearchMock } from '../../../__mocks__/saved_search';
import { useInspector } from './use_inspector';
import { Adapters, RequestAdapter } from '@kbn/inspector-plugin/common';
import { OverlayRef } from '@kbn/core/public';
import { AggregateRequestAdapter } from '../utils/aggregate_request_adapter';
import { getDiscoverStateMock } from '../../../__mocks__/discover_state.mock';
import { DataTableRecord } from '../../../types';

describe('test useInspector', () => {
test('inspector open function is executed, expanded doc is closed', async () => {
const setExpandedDoc = jest.fn();
let adapters: Adapters | undefined;
jest.spyOn(discoverServiceMock.inspector, 'open').mockImplementation((localAdapters) => {
adapters = localAdapters;
return {} as OverlayRef;
return { close: jest.fn() } as unknown as OverlayRef;
});
const requests = new RequestAdapter();
const lensRequests = new RequestAdapter();
const stateContainer = getDiscoverStateMock({ isTimeBased: true });
stateContainer.internalState.transitions.setExpandedDoc({} as unknown as DataTableRecord);
const { result } = renderHook(() => {
return useInspector({
inspectorAdapters: { requests, lensRequests },
savedSearch: savedSearchMock,
inspector: discoverServiceMock.inspector,
setExpandedDoc,
stateContainer,
});
});
result.current();
expect(setExpandedDoc).toHaveBeenCalledWith(undefined);
await act(async () => {
result.current();
});
expect(discoverServiceMock.inspector.open).toHaveBeenCalled();
expect(adapters?.requests).toBeInstanceOf(AggregateRequestAdapter);
expect(adapters?.requests?.getRequests()).toEqual([
...requests.getRequests(),
...lensRequests.getRequests(),
]);
expect(stateContainer.internalState.getState().expandedDoc).toBe(undefined);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
Start as InspectorPublicPluginStart,
} from '@kbn/inspector-plugin/public';
import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { DataTableRecord } from '../../../types';
import { DiscoverStateContainer } from '../services/discover_state';
import { AggregateRequestAdapter } from '../utils/aggregate_request_adapter';

export interface InspectorAdapters {
Expand All @@ -22,21 +22,21 @@ export interface InspectorAdapters {
}

export function useInspector({
setExpandedDoc,
inspector,
stateContainer,
inspectorAdapters,
savedSearch,
}: {
inspectorAdapters: InspectorAdapters;
savedSearch: SavedSearch;
setExpandedDoc: (doc?: DataTableRecord) => void;
inspector: InspectorPublicPluginStart;
stateContainer: DiscoverStateContainer;
}) {
const [inspectorSession, setInspectorSession] = useState<InspectorSession | undefined>(undefined);

const onOpenInspector = useCallback(() => {
// prevent overlapping
setExpandedDoc(undefined);
stateContainer.internalState.transitions.setExpandedDoc(undefined);

const requestAdapters = inspectorAdapters.lensRequests
? [inspectorAdapters.requests, inspectorAdapters.lensRequests]
Expand All @@ -49,7 +49,7 @@ export function useInspector({

setInspectorSession(session);
}, [
setExpandedDoc,
stateContainer,
inspectorAdapters.lensRequests,
inspectorAdapters.requests,
inspector,
Expand Down
Loading

0 comments on commit fba936c

Please sign in to comment.