[Discover] Address issue with defaultColumns when changing data views (#168994)

- Closes https://github.com/elastic/kibana/issues/168581

## Summary

This PR re-adds default columns when changing data views. Only columns
which are present in the data view will be re-added.

For testing: configure some fields for `defaultColumns` on Advanced
Settings page and test switching data views on Discover page.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

packages/kbn-test/src/kbn_client/kbn_client_ui_settings.ts

@@ -10,7 +10,7 @@ import { ToolingLog } from '@kbn/tooling-log';
 
 import { KbnClientRequester, pathWithSpace } from './kbn_client_requester';
 
-export type UiSettingValues = Record<string, string | number | boolean>;
+export type UiSettingValues = Record<string, string | number | boolean | string[]>;
 interface UiSettingsApiResponse {
   settings: {
     [key: string]: {

src/plugins/discover/public/__mocks__/data_view_complex.ts

@@ -426,3 +426,18 @@ export const dataViewAdHoc = {
   }),
   isPersisted: () => false,
 } as DataView;
+
+export const dataViewWithDefaultColumnMock = buildDataViewMock({
+  name: 'data-view-with-user-default-column',
+  fields: [
+    ...fields,
+    {
+      name: 'default_column',
+      type: 'date',
+      scripted: false,
+      searchable: true,
+      aggregatable: true,
+    },
+  ] as DataView['fields'],
+  timeFieldName: '@timestamp',
+});

src/plugins/discover/public/application/main/hooks/utils/change_data_view.test.ts

@@ -6,11 +6,14 @@
  * Side Public License, v 1.
  */
 
+import {
+  dataViewComplexMock,
+  dataViewWithDefaultColumnMock,
+} from '../../../../__mocks__/data_view_complex';
 import { changeDataView } from './change_data_view';
 import { savedSearchMock } from '../../../../__mocks__/saved_search';
 import { discoverServiceMock } from '../../../../__mocks__/services';
 import type { DataView } from '@kbn/data-views-plugin/common';
-import { dataViewComplexMock } from '../../../../__mocks__/data_view_complex';
 import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
 
 const setupTestParams = (dataView: DataView | undefined) => {
@@ -27,11 +30,21 @@ const setupTestParams = (dataView: DataView | undefined) => {
 };
 
 describe('changeDataView', () => {
-  it('should set the right app state when a valid data view to switch to is given', async () => {
-    const params = setupTestParams(dataViewComplexMock as DataView);
-    await changeDataView('data-view-with-various-field-types', params);
+  it('should set the right app state when a valid data view (which includes the preconfigured default column) to switch to is given', async () => {
+    const params = setupTestParams(dataViewWithDefaultColumnMock);
+    await changeDataView(dataViewWithDefaultColumnMock.id!, params);
     expect(params.appState.update).toHaveBeenCalledWith({
-      columns: ['default_column'],
+      columns: ['default_column'], // default_column would be added as dataViewWithDefaultColumn has it as a mapped field
+      index: 'data-view-with-user-default-column-id',
+      sort: [['@timestamp', 'desc']],
+    });
+  });
+
+  it('should set the right app state when a valid data view to switch to is given', async () => {
+    const params = setupTestParams(dataViewComplexMock);
+    await changeDataView(dataViewComplexMock.id!, params);
+    expect(params.appState.update).toHaveBeenCalledWith({
+      columns: [], // default_column would not be added as dataViewComplexMock does not have it as a mapped field
       index: 'data-view-with-various-field-types-id',
       sort: [['data', 'desc']],
     });

src/plugins/discover/public/application/main/hooks/utils/change_data_view.ts

@@ -8,7 +8,11 @@
 
 import { SortOrder } from '@kbn/saved-search-plugin/public';
 import { DataView } from '@kbn/data-views-plugin/common';
-import { MODIFY_COLUMNS_ON_SWITCH, SORT_DEFAULT_ORDER_SETTING } from '@kbn/discover-utils';
+import {
+  MODIFY_COLUMNS_ON_SWITCH,
+  SORT_DEFAULT_ORDER_SETTING,
+  DEFAULT_COLUMNS_SETTING,
+} from '@kbn/discover-utils';
 import { DiscoverInternalStateContainer } from '../../services/discover_internal_state_container';
 import { DiscoverAppStateContainer } from '../../services/discover_app_state_container';
 import { addLog } from '../../../../utils/add_log';
@@ -46,6 +50,7 @@ export async function changeDataView(
     const nextAppState = getDataViewAppState(
       dataView,
       nextDataView,
+      uiSettings.get(DEFAULT_COLUMNS_SETTING, []),
       state.columns || [],
       (state.sort || []) as SortOrder[],
       uiSettings.get(MODIFY_COLUMNS_ON_SWITCH),

src/plugins/discover/public/application/main/services/discover_state.test.ts

@@ -704,7 +704,7 @@ describe('Test discover state actions', () => {
     await state.actions.onChangeDataView(dataViewComplexMock.id!);
     await new Promise(process.nextTick);
     expect(getCurrentUrl()).toMatchInlineSnapshot(
-      `"/#?_g=(refreshInterval:(pause:!t,value:1000),time:(from:now-15d,to:now))&_a=(columns:!(default_column),index:data-view-with-various-field-types-id,interval:auto,sort:!(!(data,desc)))"`
+      `"/#?_g=(refreshInterval:(pause:!t,value:1000),time:(from:now-15d,to:now))&_a=(columns:!(),index:data-view-with-various-field-types-id,interval:auto,sort:!(!(data,desc)))"`
     );
     await waitFor(() => {
       expect(state.dataState.fetch).toHaveBeenCalledTimes(1);

src/plugins/discover/public/application/main/utils/get_switch_data_view_app_state.test.ts

@@ -9,6 +9,8 @@
 import { getDataViewAppState } from './get_switch_data_view_app_state';
 import { DataView } from '@kbn/data-views-plugin/public';
 
+const emptyDefaultColumns: string[] = [];
+
 /**
  * Helper function returning an data view
  */
@@ -36,23 +38,36 @@ const getDataView = (id: string, timeFieldName: string, fields: string[]) => {
 };
 
 const currentDataView = getDataView('curr', '', ['category', 'name']);
-const nextDataView = getDataView('next', '', ['category']);
+const nextDataView = getDataView('next', '', ['category', 'user_default_column']);
 
 describe('Discover getDataViewAppState', () => {
   test('removing fields that are not part of the next data view, keeping unknown fields ', async () => {
     const result = getDataViewAppState(
       currentDataView,
       nextDataView,
+      emptyDefaultColumns,
       ['category', 'name', 'unknown'],
       [['category', 'desc']]
     );
     expect(result.columns).toEqual(['category', 'unknown']);
     expect(result.sort).toEqual([['category', 'desc']]);
   });
+  test('removing fields that are not part of the next data view and adding default columns', async () => {
+    const result = getDataViewAppState(
+      currentDataView,
+      nextDataView,
+      ['user_default_column', 'user_unknown_default_column'],
+      ['category', 'name', 'user_default_column'],
+      [['category', 'desc']]
+    );
+    expect(result.columns).toEqual(['category', 'user_default_column']);
+    expect(result.sort).toEqual([['category', 'desc']]);
+  });
   test('removing sorted by fields that are not part of the next data view', async () => {
     const result = getDataViewAppState(
       currentDataView,
       nextDataView,
+      emptyDefaultColumns,
       ['name'],
       [
         ['category', 'desc'],
@@ -66,6 +81,7 @@ describe('Discover getDataViewAppState', () => {
     const result = getDataViewAppState(
       currentDataView,
       nextDataView,
+      emptyDefaultColumns,
       ['name'],
       [
         ['category', 'desc'],
@@ -80,7 +96,13 @@ describe('Discover getDataViewAppState', () => {
     const current = getDataView('a', 'timeFieldA', ['timeFieldA']);
     const next = getDataView('b', 'timeFieldB', ['timeFieldB']);
 
-    const result = getDataViewAppState(current, next, [], [['timeFieldA', 'desc']]);
+    const result = getDataViewAppState(
+      current,
+      next,
+      emptyDefaultColumns,
+      [],
+      [['timeFieldA', 'desc']]
+    );
     expect(result.columns).toEqual([]);
     expect(result.sort).toEqual([['timeFieldB', 'desc']]);
   });
@@ -89,7 +111,13 @@ describe('Discover getDataViewAppState', () => {
     const current = getDataView('a', 'timeFieldA', ['timeFieldA']);
     const next = getDataView('b', '', ['timeFieldA']);
 
-    const result = getDataViewAppState(current, next, [], [['timeFieldA', 'desc']]);
+    const result = getDataViewAppState(
+      current,
+      next,
+      emptyDefaultColumns,
+      [],
+      [['timeFieldA', 'desc']]
+    );
     expect(result.columns).toEqual([]);
     expect(result.sort).toEqual([]);
   });
@@ -97,7 +125,7 @@ describe('Discover getDataViewAppState', () => {
     const current = getDataView('b', '', ['timeFieldA']);
     const next = getDataView('a', 'timeFieldA', ['timeFieldA']);
 
-    const result = getDataViewAppState(current, next, [], []);
+    const result = getDataViewAppState(current, next, emptyDefaultColumns, [], []);
     expect(result.columns).toEqual([]);
     expect(result.sort).toEqual([['timeFieldA', 'desc']]);
   });
@@ -105,8 +133,16 @@ describe('Discover getDataViewAppState', () => {
     const current = getDataView('timebased', 'timefield', ['timefield']);
     const next = getDataView('timebased2', 'timefield2', ['timefield', 'timefield2']);
 
-    const result = getDataViewAppState(current, next, [], [['timefield', 'desc']]);
+    const result = getDataViewAppState(
+      current,
+      next,
+      emptyDefaultColumns,
+      [],
+      [['timefield', 'desc']]
+    );
     expect(result.columns).toEqual([]);
     expect(result.sort).toEqual([['timefield2', 'desc']]);
   });
+
+  // TODO: add tests
 });

src/plugins/discover/public/application/main/utils/get_switch_data_view_app_state.ts

@@ -5,6 +5,7 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
+import { uniq } from 'lodash';
 import { isOfAggregateQueryType, Query, AggregateQuery } from '@kbn/es-query';
 import type { DataView } from '@kbn/data-views-plugin/public';
 import type { SortOrder } from '@kbn/saved-search-plugin/public';
@@ -17,20 +18,25 @@ import { getSortArray } from '../../../utils/sorting';
 export function getDataViewAppState(
   currentDataView: DataView,
   nextDataView: DataView,
+  defaultColumns: string[],
   currentColumns: string[],
   currentSort: SortOrder[],
   modifyColumns: boolean = true,
   sortDirection: string = 'desc',
   query?: Query | AggregateQuery
 ) {
-  const nextColumns = modifyColumns
-    ? currentColumns.filter(
-        (column) =>
-          nextDataView.fields.getByName(column) || !currentDataView.fields.getByName(column)
-      )
-    : currentColumns;
+  let columns = currentColumns || [];
+
+  if (modifyColumns) {
+    const currentUnknownColumns = columns.filter(
+      (column) => !currentDataView.fields.getByName(column) && !defaultColumns.includes(column)
+    );
+    const currentColumnsRefreshed = uniq([...columns, ...defaultColumns]);
+    columns = currentColumnsRefreshed.filter(
+      (column) => nextDataView.fields.getByName(column) || currentUnknownColumns.includes(column)
+    );
+  }
 
-  let columns = nextColumns.length ? nextColumns : [];
   if (query && isOfAggregateQueryType(query)) {
     columns = [];
   }

test/functional/apps/discover/group3/_default_columns.ts

@@ -0,0 +1,175 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import expect from '@kbn/expect';
+import { FtrProviderContext } from '../ftr_provider_context';
+
+export default function ({ getService, getPageObjects }: FtrProviderContext) {
+  const dataGrid = getService('dataGrid');
+  const PageObjects = getPageObjects([
+    'common',
+    'discover',
+    'timePicker',
+    'unifiedFieldList',
+    'unifiedSearch',
+    'header',
+  ]);
+  const esArchiver = getService('esArchiver');
+  const kibanaServer = getService('kibanaServer');
+  const security = getService('security');
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+    defaultColumns: ['message', 'extension', 'DestCountry'],
+    hideAnnouncements: true,
+  };
+
+  describe('discover default columns', function () {
+    before(async () => {
+      await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']);
+      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
+      await esArchiver.load('test/functional/fixtures/es_archiver/kibana_sample_data_logs_tsdb');
+      await esArchiver.loadIfNeeded(
+        'test/functional/fixtures/es_archiver/kibana_sample_data_flights'
+      );
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover');
+      await kibanaServer.importExport.load(
+        'test/functional/fixtures/kbn_archiver/kibana_sample_data_logs_tsdb'
+      );
+      await kibanaServer.importExport.load(
+        'test/functional/fixtures/kbn_archiver/kibana_sample_data_flights_index_pattern'
+      );
+    });
+
+    after(async () => {
+      await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
+      await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional');
+      await esArchiver.unload('test/functional/fixtures/es_archiver/kibana_sample_data_logs_tsdb');
+      await esArchiver.unload('test/functional/fixtures/es_archiver/kibana_sample_data_flights');
+      await kibanaServer.savedObjects.cleanStandardList();
+    });
+
+    beforeEach(async function () {
+      await PageObjects.timePicker.setDefaultAbsoluteRangeViaUiSettings();
+      await kibanaServer.uiSettings.update(defaultSettings);
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+    });
+
+    it('should render default columns', async function () {
+      expect(await dataGrid.getHeaderFields()).to.eql([
+        '@timestamp',
+        'message',
+        'extension',
+        'DestCountry',
+      ]);
+    });
+
+    it('should render only available default columns after switching data views', async function () {
+      expect(await dataGrid.getHeaderFields()).to.eql([
+        '@timestamp',
+        'message',
+        'extension',
+        'DestCountry',
+      ]);
+
+      await PageObjects.unifiedSearch.switchDataView(
+        'discover-dataView-switch-link',
+        'Kibana Sample Data Logs (TSDB)',
+        false
+      );
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await PageObjects.discover.expandTimeRangeAsSuggestedInNoResultsMessage();
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await dataGrid.getHeaderFields()).to.eql(['timestamp', 'message', 'extension']);
+
+      await PageObjects.unifiedSearch.switchDataView(
+        'discover-dataView-switch-link',
+        'kibana_sample_data_flights',
+        false
+      );
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await PageObjects.discover.expandTimeRangeAsSuggestedInNoResultsMessage();
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await dataGrid.getHeaderFields()).to.eql(['timestamp', 'DestCountry']);
+
+      await PageObjects.unifiedSearch.switchDataView(
+        'discover-dataView-switch-link',
+        'logstash-*',
+        false
+      );
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await PageObjects.discover.expandTimeRangeAsSuggestedInNoResultsMessage();
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await dataGrid.getHeaderFields()).to.eql(['@timestamp', 'extension']);
+    });
+
+    it('should combine selected columns and default columns after switching data views', async function () {
+      await PageObjects.unifiedFieldList.clickFieldListItemAdd('bytes');
+      await PageObjects.unifiedFieldList.clickFieldListItemRemove('DestCountry');
+      await PageObjects.unifiedFieldList.clickFieldListItemRemove('message');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+
+      expect(await dataGrid.getHeaderFields()).to.eql(['@timestamp', 'extension', 'bytes']);
+
+      await PageObjects.unifiedSearch.switchDataView(
+        'discover-dataView-switch-link',
+        'Kibana Sample Data Logs (TSDB)',
+        false
+      );
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await PageObjects.discover.expandTimeRangeAsSuggestedInNoResultsMessage();
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await dataGrid.getHeaderFields()).to.eql([
+        'timestamp',
+        'extension',
+        'bytes',
+        'message',
+      ]);
+
+      await PageObjects.unifiedSearch.switchDataView(
+        'discover-dataView-switch-link',
+        'logstash-*',
+        false
+      );
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await PageObjects.discover.expandTimeRangeAsSuggestedInNoResultsMessage();
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await dataGrid.getHeaderFields()).to.eql(['@timestamp', 'extension', 'bytes']);
+    });
+
+    it('should not change columns if discover:modifyColumnsOnSwitch is off', async function () {
+      await kibanaServer.uiSettings.update({
+        'discover:modifyColumnsOnSwitch': false,
+      });
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+
+      expect(await dataGrid.getHeaderFields()).to.eql([
+        '@timestamp',
+        'message',
+        'extension',
+        'DestCountry',
+      ]);
+
+      await PageObjects.unifiedSearch.switchDataView(
+        'discover-dataView-switch-link',
+        'kibana_sample_data_flights',
+        false
+      );
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      await PageObjects.discover.expandTimeRangeAsSuggestedInNoResultsMessage();
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await dataGrid.getHeaderFields()).to.eql([
+        'timestamp',
+        'message',
+        'extension',
+        'DestCountry',
+      ]);
+    });
+  });
+}

test/functional/apps/discover/group3/index.ts

@@ -20,6 +20,7 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
       await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional');
     });
 
+    loadTestFile(require.resolve('./_default_columns'));
     loadTestFile(require.resolve('./_drag_drop'));
     loadTestFile(require.resolve('./_sidebar'));
     loadTestFile(require.resolve('./_request_counts'));