From 82a6d39ba22d9f2abae2b4e3079fdd9869a12e71 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 6 Oct 2020 09:00:08 +0200 Subject: [PATCH] [Discover] Modify columns and sort when switching index pattern (#74336) * Modify columns and sort when switching index pattern * Add unit tests * Add uiSetting to allow switch to legacy behavior --- src/plugins/discover/common/index.ts | 1 + .../public/application/angular/discover.js | 22 ++++- ...get_switch_index_pattern_app_state.test.ts | 90 +++++++++++++++++++ .../get_switch_index_pattern_app_state.ts | 46 ++++++++++ src/plugins/discover/server/ui_settings.ts | 12 +++ 5 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.test.ts create mode 100644 src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.ts diff --git a/src/plugins/discover/common/index.ts b/src/plugins/discover/common/index.ts index 72030d91220b5..4334af63539e3 100644 --- a/src/plugins/discover/common/index.ts +++ b/src/plugins/discover/common/index.ts @@ -27,3 +27,4 @@ export const FIELDS_LIMIT_SETTING = 'fields:popularLimit'; export const CONTEXT_DEFAULT_SIZE_SETTING = 'context:defaultSize'; export const CONTEXT_STEP_SETTING = 'context:step'; export const CONTEXT_TIE_BREAKER_FIELDS_SETTING = 'context:tieBreakerFields'; +export const MODIFY_COLUMNS_ON_SWITCH = 'discover:modifyColumnsOnSwitch'; diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index 92b96d11723e0..078a047324113 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -71,7 +71,7 @@ const { import { getRootBreadcrumbs, getSavedSearchBreadcrumbs } from '../helpers/breadcrumbs'; import { validateTimeRange } from '../helpers/validate_time_range'; import { popularizeField } from '../helpers/popularize_field'; - +import { getSwitchIndexPatternAppState } from '../helpers/get_switch_index_pattern_app_state'; import { getIndexPatternId } from '../helpers/get_index_pattern_id'; import { addFatalError } from '../../../../kibana_legacy/public'; import { @@ -80,6 +80,7 @@ import { SORT_DEFAULT_ORDER_SETTING, SEARCH_ON_PAGE_LOAD_SETTING, DOC_HIDE_TIME_COLUMN_SETTING, + MODIFY_COLUMNS_ON_SWITCH, } from '../../../common'; const fetchStatuses = { @@ -253,6 +254,11 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise if (!_.isEqual(newStatePartial, oldStatePartial)) { $scope.$evalAsync(async () => { + if (oldStatePartial.index !== newStatePartial.index) { + //in case of index switch the route has currently to be reloaded, legacy + return; + } + $scope.state = { ...newState }; // detect changes that should trigger fetching of new data @@ -277,8 +283,18 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise }); $scope.setIndexPattern = async (id) => { - await replaceUrlAppState({ index: id }); - $route.reload(); + const nextIndexPattern = await indexPatterns.get(id); + if (nextIndexPattern) { + const nextAppState = getSwitchIndexPatternAppState( + $scope.indexPattern, + nextIndexPattern, + $scope.state.columns, + $scope.state.sort, + config.get(MODIFY_COLUMNS_ON_SWITCH) + ); + await replaceUrlAppState(nextAppState); + $route.reload(); + } }; // update data source when filters update 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 new file mode 100644 index 0000000000000..d35346ed24737 --- /dev/null +++ b/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.test.ts @@ -0,0 +1,90 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { getSwitchIndexPatternAppState } from './get_switch_index_pattern_app_state'; +import { IIndexPatternFieldList, 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); + }, + }, +} 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); + }, + }, +} as IndexPattern; + +describe('Discover getSwitchIndexPatternAppState', () => { + test('removing fields that are not part of the next index pattern, keeping unknown fields ', async () => { + const result = getSwitchIndexPatternAppState( + currentIndexPattern, + nextIndexPattern, + ['category', 'name', 'unknown'], + [['category', 'desc']] + ); + expect(result.columns).toEqual(['category', 'unknown']); + expect(result.sort).toEqual([['category', 'desc']]); + }); + test('removing sorted by fields that are not part of the next index pattern', async () => { + const result = getSwitchIndexPatternAppState( + currentIndexPattern, + nextIndexPattern, + ['name'], + [ + ['category', 'desc'], + ['name', 'asc'], + ] + ); + expect(result.columns).toEqual(['_source']); + expect(result.sort).toEqual([['category', 'desc']]); + }); + test('removing sorted by fields that without modifying columns', async () => { + const result = getSwitchIndexPatternAppState( + currentIndexPattern, + nextIndexPattern, + ['name'], + [ + ['category', 'desc'], + ['name', 'asc'], + ], + false + ); + expect(result.columns).toEqual(['name']); + expect(result.sort).toEqual([['category', '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 new file mode 100644 index 0000000000000..458b9b7e066fd --- /dev/null +++ b/src/plugins/discover/public/application/helpers/get_switch_index_pattern_app_state.ts @@ -0,0 +1,46 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { getSortArray } from '../angular/doc_table'; +import { SortPairArr } from '../angular/doc_table/lib/get_sort'; +import { IndexPattern } from '../../kibana_services'; + +/** + * Helper function to remove or adapt the currently selected columns/sort to be valid with the next + * index pattern, returns a new state object + */ +export function getSwitchIndexPatternAppState( + currentIndexPattern: IndexPattern, + nextIndexPattern: IndexPattern, + currentColumns: string[], + currentSort: SortPairArr[], + modifyColumns: boolean = true +) { + const nextColumns = modifyColumns + ? currentColumns.filter( + (column) => + nextIndexPattern.fields.getByName(column) || !currentIndexPattern.fields.getByName(column) + ) + : currentColumns; + const nextSort = getSortArray(currentSort, nextIndexPattern); + return { + index: nextIndexPattern.id, + columns: nextColumns.length ? nextColumns : ['_source'], + sort: nextSort, + }; +} diff --git a/src/plugins/discover/server/ui_settings.ts b/src/plugins/discover/server/ui_settings.ts index 3eca11cc584a9..5447b982eef14 100644 --- a/src/plugins/discover/server/ui_settings.ts +++ b/src/plugins/discover/server/ui_settings.ts @@ -32,6 +32,7 @@ import { CONTEXT_DEFAULT_SIZE_SETTING, CONTEXT_STEP_SETTING, CONTEXT_TIE_BREAKER_FIELDS_SETTING, + MODIFY_COLUMNS_ON_SWITCH, } from '../common'; export const uiSettings: Record = { @@ -163,4 +164,15 @@ export const uiSettings: Record = { category: ['discover'], schema: schema.arrayOf(schema.string()), }, + [MODIFY_COLUMNS_ON_SWITCH]: { + name: i18n.translate('discover.advancedSettings.discover.modifyColumnsOnSwitchTitle', { + defaultMessage: 'Modify columns when changing index patterns', + }), + value: true, + description: i18n.translate('discover.advancedSettings.discover.modifyColumnsOnSwitchText', { + defaultMessage: 'Remove columns that not available in the new index pattern.', + }), + category: ['discover'], + schema: schema.boolean(), + }, };