From aed58ae2e32a11e8a88415cee4afc54c23d6f05f Mon Sep 17 00:00:00 2001 From: Dmitry Tomashevich <39378793+Dmitriynj@users.noreply.github.com> Date: Tue, 23 Nov 2021 18:28:05 +0300 Subject: [PATCH] [Discover] Fix timefield sorting when switching similar index patterns (#116145) * [Discover] fix timefield sorting * [Discover] resolve review comments * [Discover] replace primary time field for sorting * [Discover] improve solution * Improve typing and time based checking * Fix missing isTimeBased call * [Discover] fix linting * [Discover] replace time fields when switching between time based data views * [Discover] apply suggestions Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Tim Roes --- .../data_views/common/data_views/data_view.ts | 12 ++++++++-- src/plugins/data_views/common/index.ts | 2 +- ...get_switch_index_pattern_app_state.test.ts | 11 +++++++++ .../get_switch_index_pattern_app_state.ts | 23 +++++++++++++++---- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/plugins/data_views/common/data_views/data_view.ts b/src/plugins/data_views/common/data_views/data_view.ts index 76cc54e984712..921a5897dd232 100644 --- a/src/plugins/data_views/common/data_views/data_view.ts +++ b/src/plugins/data_views/common/data_views/data_view.ts @@ -44,6 +44,14 @@ interface SavedObjectBody { type?: string; } +/** + * An interface representing a data view that is time based. + */ +export interface TimeBasedDataView extends DataView { + timeFieldName: NonNullable; + getTimeField: () => DataViewField; +} + export class DataView implements IIndexPattern { public id?: string; public title: string = ''; @@ -286,11 +294,11 @@ export class DataView implements IIndexPattern { return [...this.fields.getAll().filter((field) => field.scripted)]; } - isTimeBased(): boolean { + isTimeBased(): this is TimeBasedDataView { return !!this.timeFieldName && (!this.fields || !!this.getTimeField()); } - isTimeNanosBased(): boolean { + isTimeNanosBased(): this is TimeBasedDataView { const timeField = this.getTimeField(); return !!(timeField && timeField.esTypes && timeField.esTypes.indexOf('date_nanos') !== -1); } diff --git a/src/plugins/data_views/common/index.ts b/src/plugins/data_views/common/index.ts index b3123df2597cc..25d2ddd874256 100644 --- a/src/plugins/data_views/common/index.ts +++ b/src/plugins/data_views/common/index.ts @@ -57,7 +57,7 @@ export type { export { DataViewType, IndexPatternType } from './types'; export type { IndexPatternsContract, DataViewsContract } from './data_views'; export { IndexPatternsService, DataViewsService } from './data_views'; -export type { IndexPatternListItem, DataViewListItem } from './data_views'; +export type { IndexPatternListItem, DataViewListItem, TimeBasedDataView } from './data_views'; export { IndexPattern, DataView } from './data_views'; export { DuplicateDataViewError, DataViewSavedObjectConflictError } from './errors'; export type { diff --git a/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.test.ts b/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.test.ts index 412ad060b5565..d37ccb808c966 100644 --- a/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.test.ts +++ b/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.test.ts @@ -16,6 +16,9 @@ const getIndexPattern = (id: string, timeFieldName: string, fields: string[]) => return { id, timeFieldName, + isTimeBased() { + return !!timeFieldName; + }, getFieldByName(name) { return this.fields.getByName(name); }, @@ -98,4 +101,12 @@ describe('Discover getSwitchIndexPatternAppState', () => { expect(result.columns).toEqual([]); expect(result.sort).toEqual([['timeFieldA', 'desc']]); }); + test('should change sorting for similar index patterns', async () => { + const current = getIndexPattern('timebased', 'timefield', ['timefield']); + const next = getIndexPattern('timebased2', 'timefield2', ['timefield', 'timefield2']); + + const result = getSwitchIndexPatternAppState(current, next, [], [['timefield', 'desc']]); + expect(result.columns).toEqual([]); + expect(result.sort).toEqual([['timefield2', 'desc']]); + }); }); diff --git a/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.ts b/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.ts index b6dfe7a63f3a8..6da410908c650 100644 --- a/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.ts +++ b/src/plugins/discover/public/application/main/utils/get_switch_index_pattern_app_state.ts @@ -28,19 +28,32 @@ export function getSwitchIndexPatternAppState( ) : currentColumns; 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) => { + let nextSort = getSortArray(currentSort, nextIndexPattern).filter((value) => { return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName; }); + + if (nextIndexPattern.isTimeBased() && !nextSort.length) { + // set default sorting if it was not set + nextSort = [[nextIndexPattern.timeFieldName, sortDirection]]; + } else if ( + nextIndexPattern.isTimeBased() && + currentIndexPattern.isTimeBased() && + nextIndexPattern.timeFieldName !== currentIndexPattern.timeFieldName + ) { + // switch time fields + nextSort = nextSort.map((cur) => + cur[0] === currentIndexPattern.timeFieldName ? [nextIndexPattern.timeFieldName, cur[1]] : cur + ); + } + return { index: nextIndexPattern.id, columns, - sort: - nextIndexPattern.timeFieldName && !nextSort.length - ? [[nextIndexPattern.timeFieldName, sortDirection]] - : nextSort, + sort: nextSort, }; }