[Discover] Change default sort handling for index patterns without timefield (#54427)

Default sort is no longer in state. There's now a separate function to provide default sort for ES and UI, in case the user didn't actively select a field to sort by

Author: kertal

src/legacy/core_plugins/kibana/public/discover/__tests__/doc_table/lib/get_sort.js

@@ -23,7 +23,6 @@ import ngMock from 'ng_mock';
 import { getSort } from '../../../np_ready/angular/doc_table/lib/get_sort';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
 
-const defaultSort = [{ time: 'desc' }];
 let indexPattern;
 
 describe('docTable', function() {
@@ -51,26 +50,26 @@ describe('docTable', function() {
       expect(getSort([{ bytes: 'desc' }], indexPattern)).to.eql([{ bytes: 'desc' }]);
     });
 
-    it('should sort by the default when passed an unsortable field', function() {
-      expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql(defaultSort);
-      expect(getSort(['lol_nope', 'asc'], indexPattern)).to.eql(defaultSort);
+    it('should return an empty array when passed an unsortable field', function() {
+      expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql([]);
+      expect(getSort(['lol_nope', 'asc'], indexPattern)).to.eql([]);
 
       delete indexPattern.timeFieldName;
-      expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql([{ _score: 'desc' }]);
+      expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql([]);
     });
 
-    it('should sort in reverse chrono order otherwise on time based patterns', function() {
-      expect(getSort([], indexPattern)).to.eql(defaultSort);
-      expect(getSort(['foo'], indexPattern)).to.eql(defaultSort);
-      expect(getSort({ foo: 'bar' }, indexPattern)).to.eql(defaultSort);
+    it('should return an empty array ', function() {
+      expect(getSort([], indexPattern)).to.eql([]);
+      expect(getSort(['foo'], indexPattern)).to.eql([]);
+      expect(getSort({ foo: 'bar' }, indexPattern)).to.eql([]);
     });
 
-    it('should sort by score on non-time patterns', function() {
+    it('should return an empty array on non-time patterns', function() {
       delete indexPattern.timeFieldName;
 
-      expect(getSort([], indexPattern)).to.eql([{ _score: 'desc' }]);
-      expect(getSort(['foo'], indexPattern)).to.eql([{ _score: 'desc' }]);
-      expect(getSort({ foo: 'bar' }, indexPattern)).to.eql([{ _score: 'desc' }]);
+      expect(getSort([], indexPattern)).to.eql([]);
+      expect(getSort(['foo'], indexPattern)).to.eql([]);
+      expect(getSort({ foo: 'bar' }, indexPattern)).to.eql([]);
     });
   });
 
@@ -87,19 +86,19 @@ describe('docTable', function() {
       expect(getSort.array([{ bytes: 'desc' }], indexPattern)).to.eql([['bytes', 'desc']]);
     });
 
-    it('should sort by the default when passed an unsortable field', function() {
-      expect(getSort.array([{ 'non-sortable': 'asc' }], indexPattern)).to.eql([['time', 'desc']]);
-      expect(getSort.array([{ lol_nope: 'asc' }], indexPattern)).to.eql([['time', 'desc']]);
+    it('should sort by an empty array when an unsortable field is given', function() {
+      expect(getSort.array([{ 'non-sortable': 'asc' }], indexPattern)).to.eql([]);
+      expect(getSort.array([{ lol_nope: 'asc' }], indexPattern)).to.eql([]);
 
       delete indexPattern.timeFieldName;
-      expect(getSort.array([{ 'non-sortable': 'asc' }], indexPattern)).to.eql([['_score', 'desc']]);
+      expect(getSort.array([{ 'non-sortable': 'asc' }], indexPattern)).to.eql([]);
     });
 
-    it('should sort by the default when passed an empty sort', () => {
-      expect(getSort.array([], indexPattern)).to.eql([['time', 'desc']]);
+    it('should return an empty array when passed an empty sort array', () => {
+      expect(getSort.array([], indexPattern)).to.eql([]);
 
       delete indexPattern.timeFieldName;
-      expect(getSort.array([], indexPattern)).to.eql([['_score', 'desc']]);
+      expect(getSort.array([], indexPattern)).to.eql([]);
     });
   });
 });

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js

@@ -486,7 +486,14 @@ function discoverController(
 
     const { searchFields, selectFields } = await getSharingDataFields();
     searchSource.setField('fields', searchFields);
-    searchSource.setField('sort', getSortForSearchSource($state.sort, $scope.indexPattern));
+    searchSource.setField(
+      'sort',
+      getSortForSearchSource(
+        $state.sort,
+        $scope.indexPattern,
+        config.get('discover:sort:defaultOrder')
+      )
+    );
     searchSource.setField('highlight', null);
     searchSource.setField('highlightAll', null);
     searchSource.setField('aggs', null);
@@ -517,11 +524,7 @@ function discoverController(
           language:
             localStorage.get('kibana.userQueryLanguage') || config.get('search:queryLanguage'),
         },
-      sort: getSort.array(
-        savedSearch.sort,
-        $scope.indexPattern,
-        config.get('discover:sort:defaultOrder')
-      ),
+      sort: getSort.array(savedSearch.sort, $scope.indexPattern),
       columns:
         savedSearch.columns.length > 0 ? savedSearch.columns : config.get('defaultColumns').slice(),
       index: $scope.indexPattern.id,
@@ -934,7 +937,10 @@ function discoverController(
     const { indexPattern, searchSource } = $scope;
     searchSource
       .setField('size', $scope.opts.sampleSize)
-      .setField('sort', getSortForSearchSource($state.sort, indexPattern))
+      .setField(
+        'sort',
+        getSortForSearchSource($state.sort, indexPattern, config.get('discover:sort:defaultOrder'))
+      )
       .setField('query', !$state.query ? null : $state.query)
       .setField('filter', filterManager.getFilters());
   });

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_header.ts

@@ -37,6 +37,7 @@ export function createTableHeaderDirective(reactDirective: any, config: IUiSetti
     {
       hideTimeColumn: config.get('doc_table:hideTimeColumn'),
       isShortDots: config.get('shortDots:enable'),
+      defaultSortOrder: config.get('discover:sort:defaultOrder'),
     }
   );
 }

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_header/helpers.tsx

@@ -20,7 +20,7 @@ import { IndexPattern } from '../../../../../kibana_services';
 // @ts-ignore
 import { shortenDottedString } from '../../../../../../../common/utils/shorten_dotted_string';
 
-export type SortOrder = [string, 'asc' | 'desc'];
+export type SortOrder = [string, string];
 export interface ColumnProps {
   name: string;
   displayName: string;

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_header/table_header.test.tsx

@@ -59,6 +59,7 @@ function getMockProps(props = {}) {
     indexPattern: getMockIndexPattern(),
     hideTimeColumn: false,
     columns: ['first', 'middle', 'last'],
+    defaultSortOrder: 'desc',
     sortOrder: [['time', 'asc']] as SortOrder[],
     isShortDots: true,
     onRemoveColumn: jest.fn(),

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_header/table_header.tsx

@@ -21,9 +21,11 @@ import { IndexPattern } from '../../../../../kibana_services';
 // @ts-ignore
 import { TableHeaderColumn } from './table_header_column';
 import { SortOrder, getDisplayedColumns } from './helpers';
+import { getDefaultSort } from '../../lib/get_default_sort';
 
 interface Props {
   columns: string[];
+  defaultSortOrder: string;
   hideTimeColumn: boolean;
   indexPattern: IndexPattern;
   isShortDots: boolean;
@@ -35,6 +37,7 @@ interface Props {
 
 export function TableHeader({
   columns,
+  defaultSortOrder,
   hideTimeColumn,
   indexPattern,
   isShortDots,
@@ -53,7 +56,9 @@ export function TableHeader({
           <TableHeaderColumn
             key={col.name}
             {...col}
-            sortOrder={sortOrder}
+            sortOrder={
+              sortOrder.length ? sortOrder : getDefaultSort(indexPattern, defaultSortOrder)
+            }
             onMoveColumn={onMoveColumn}
             onRemoveColumn={onRemoveColumn}
             onChangeSortOrder={onChangeSortOrder}

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/components/table_header/table_header_column.tsx

@@ -34,7 +34,7 @@ interface Props {
   sortOrder: SortOrder[];
 }
 
-const sortDirectionToIcon = {
+const sortDirectionToIcon: Record<string, string> = {
   desc: 'fa fa-sort-down',
   asc: 'fa fa-sort-up',
   '': 'fa fa-sort',

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/lib/get_default_sort.ts

@@ -0,0 +1,37 @@
+/*
+ * 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 { IndexPattern } from '../../../../kibana_services';
+// @ts-ignore
+import { isSortable } from './get_sort';
+import { SortOrder } from '../components/table_header/helpers';
+
+/**
+ * use in case the user didn't manually sort.
+ * the default sort is returned depending of the index pattern
+ */
+export function getDefaultSort(
+  indexPattern: IndexPattern,
+  defaultSortOrder: string = 'desc'
+): SortOrder[] {
+  if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName, indexPattern)) {
+    return [[indexPattern.timeFieldName, defaultSortOrder]];
+  } else {
+    return [['_score', defaultSortOrder]];
+  }
+}

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/lib/get_sort.js

@@ -19,7 +19,7 @@
 
 import _ from 'lodash';
 
-function isSortable(field, indexPattern) {
+export function isSortable(field, indexPattern) {
   return indexPattern.fields.getByName(field) && indexPattern.fields.getByName(field).sortable;
 }
 
@@ -41,23 +41,20 @@ function createSortObject(sortPair, indexPattern) {
  * @param {object} indexPattern used for determining default sort
  * @returns {object} a sort object suitable for returning to elasticsearch
  */
-export function getSort(sort, indexPattern, defaultSortOrder = 'desc') {
+export function getSort(sort, indexPattern) {
   let sortObjects;
   if (Array.isArray(sort)) {
     sortObjects = _.compact(sort.map(sortPair => createSortObject(sortPair, indexPattern)));
   }
 
   if (!_.isEmpty(sortObjects)) {
     return sortObjects;
-  } else if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName, indexPattern)) {
-    return [{ [indexPattern.timeFieldName]: defaultSortOrder }];
-  } else {
-    return [{ _score: 'desc' }];
   }
+  return [];
 }
 
-getSort.array = function(sort, indexPattern, defaultSortOrder) {
-  return getSort(sort, indexPattern, defaultSortOrder).map(sortPair =>
+getSort.array = function(sort, indexPattern) {
+  return getSort(sort, indexPattern).map(sortPair =>
     _(sortPair)
       .pairs()
       .pop()

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/doc_table/lib/get_sort_for_search_source.ts

@@ -19,16 +19,24 @@
 import { IndexPattern } from '../../../../kibana_services';
 import { SortOrder } from '../components/table_header/helpers';
 import { getSort } from './get_sort';
+import { getDefaultSort } from './get_default_sort';
 
 /**
- * prepares sort for search source, that's sending the request to ES
- * handles the special case when there's sorting by date_nanos typed fields
- * the addon of the numeric_type guarantees the right sort order
- * when there are indices with date and indices with date_nanos field
+ * Prepares sort for search source, that's sending the request to ES
+ * - Adds default sort if necessary
+ * - Handles the special case when there's sorting by date_nanos typed fields
+ *   the addon of the numeric_type guarantees the right sort order
+ *   when there are indices with date and indices with date_nanos field
  */
-export function getSortForSearchSource(sort?: SortOrder[], indexPattern?: IndexPattern) {
+export function getSortForSearchSource(
+  sort?: SortOrder[],
+  indexPattern?: IndexPattern,
+  defaultDirection: 'asc' | 'desc' = 'desc'
+) {
   if (!sort || !indexPattern) {
     return [];
+  } else if (Array.isArray(sort) && sort.length === 0) {
+    sort = getDefaultSort(indexPattern, defaultDirection);
   }
   const { timeFieldName } = indexPattern;
   return getSort(sort, indexPattern).map((sortPair: Record<string, string>) => {