Skip to content

Commit

Permalink
[Discover][Main] Improve state related code (#102028) (#102959)
Browse files Browse the repository at this point in the history
Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>
  • Loading branch information
kibanamachine and kertal authored Jun 22, 2021
1 parent ba736fc commit 11dec59
Show file tree
Hide file tree
Showing 14 changed files with 323 additions and 325 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { createSearchSourceMock } from '../../../../../../../data/common/search/
import { IndexPattern, IndexPatternAttributes } from '../../../../../../../data/common';
import { SavedObject } from '../../../../../../../../core/types';
import { indexPatternWithTimefieldMock } from '../../../../../__mocks__/index_pattern_with_timefield';
import { DiscoverSearchSessionManager } from '../../services/discover_search_session';
import { GetStateReturn } from '../../services/discover_state';
import { DiscoverLayoutProps } from './types';
import { SavedSearchDataSubject } from '../../services/use_saved_search';
Expand Down Expand Up @@ -50,11 +49,12 @@ function getProps(indexPattern: IndexPattern): DiscoverLayoutProps {
indexPattern,
indexPatternList,
navigateTo: jest.fn(),
onChangeIndexPattern: jest.fn(),
onUpdateQuery: jest.fn(),
resetQuery: jest.fn(),
savedSearch: savedSearchMock,
savedSearchData$: savedSearch$,
savedSearchRefetch$: new Subject(),
searchSessionManager: {} as DiscoverSearchSessionManager,
searchSource: searchSourceMock,
services,
state: { columns: [] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,8 @@ import { SortPairArr } from '../../../../angular/doc_table/lib/get_sort';
import {
DOC_HIDE_TIME_COLUMN_SETTING,
DOC_TABLE_LEGACY,
MODIFY_COLUMNS_ON_SWITCH,
SAMPLE_SIZE_SETTING,
SEARCH_FIELDS_FROM_SOURCE,
SORT_DEFAULT_ORDER_SETTING,
} from '../../../../../../common';
import { popularizeField } from '../../../../helpers/popularize_field';
import { DocViewFilterFn } from '../../../../doc_views/doc_views_types';
Expand All @@ -52,7 +50,6 @@ import { InspectorSession } from '../../../../../../../inspector/public';
import { DiscoverUninitialized } from '../uninitialized/uninitialized';
import { SavedSearchDataMessage } from '../../services/use_saved_search';
import { useDataGridColumns } from '../../../../helpers/use_data_grid_columns';
import { getSwitchIndexPatternAppState } from '../../utils/get_switch_index_pattern_app_state';
import { FetchStatus } from '../../../../types';

const DocTableLegacyMemoized = React.memo(DocTableLegacy);
Expand All @@ -72,26 +69,20 @@ export function DiscoverLayout({
indexPattern,
indexPatternList,
navigateTo,
onChangeIndexPattern,
onUpdateQuery,
savedSearchRefetch$,
resetQuery,
savedSearchData$,
savedSearch,
searchSessionManager,
searchSource,
services,
state,
stateContainer,
}: DiscoverLayoutProps) {
const {
trackUiMetric,
capabilities,
indexPatterns,
data,
uiSettings: config,
filterManager,
} = services;
const { trackUiMetric, capabilities, indexPatterns, data, uiSettings, filterManager } = services;

const sampleSize = useMemo(() => config.get(SAMPLE_SIZE_SETTING), [config]);
const sampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]);
const [expandedDoc, setExpandedDoc] = useState<ElasticSearchHit | undefined>(undefined);
const [inspectorSession, setInspectorSession] = useState<InspectorSession | undefined>(undefined);
const scrollableDesktop = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -121,42 +112,21 @@ export function DiscoverLayout({
};
}, [savedSearchData$, fetchState]);

const isMobile = () => {
// collapse icon isn't displayed in mobile view, use it to detect which view is displayed
return collapseIcon && !collapseIcon.current;
};
// collapse icon isn't displayed in mobile view, use it to detect which view is displayed
const isMobile = () => collapseIcon && !collapseIcon.current;
const timeField = useMemo(() => {
return indexPatternsUtils.isDefault(indexPattern) ? indexPattern.timeFieldName : undefined;
}, [indexPattern]);

const [isSidebarClosed, setIsSidebarClosed] = useState(false);
const isLegacy = useMemo(() => services.uiSettings.get(DOC_TABLE_LEGACY), [services]);
const useNewFieldsApi = useMemo(() => !services.uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [
services,
]);

const unmappedFieldsConfig = useMemo(
() => ({
showUnmappedFields: useNewFieldsApi,
}),
[useNewFieldsApi]
);
const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]);
const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]);

const resultState = useMemo(() => getResultState(fetchStatus, rows!), [fetchStatus, rows]);

const updateQuery = useCallback(
(_payload, isUpdate?: boolean) => {
if (isUpdate === false) {
searchSessionManager.removeSearchSessionIdFromURL({ replace: false });
savedSearchRefetch$.next();
}
},
[savedSearchRefetch$, searchSessionManager]
);

const { columns, onAddColumn, onRemoveColumn, onMoveColumn, onSetColumns } = useDataGridColumns({
capabilities,
config,
config: uiSettings,
indexPattern,
indexPatterns,
setAppState: stateContainer.setAppState,
Expand Down Expand Up @@ -243,42 +213,8 @@ export function DiscoverLayout({

const contentCentered = resultState === 'uninitialized';
const showTimeCol = useMemo(
() => !config.get(DOC_HIDE_TIME_COLUMN_SETTING, false) && !!indexPattern.timeFieldName,
[config, indexPattern.timeFieldName]
);

const onChangeIndexPattern = useCallback(
async (id: string) => {
const nextIndexPattern = await indexPatterns.get(id);
if (nextIndexPattern && indexPattern) {
/**
* Without resetting the fetch state, e.g. a time column would be displayed when switching
* from a index pattern without to a index pattern with time filter for a brief moment
* That's because appState is updated before savedSearchData$
* The following line of code catches this, but should be improved
*/
savedSearchData$.next({ rows: [], state: FetchStatus.LOADING, fieldCounts: {} });

const nextAppState = getSwitchIndexPatternAppState(
indexPattern,
nextIndexPattern,
state.columns || [],
(state.sort || []) as SortPairArr[],
config.get(MODIFY_COLUMNS_ON_SWITCH),
config.get(SORT_DEFAULT_ORDER_SETTING)
);
stateContainer.setAppState(nextAppState);
}
},
[
config,
indexPattern,
indexPatterns,
savedSearchData$,
state.columns,
state.sort,
stateContainer,
]
() => !uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false) && !!indexPattern.timeFieldName,
[uiSettings, indexPattern.timeFieldName]
);

return (
Expand All @@ -294,7 +230,7 @@ export function DiscoverLayout({
searchSource={searchSource}
services={services}
stateContainer={stateContainer}
updateQuery={updateQuery}
updateQuery={onUpdateQuery}
/>
<EuiPageBody className="dscPageBody" aria-describedby="savedSearchTitle">
<h1 id="savedSearchTitle" className="euiScreenReaderOnly">
Expand All @@ -316,7 +252,6 @@ export function DiscoverLayout({
state={state}
isClosed={isSidebarClosed}
trackUiMetric={trackUiMetric}
unmappedFieldsConfig={unmappedFieldsConfig}
useNewFieldsApi={useNewFieldsApi}
onEditRuntimeField={onEditRuntimeField}
/>
Expand Down Expand Up @@ -373,7 +308,7 @@ export function DiscoverLayout({
>
<EuiFlexItem grow={false}>
<DiscoverChartMemoized
config={config}
config={uiSettings}
chartData={fetchState.chartData}
bucketInterval={fetchState.bucketInterval}
data={data}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
import {
IndexPattern,
IndexPatternAttributes,
Query,
SavedObject,
TimeRange,
} from '../../../../../../../data/common';
import { ISearchSource } from '../../../../../../../data/public';
import { DiscoverSearchSessionManager } from '../../services/discover_search_session';
import { AppState, GetStateReturn } from '../../services/discover_state';
import { SavedSearchRefetchSubject, SavedSearchDataSubject } from '../../services/use_saved_search';
import { DiscoverServices } from '../../../../../build_services';
Expand All @@ -21,12 +22,13 @@ import { SavedSearch } from '../../../../../saved_searches';
export interface DiscoverLayoutProps {
indexPattern: IndexPattern;
indexPatternList: Array<SavedObject<IndexPatternAttributes>>;
resetQuery: () => void;
navigateTo: (url: string) => void;
onChangeIndexPattern: (id: string) => void;
onUpdateQuery: (payload: { dateRange: TimeRange; query?: Query }, isUpdate?: boolean) => void;
resetQuery: () => void;
savedSearch: SavedSearch;
savedSearchData$: SavedSearchDataSubject;
savedSearchRefetch$: SavedSearchRefetchSubject;
searchSessionManager: DiscoverSearchSessionManager;
searchSource: ISearchSource;
services: DiscoverServices;
state: AppState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export function DiscoverSidebar({
trackUiMetric,
useNewFieldsApi = false,
useFlyout = false,
unmappedFieldsConfig,
onEditRuntimeField,
onChangeIndexPattern,
setFieldEditorRef,
Expand Down Expand Up @@ -129,25 +128,8 @@ export function DiscoverSidebar({
popular: popularFields,
unpopular: unpopularFields,
} = useMemo(
() =>
groupFields(
fields,
columns,
popularLimit,
fieldCounts,
fieldFilter,
useNewFieldsApi,
!!unmappedFieldsConfig?.showUnmappedFields
),
[
fields,
columns,
popularLimit,
fieldCounts,
fieldFilter,
useNewFieldsApi,
unmappedFieldsConfig?.showUnmappedFields,
]
() => groupFields(fields, columns, popularLimit, fieldCounts, fieldFilter, useNewFieldsApi),
[fields, columns, popularLimit, fieldCounts, fieldFilter, useNewFieldsApi]
);

const paginate = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
} from './discover_sidebar_responsive';
import { DiscoverServices } from '../../../../../build_services';
import { ElasticSearchHit } from '../../../../doc_views/doc_views_types';
import { DiscoverSidebar } from './discover_sidebar';

const mockServices = ({
history: () => ({
Expand Down Expand Up @@ -132,14 +131,4 @@ describe('discover responsive sidebar', function () {
findTestSubject(comp, 'plus-extension-gif').simulate('click');
expect(props.onAddFilter).toHaveBeenCalled();
});
it('renders sidebar with unmapped fields config', function () {
const unmappedFieldsConfig = {
showUnmappedFields: false,
};
const componentProps = { ...props, unmappedFieldsConfig };
const component = mountWithIntl(<DiscoverSidebarResponsive {...componentProps} />);
const discoverSidebar = component.find(DiscoverSidebar);
expect(discoverSidebar).toHaveLength(1);
expect(discoverSidebar.props().unmappedFieldsConfig).toEqual(unmappedFieldsConfig);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,6 @@ export interface DiscoverSidebarResponsiveProps {
* Read from the Fields API
*/
useNewFieldsApi?: boolean;
/**
* an object containing properties for proper handling of unmapped fields
*/
unmappedFieldsConfig?: {
/**
* determines whether to display unmapped fields
*/
showUnmappedFields: boolean;
};
/**
* callback to execute on edit runtime field
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ describe('group_fields', function () {
5,
fieldCounts,
fieldFilterState,
true,
false
true
);
expect(actual.unpopular).toEqual([]);
});
Expand All @@ -270,8 +269,7 @@ describe('group_fields', function () {
5,
fieldCounts,
fieldFilterState,
false,
undefined
false
);
expect(actual.unpopular.map((field) => field.name)).toEqual(['unknown_field']);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export function groupFields(
popularLimit: number,
fieldCounts: Record<string, number>,
fieldFilterState: FieldFilterState,
useNewFieldsApi: boolean,
showUnmappedFields = true
useNewFieldsApi: boolean
): GroupedFields {
const showUnmappedFields = useNewFieldsApi;
const result: GroupedFields = {
selected: [],
popular: [],
Expand Down
Loading

0 comments on commit 11dec59

Please sign in to comment.