From 4da82801998aa54697c63d7c654188c63b87f009 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Mon, 25 Oct 2021 17:23:37 +0300 Subject: [PATCH 1/9] [Discover] fix timefield sorting --- .../main/utils/get_switch_index_pattern_app_state.test.ts | 8 ++++++++ .../apps/main/utils/get_switch_index_pattern_app_state.ts | 7 +++---- 2 files changed, 11 insertions(+), 4 deletions(-) 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..78f0c29d2b7e2 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 @@ -98,4 +98,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..211a1a23f656e 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 @@ -38,9 +38,8 @@ export function getSwitchIndexPatternAppState( return { index: nextIndexPattern.id, columns, - sort: - nextIndexPattern.timeFieldName && !nextSort.length - ? [[nextIndexPattern.timeFieldName, sortDirection]] - : nextSort, + sort: nextIndexPattern.timeFieldName + ? [[nextIndexPattern.timeFieldName, sortDirection]] + : nextSort, }; } From 60dcfa8ffa1692e151ce43cc20552bb213dbec93 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Mon, 15 Nov 2021 23:04:40 +0300 Subject: [PATCH 2/9] [Discover] resolve review comments --- .../get_switch_index_pattern_app_state.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) 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 749b628dcdfed..13e6a5dfa1b46 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,18 +28,20 @@ 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) => { - return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName; - }); + + // remove prev time field sort + let nextSort = getSortArray(currentSort, nextIndexPattern).filter( + (value) => value[0] !== currentIndexPattern.timeFieldName + ); + + // append default sorting if indexPattern has time field + if (nextIndexPattern.timeFieldName) { + nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...nextSort]; + } + return { index: nextIndexPattern.id, columns, - sort: nextIndexPattern.timeFieldName - ? [[nextIndexPattern.timeFieldName, sortDirection]] - : nextSort, + sort: nextSort, }; } From 63252f6a8bae8ae79d28be878301d5e60b46cb00 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Wed, 17 Nov 2021 15:23:13 +0300 Subject: [PATCH 3/9] [Discover] replace primary time field for sorting --- .../utils/get_switch_index_pattern_app_state.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) 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 13e6a5dfa1b46..f2b6601381f63 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 @@ -29,19 +29,16 @@ export function getSwitchIndexPatternAppState( : currentColumns; const columns = nextColumns.length ? nextColumns : []; - // remove prev time field sort - let nextSort = getSortArray(currentSort, nextIndexPattern).filter( - (value) => value[0] !== currentIndexPattern.timeFieldName - ); - - // append default sorting if indexPattern has time field - if (nextIndexPattern.timeFieldName) { - nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...nextSort]; + let nextSort = getSortArray(currentSort, nextIndexPattern); + const [first, ...restSorting] = nextSort; + // replace primary timeFieldName if it was sorted + if (nextIndexPattern.timeFieldName && first && first[0] === currentIndexPattern.timeFieldName) { + nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...restSorting]; } return { index: nextIndexPattern.id, columns, - sort: nextSort, + sort: !nextSort.length ? [[nextIndexPattern.timeFieldName, sortDirection]] : nextSort, }; } From 9ee5c805ef274e57e2d9410c5d0f52fb01efe9ab Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 18 Nov 2021 11:36:14 +0300 Subject: [PATCH 4/9] [Discover] improve solution --- .../get_switch_index_pattern_app_state.ts | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) 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 f2b6601381f63..3d89b37160978 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 @@ -29,16 +29,29 @@ export function getSwitchIndexPatternAppState( : currentColumns; const columns = nextColumns.length ? nextColumns : []; - let nextSort = getSortArray(currentSort, nextIndexPattern); - const [first, ...restSorting] = nextSort; - // replace primary timeFieldName if it was sorted - if (nextIndexPattern.timeFieldName && first && first[0] === currentIndexPattern.timeFieldName) { - nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...restSorting]; + // 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 + let nextSort = getSortArray(currentSort, nextIndexPattern).filter((value) => { + return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName; + }); + + if (nextIndexPattern.timeFieldName && !nextSort.length) { + // set default sorting when it was not changed + nextSort = [[nextIndexPattern.timeFieldName, sortDirection]]; + } else if ( + nextIndexPattern.timeFieldName && + nextSort.length !== 0 && + nextSort[0][0] === currentIndexPattern.timeFieldName + ) { + // replace previous data view timeFieldName with a new one + nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...nextSort.slice(1)]; } return { index: nextIndexPattern.id, columns, - sort: !nextSort.length ? [[nextIndexPattern.timeFieldName, sortDirection]] : nextSort, + sort: nextSort, }; } From 716c895cabfabf3ced719abcfd8e4ab1030d90c8 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 18 Nov 2021 10:07:21 +0100 Subject: [PATCH 5/9] Improve typing and time based checking --- .../data_views/common/data_views/data_view.ts | 12 ++++++++++-- src/plugins/data_views/common/index.ts | 2 +- .../utils/get_switch_index_pattern_app_state.test.ts | 3 +++ .../main/utils/get_switch_index_pattern_app_state.ts | 6 +++--- 4 files changed, 17 insertions(+), 6 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..54a9ad3d8327c 100644 --- a/src/plugins/data_views/common/index.ts +++ b/src/plugins/data_views/common/index.ts @@ -58,7 +58,7 @@ 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 { IndexPattern, DataView } from './data_views'; +export { IndexPattern, DataView, TimeBasedDataView } from './data_views'; export { DuplicateDataViewError, DataViewSavedObjectConflictError } from './errors'; export type { IndexPatternExpressionType, 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 8a6735e1c6d67..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); }, 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 3d89b37160978..dd593357b3fa9 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 @@ -37,15 +37,15 @@ export function getSwitchIndexPatternAppState( return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName; }); - if (nextIndexPattern.timeFieldName && !nextSort.length) { - // set default sorting when it was not changed + if (nextIndexPattern.isTimeBased() && !nextSort.length) { + // set default timefield sorting when there was no new sorting nextSort = [[nextIndexPattern.timeFieldName, sortDirection]]; } else if ( nextIndexPattern.timeFieldName && nextSort.length !== 0 && nextSort[0][0] === currentIndexPattern.timeFieldName ) { - // replace previous data view timeFieldName with a new one + // replace previous data view timeFieldName with new one nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...nextSort.slice(1)]; } From e271656a2ae8312832dd31f5295f2328e67a1ad3 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 18 Nov 2021 10:10:10 +0100 Subject: [PATCH 6/9] Fix missing isTimeBased call --- .../main/utils/get_switch_index_pattern_app_state.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dd593357b3fa9..004b045fa1aa6 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 @@ -41,7 +41,7 @@ export function getSwitchIndexPatternAppState( // set default timefield sorting when there was no new sorting nextSort = [[nextIndexPattern.timeFieldName, sortDirection]]; } else if ( - nextIndexPattern.timeFieldName && + nextIndexPattern.isTimeBased() && nextSort.length !== 0 && nextSort[0][0] === currentIndexPattern.timeFieldName ) { From 74592b796b4c32547bcbe3e6605d32c055e5cc5f Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 18 Nov 2021 13:18:13 +0300 Subject: [PATCH 7/9] [Discover] fix linting --- src/plugins/data_views/common/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/data_views/common/index.ts b/src/plugins/data_views/common/index.ts index 54a9ad3d8327c..25d2ddd874256 100644 --- a/src/plugins/data_views/common/index.ts +++ b/src/plugins/data_views/common/index.ts @@ -57,8 +57,8 @@ 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 { IndexPattern, DataView, TimeBasedDataView } from './data_views'; +export type { IndexPatternListItem, DataViewListItem, TimeBasedDataView } from './data_views'; +export { IndexPattern, DataView } from './data_views'; export { DuplicateDataViewError, DataViewSavedObjectConflictError } from './errors'; export type { IndexPatternExpressionType, From c3512b9353756484a59a203bc6fb54056d95518c Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Mon, 22 Nov 2021 15:25:31 +0300 Subject: [PATCH 8/9] [Discover] replace time fields when switching between time based data views --- .../utils/get_switch_index_pattern_app_state.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 004b045fa1aa6..cb64896c251c7 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 @@ -38,15 +38,15 @@ export function getSwitchIndexPatternAppState( }); if (nextIndexPattern.isTimeBased() && !nextSort.length) { - // set default timefield sorting when there was no new sorting + // set default sorting if it was not set nextSort = [[nextIndexPattern.timeFieldName, sortDirection]]; - } else if ( - nextIndexPattern.isTimeBased() && - nextSort.length !== 0 && - nextSort[0][0] === currentIndexPattern.timeFieldName - ) { - // replace previous data view timeFieldName with new one - nextSort = [[nextIndexPattern.timeFieldName, sortDirection], ...nextSort.slice(1)]; + } else if (nextIndexPattern.isTimeBased() && currentIndexPattern.isTimeBased()) { + // switch time fields + nextSort = nextSort.map((cur) => + cur[0] === currentIndexPattern.timeFieldName + ? [nextIndexPattern.timeFieldName, sortDirection] + : cur + ); } return { From 45137178a5c438f3f5428de538cbd9fbb48f9e87 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Tue, 23 Nov 2021 14:43:34 +0300 Subject: [PATCH 9/9] [Discover] apply suggestions --- .../main/utils/get_switch_index_pattern_app_state.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 cb64896c251c7..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 @@ -40,12 +40,14 @@ export function getSwitchIndexPatternAppState( if (nextIndexPattern.isTimeBased() && !nextSort.length) { // set default sorting if it was not set nextSort = [[nextIndexPattern.timeFieldName, sortDirection]]; - } else if (nextIndexPattern.isTimeBased() && currentIndexPattern.isTimeBased()) { + } else if ( + nextIndexPattern.isTimeBased() && + currentIndexPattern.isTimeBased() && + nextIndexPattern.timeFieldName !== currentIndexPattern.timeFieldName + ) { // switch time fields nextSort = nextSort.map((cur) => - cur[0] === currentIndexPattern.timeFieldName - ? [nextIndexPattern.timeFieldName, sortDirection] - : cur + cur[0] === currentIndexPattern.timeFieldName ? [nextIndexPattern.timeFieldName, cur[1]] : cur ); }