From 3e28d3c3eb8796e6d8d6fad07d18cc9f4810a843 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 23 Feb 2021 17:24:48 +0100 Subject: [PATCH] [Discover] Fix index pattern switch behavior (#92131) --- .../sidebar/discover_index_pattern.tsx | 11 +-- .../components/sidebar/discover_sidebar.tsx | 2 - .../sidebar/discover_sidebar_responsive.tsx | 1 - ...get_switch_index_pattern_app_state.test.ts | 80 ++++++++++++------- .../get_switch_index_pattern_app_state.ts | 18 +++-- 5 files changed, 67 insertions(+), 45 deletions(-) diff --git a/src/plugins/discover/public/application/components/sidebar/discover_index_pattern.tsx b/src/plugins/discover/public/application/components/sidebar/discover_index_pattern.tsx index ea3e35f607be41..021d5a0252f7ca 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_index_pattern.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_index_pattern.tsx @@ -19,7 +19,7 @@ import { IndexPatternRef } from './types'; import { ChangeIndexPattern } from './change_indexpattern'; import { getSwitchIndexPatternAppState } from '../../helpers/get_switch_index_pattern_app_state'; import { SortPairArr } from '../../angular/doc_table/lib/get_sort'; -import { MODIFY_COLUMNS_ON_SWITCH } from '../../../../common'; +import { MODIFY_COLUMNS_ON_SWITCH, SORT_DEFAULT_ORDER_SETTING } from '../../../../common'; import { AppState } from '../../angular/discover_state'; export interface DiscoverIndexPatternProps { /** @@ -46,10 +46,6 @@ export interface DiscoverIndexPatternProps { * Discover App state */ state: AppState; - /** - * Read from the Fields API - */ - useNewFieldsApi?: boolean; } /** @@ -62,7 +58,6 @@ export function DiscoverIndexPattern({ indexPatterns, state, setAppState, - useNewFieldsApi, }: DiscoverIndexPatternProps) { const options: IndexPatternRef[] = (indexPatternList || []).map((entity) => ({ id: entity.id, @@ -80,12 +75,12 @@ export function DiscoverIndexPattern({ state.columns || [], (state.sort || []) as SortPairArr[], config.get(MODIFY_COLUMNS_ON_SWITCH), - useNewFieldsApi + config.get(SORT_DEFAULT_ORDER_SETTING) ); setAppState(nextAppState); } }, - [selectedIndexPattern, state, config, indexPatterns, setAppState, useNewFieldsApi] + [selectedIndexPattern, state, config, indexPatterns, setAppState] ); const [selected, setSelected] = useState({ diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx index c0a192550e6c4a..138122d80c7657 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx @@ -165,7 +165,6 @@ export function DiscoverSidebar({ indexPatterns={indexPatterns} state={state} setAppState={setAppState} - useNewFieldsApi={useNewFieldsApi} /> ); @@ -195,7 +194,6 @@ export function DiscoverSidebar({ indexPatterns={indexPatterns} state={state} setAppState={setAppState} - useNewFieldsApi={useNewFieldsApi} /> diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx index f0e7c71f9c970d..0808ef47c0dc17 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx @@ -160,7 +160,6 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps) indexPatterns={props.indexPatterns} state={props.state} setAppState={props.setAppState} - useNewFieldsApi={props.useNewFieldsApi} /> diff --git a/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.test.ts b/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.test.ts index de95c1343c99f7..31addd807b78dc 100644 --- a/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.test.ts +++ b/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.test.ts @@ -7,36 +7,33 @@ */ import { getSwitchIndexPatternAppState } from './get_switch_index_pattern_app_state'; -import { IIndexPatternFieldList, IndexPattern } from '../../../../data/common/index_patterns'; +import { IndexPattern } from '../../../../data/common/index_patterns'; -const currentIndexPattern: IndexPattern = { - id: 'prev', - getFieldByName(name) { - return this.fields.getByName(name); - }, - fields: { - getByName: (name: string) => { - const fields = [ - { name: 'category', sortable: true }, - { name: 'name', sortable: true }, - ] as IIndexPatternFieldList; - return fields.find((field) => field.name === name); +/** + * Helper function returning an index pattern + */ +const getIndexPattern = (id: string, timeFieldName: string, fields: string[]) => { + return { + id, + timeFieldName, + getFieldByName(name) { + return this.fields.getByName(name); }, - }, -} as IndexPattern; - -const nextIndexPattern = { - id: 'next', - getFieldByName(name) { - return this.fields.getByName(name); - }, - fields: { - getByName: (name: string) => { - const fields = [{ name: 'category', sortable: true }] as IIndexPatternFieldList; - return fields.find((field) => field.name === name); + fields: { + getByName: (name: string) => { + return fields + .map((field) => ({ + name: field, + sortable: true, + })) + .find((field) => field.name === name); + }, }, - }, -} as IndexPattern; + } as IndexPattern; +}; + +const currentIndexPattern = getIndexPattern('curr', '', ['category', 'name']); +const nextIndexPattern = getIndexPattern('next', '', ['category']); describe('Discover getSwitchIndexPatternAppState', () => { test('removing fields that are not part of the next index pattern, keeping unknown fields ', async () => { @@ -59,10 +56,10 @@ describe('Discover getSwitchIndexPatternAppState', () => { ['name', 'asc'], ] ); - expect(result.columns).toEqual(['_source']); + expect(result.columns).toEqual([]); expect(result.sort).toEqual([['category', 'desc']]); }); - test('removing sorted by fields that without modifying columns', async () => { + test('removing sorted by fields not available in the next index pattern without modifying columns', async () => { const result = getSwitchIndexPatternAppState( currentIndexPattern, nextIndexPattern, @@ -76,4 +73,29 @@ describe('Discover getSwitchIndexPatternAppState', () => { expect(result.columns).toEqual(['name']); expect(result.sort).toEqual([['category', 'desc']]); }); + test('keep sorting by timefield when switching between index patterns with different timeFields', async () => { + const current = getIndexPattern('a', 'timeFieldA', ['timeFieldA']); + const next = getIndexPattern('b', 'timeFieldB', ['timeFieldB']); + + const result = getSwitchIndexPatternAppState(current, next, [], [['timeFieldA', 'desc']]); + expect(result.columns).toEqual([]); + expect(result.sort).toEqual([['timeFieldB', 'desc']]); + }); + test('remove sorting by timefield when switching to an index pattern without timefield that contains the timefield column', async () => { + // Why: timefield column is prepended, keeping the sort, user would need to add the column to remove sorting in legacy grid + const current = getIndexPattern('a', 'timeFieldA', ['timeFieldA']); + const next = getIndexPattern('b', '', ['timeFieldA']); + + const result = getSwitchIndexPatternAppState(current, next, [], [['timeFieldA', 'desc']]); + expect(result.columns).toEqual([]); + expect(result.sort).toEqual([]); + }); + test('add sorting by timefield when switching from an index pattern without timefield to an indexpattern with timefield', async () => { + const current = getIndexPattern('b', '', ['timeFieldA']); + const next = getIndexPattern('a', 'timeFieldA', ['timeFieldA']); + + const result = getSwitchIndexPatternAppState(current, next, [], []); + expect(result.columns).toEqual([]); + expect(result.sort).toEqual([['timeFieldA', 'desc']]); + }); }); diff --git a/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.ts b/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.ts index 32d0bda4260e18..9756fc8fd448ba 100644 --- a/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.ts +++ b/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.ts @@ -20,7 +20,7 @@ export function getSwitchIndexPatternAppState( currentColumns: string[], currentSort: SortPairArr[], modifyColumns: boolean = true, - useNewFieldsApi: boolean = false + sortDirection: string = 'desc' ) { const nextColumns = modifyColumns ? currentColumns.filter( @@ -28,12 +28,20 @@ export function getSwitchIndexPatternAppState( nextIndexPattern.fields.getByName(column) || !currentIndexPattern.fields.getByName(column) ) : currentColumns; - const nextSort = getSortArray(currentSort, nextIndexPattern); - const defaultColumns = useNewFieldsApi ? [] : ['_source']; - const columns = nextColumns.length ? nextColumns : defaultColumns; + const columns = nextColumns.length ? nextColumns : []; + // when switching from an index pattern with timeField to an index pattern without timeField + // filter out sorting by timeField in case it is set. index patterns without timeField don't + // prepend this field in the table, so in legacy grid you would need to add this column to + // remove sorting + const nextSort = getSortArray(currentSort, nextIndexPattern).filter((value) => { + return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName; + }); return { index: nextIndexPattern.id, columns, - sort: nextSort, + sort: + nextIndexPattern.timeFieldName && !nextSort.length + ? [[nextIndexPattern.timeFieldName, sortDirection]] + : nextSort, }; }