[Discover] Fix index pattern switch behavior (#92131)

elastic · Feb 23, 2021 · 8be841a · 8be841a
1 parent 70645b3
commit 8be841a
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 45 deletions.
diff --git a/src/plugins/discover/public/application/components/sidebar/discover_index_pattern.tsx b/src/plugins/discover/public/application/components/sidebar/discover_index_pattern.tsx
@@ -19,7 +19,7 @@ import { IndexPatternRef } from './types';
 import { ChangeIndexPattern } from './change_indexpattern';
 import { getSwitchIndexPatternAppState } from '../../helpers/get_switch_index_pattern_app_state';
 import { SortPairArr } from '../../angular/doc_table/lib/get_sort';
-import { MODIFY_COLUMNS_ON_SWITCH } from '../../../../common';
+import { MODIFY_COLUMNS_ON_SWITCH, SORT_DEFAULT_ORDER_SETTING } from '../../../../common';
 import { AppState } from '../../angular/discover_state';
 export interface DiscoverIndexPatternProps {
   /**
@@ -46,10 +46,6 @@ export interface DiscoverIndexPatternProps {
    * Discover App state
    */
   state: AppState;
-  /**
-   * Read from the Fields API
-   */
-  useNewFieldsApi?: boolean;
 }
 
 /**
@@ -62,7 +58,6 @@ export function DiscoverIndexPattern({
   indexPatterns,
   state,
   setAppState,
-  useNewFieldsApi,
 }: DiscoverIndexPatternProps) {
   const options: IndexPatternRef[] = (indexPatternList || []).map((entity) => ({
     id: entity.id,
@@ -80,12 +75,12 @@ export function DiscoverIndexPattern({
           state.columns || [],
           (state.sort || []) as SortPairArr[],
           config.get(MODIFY_COLUMNS_ON_SWITCH),
-          useNewFieldsApi
+          config.get(SORT_DEFAULT_ORDER_SETTING)
         );
         setAppState(nextAppState);
       }
     },
-    [selectedIndexPattern, state, config, indexPatterns, setAppState, useNewFieldsApi]
+    [selectedIndexPattern, state, config, indexPatterns, setAppState]
   );
 
   const [selected, setSelected] = useState({

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx
@@ -165,7 +165,6 @@ export function DiscoverSidebar({
           indexPatterns={indexPatterns}
           state={state}
           setAppState={setAppState}
-          useNewFieldsApi={useNewFieldsApi}
         />
       </section>
     );
@@ -195,7 +194,6 @@ export function DiscoverSidebar({
             indexPatterns={indexPatterns}
             state={state}
             setAppState={setAppState}
-            useNewFieldsApi={useNewFieldsApi}
           />
         </EuiFlexItem>
         <EuiFlexItem grow={false}>

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx
@@ -160,7 +160,6 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
               indexPatterns={props.indexPatterns}
               state={props.state}
               setAppState={props.setAppState}
-              useNewFieldsApi={props.useNewFieldsApi}
             />
           </section>
           <EuiSpacer size="s" />

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
@@ -7,36 +7,33 @@
  */
 
 import { getSwitchIndexPatternAppState } from './get_switch_index_pattern_app_state';
-import { IIndexPatternFieldList, IndexPattern } from '../../../../data/common/index_patterns';
+import { 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);
+/**
+ * Helper function returning an index pattern
+ */
+const getIndexPattern = (id: string, timeFieldName: string, fields: string[]) => {
+  return {
+    id,
+    timeFieldName,
+    getFieldByName(name) {
+      return this.fields.getByName(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);
+    fields: {
+      getByName: (name: string) => {
+        return fields
+          .map((field) => ({
+            name: field,
+            sortable: true,
+          }))
+          .find((field) => field.name === name);
+      },
     },
-  },
-} as IndexPattern;
+  } as IndexPattern;
+};
+
+const currentIndexPattern = getIndexPattern('curr', '', ['category', 'name']);
+const nextIndexPattern = getIndexPattern('next', '', ['category']);
 
 describe('Discover getSwitchIndexPatternAppState', () => {
   test('removing fields that are not part of the next index pattern, keeping unknown fields ', async () => {
@@ -59,10 +56,10 @@ describe('Discover getSwitchIndexPatternAppState', () => {
         ['name', 'asc'],
       ]
     );
-    expect(result.columns).toEqual(['_source']);
+    expect(result.columns).toEqual([]);
     expect(result.sort).toEqual([['category', 'desc']]);
   });
-  test('removing sorted by fields that without modifying columns', async () => {
+  test('removing sorted by fields not available in the next index pattern without modifying columns', async () => {
     const result = getSwitchIndexPatternAppState(
       currentIndexPattern,
       nextIndexPattern,
@@ -76,4 +73,29 @@ describe('Discover getSwitchIndexPatternAppState', () => {
     expect(result.columns).toEqual(['name']);
     expect(result.sort).toEqual([['category', 'desc']]);
   });
+  test('keep sorting by timefield when switching between index patterns with different timeFields', async () => {
+    const current = getIndexPattern('a', 'timeFieldA', ['timeFieldA']);
+    const next = getIndexPattern('b', 'timeFieldB', ['timeFieldB']);
+
+    const result = getSwitchIndexPatternAppState(current, next, [], [['timeFieldA', 'desc']]);
+    expect(result.columns).toEqual([]);
+    expect(result.sort).toEqual([['timeFieldB', 'desc']]);
+  });
+  test('remove sorting by timefield when switching to an index pattern without timefield that contains the timefield column', async () => {
+    // Why: timefield column is prepended, keeping the sort, user would need to add the column to remove sorting in legacy grid
+    const current = getIndexPattern('a', 'timeFieldA', ['timeFieldA']);
+    const next = getIndexPattern('b', '', ['timeFieldA']);
+
+    const result = getSwitchIndexPatternAppState(current, next, [], [['timeFieldA', 'desc']]);
+    expect(result.columns).toEqual([]);
+    expect(result.sort).toEqual([]);
+  });
+  test('add sorting by timefield when switching from an index pattern without timefield to an indexpattern with timefield', async () => {
+    const current = getIndexPattern('b', '', ['timeFieldA']);
+    const next = getIndexPattern('a', 'timeFieldA', ['timeFieldA']);
+
+    const result = getSwitchIndexPatternAppState(current, next, [], []);
+    expect(result.columns).toEqual([]);
+    expect(result.sort).toEqual([['timeFieldA', '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
@@ -20,20 +20,28 @@ export function getSwitchIndexPatternAppState(
   currentColumns: string[],
   currentSort: SortPairArr[],
   modifyColumns: boolean = true,
-  useNewFieldsApi: boolean = false
+  sortDirection: string = 'desc'
 ) {
   const nextColumns = modifyColumns
     ? currentColumns.filter(
         (column) =>
           nextIndexPattern.fields.getByName(column) || !currentIndexPattern.fields.getByName(column)
       )
     : currentColumns;
-  const nextSort = getSortArray(currentSort, nextIndexPattern);
-  const defaultColumns = useNewFieldsApi ? [] : ['_source'];
-  const columns = nextColumns.length ? nextColumns : defaultColumns;
+  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;
+  });
   return {
     index: nextIndexPattern.id,
     columns,
-    sort: nextSort,
+    sort:
+      nextIndexPattern.timeFieldName && !nextSort.length
+        ? [[nextIndexPattern.timeFieldName, sortDirection]]
+        : nextSort,
   };
 }