From b4008238892214b4c9f1edc11d6fa03c2777de89 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 23 Nov 2021 11:48:58 -0500 Subject: [PATCH] [Discover] Fix timefield sorting when switching similar index patterns (#116145) (#119492) * [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 Co-authored-by: Dmitry Tomashevich <39378793+Dmitriynj@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 8d3fcbf7d0ced..2998ca6fe9ca1 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 = ''; @@ -290,11 +298,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/apps/main/utils/get_switch_index_pattern_app_state.test.ts b/src/plugins/discover/public/application/apps/main/utils/get_switch_index_pattern_app_state.test.ts index 83107d6c57ab8..87be8c39c906a 100644 --- a/src/plugins/discover/public/application/apps/main/utils/get_switch_index_pattern_app_state.test.ts +++ b/src/plugins/discover/public/application/apps/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/apps/main/utils/get_switch_index_pattern_app_state.ts b/src/plugins/discover/public/application/apps/main/utils/get_switch_index_pattern_app_state.ts index ff082587172a0..1e75dbeb7a9d4 100644 --- a/src/plugins/discover/public/application/apps/main/utils/get_switch_index_pattern_app_state.ts +++ b/src/plugins/discover/public/application/apps/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, }; }