[Discover] Change default sort handling (#85561)

Author: kertal

src/plugins/discover/public/application/angular/discover.js

@@ -65,11 +65,13 @@ import {
   MODIFY_COLUMNS_ON_SWITCH,
   SAMPLE_SIZE_SETTING,
   SEARCH_ON_PAGE_LOAD_SETTING,
+  SORT_DEFAULT_ORDER_SETTING,
 } from '../../../common';
 import { loadIndexPattern, resolveIndexPattern } from '../helpers/resolve_index_pattern';
 import { getTopNavLinks } from '../components/top_nav/get_top_nav_links';
 import { updateSearchSource } from '../helpers/update_search_source';
 import { calcFieldCounts } from '../helpers/calc_field_counts';
+import { getDefaultSort } from './doc_table/lib/get_default_sort';
 
 const services = getServices();
 
@@ -410,9 +412,13 @@ function discoverController($element, $route, $scope, $timeout, Promise, uiCapab
 
   function getStateDefaults() {
     const query = $scope.searchSource.getField('query') || data.query.queryString.getDefaultQuery();
+    const sort = getSortArray(savedSearch.sort, $scope.indexPattern);
+
     return {
       query,
-      sort: getSortArray(savedSearch.sort, $scope.indexPattern),
+      sort: !sort.length
+        ? getDefaultSort($scope.indexPattern, config.get(SORT_DEFAULT_ORDER_SETTING, 'desc'))
+        : sort,
       columns:
         savedSearch.columns.length > 0
           ? savedSearch.columns

src/plugins/discover/public/application/angular/doc_table/lib/get_default_sort.test.ts

@@ -0,0 +1,43 @@
+/*
+ * 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 { getDefaultSort } from './get_default_sort';
+// @ts-ignore
+import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
+import { IndexPattern } from '../../../../kibana_services';
+
+describe('getDefaultSort function', function () {
+  let indexPattern: IndexPattern;
+  beforeEach(() => {
+    indexPattern = FixturesStubbedLogstashIndexPatternProvider() as IndexPattern;
+  });
+  test('should be a function', function () {
+    expect(typeof getDefaultSort === 'function').toBeTruthy();
+  });
+
+  test('should return default sort for an index pattern with timeFieldName', function () {
+    expect(getDefaultSort(indexPattern, 'desc')).toEqual([['time', 'desc']]);
+    expect(getDefaultSort(indexPattern, 'asc')).toEqual([['time', 'asc']]);
+  });
+
+  test('should return default sort for an index pattern without timeFieldName', function () {
+    delete indexPattern.timeFieldName;
+    expect(getDefaultSort(indexPattern, 'desc')).toEqual([]);
+    expect(getDefaultSort(indexPattern, 'asc')).toEqual([]);
+  });
+});

src/plugins/discover/public/application/angular/doc_table/lib/get_default_sort.ts

@@ -17,7 +17,6 @@
  * under the License.
  */
 import { IndexPattern } from '../../../../kibana_services';
-// @ts-ignore
 import { isSortable } from './get_sort';
 import { SortOrder } from '../components/table_header/helpers';
 
@@ -26,12 +25,12 @@ import { SortOrder } from '../components/table_header/helpers';
  * the default sort is returned depending of the index pattern
  */
 export function getDefaultSort(
-  indexPattern: IndexPattern,
+  indexPattern: IndexPattern | undefined,
   defaultSortOrder: string = 'desc'
 ): SortOrder[] {
-  if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName, indexPattern)) {
+  if (indexPattern?.timeFieldName && isSortable(indexPattern.timeFieldName, indexPattern)) {
     return [[indexPattern.timeFieldName, defaultSortOrder]];
   } else {
-    return [['_score', defaultSortOrder]];
+    return [];
   }
 }

src/plugins/discover/public/application/angular/doc_table/lib/get_sort_for_search_source.test.ts

@@ -0,0 +1,51 @@
+/*
+ * 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 { getSortForSearchSource } from './get_sort_for_search_source';
+// @ts-ignore
+import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
+import { IndexPattern } from '../../../../kibana_services';
+import { SortOrder } from '../components/table_header/helpers';
+
+describe('getSortForSearchSource function', function () {
+  let indexPattern: IndexPattern;
+  beforeEach(() => {
+    indexPattern = FixturesStubbedLogstashIndexPatternProvider() as IndexPattern;
+  });
+  test('should be a function', function () {
+    expect(typeof getSortForSearchSource === 'function').toBeTruthy();
+  });
+
+  test('should return an object to use for searchSource when columns are given', function () {
+    const cols = [['bytes', 'desc']] as SortOrder[];
+    expect(getSortForSearchSource(cols, indexPattern)).toEqual([{ bytes: 'desc' }]);
+    expect(getSortForSearchSource(cols, indexPattern, 'asc')).toEqual([{ bytes: 'desc' }]);
+    delete indexPattern.timeFieldName;
+    expect(getSortForSearchSource(cols, indexPattern)).toEqual([{ bytes: 'desc' }]);
+    expect(getSortForSearchSource(cols, indexPattern, 'asc')).toEqual([{ bytes: 'desc' }]);
+  });
+
+  test('should return an object to use for searchSource when no columns are given', function () {
+    const cols = [] as SortOrder[];
+    expect(getSortForSearchSource(cols, indexPattern)).toEqual([{ _doc: 'desc' }]);
+    expect(getSortForSearchSource(cols, indexPattern, 'asc')).toEqual([{ _doc: 'asc' }]);
+    delete indexPattern.timeFieldName;
+    expect(getSortForSearchSource(cols, indexPattern)).toEqual([{ _score: 'desc' }]);
+    expect(getSortForSearchSource(cols, indexPattern, 'asc')).toEqual([{ _score: 'asc' }]);
+  });
+});

src/plugins/discover/public/application/angular/doc_table/lib/get_sort_for_search_source.ts

@@ -19,7 +19,6 @@
 import { EsQuerySortValue, 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
@@ -33,10 +32,13 @@ export function getSortForSearchSource(
   indexPattern?: IndexPattern,
   defaultDirection: string = 'desc'
 ): EsQuerySortValue[] {
-  if (!sort || !indexPattern) {
-    return [];
-  } else if (Array.isArray(sort) && sort.length === 0) {
-    sort = getDefaultSort(indexPattern, defaultDirection);
+  if (!sort || !indexPattern || (Array.isArray(sort) && sort.length === 0)) {
+    if (indexPattern?.timeFieldName) {
+      // sorting by index order
+      return [{ _doc: defaultDirection } as EsQuerySortValue];
+    } else {
+      return [{ _score: defaultDirection } as EsQuerySortValue];
+    }
   }
   const { timeFieldName } = indexPattern;
   return getSort(sort, indexPattern).map((sortPair: Record<string, string>) => {

src/plugins/discover/public/application/embeddable/search_embeddable.ts

@@ -48,6 +48,7 @@ import {
 import { SEARCH_EMBEDDABLE_TYPE } from './constants';
 import { SavedSearch } from '../..';
 import { SAMPLE_SIZE_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../common';
+import { getDefaultSort } from '../angular/doc_table/lib/get_default_sort';
 
 interface SearchScope extends ng.IScope {
   columns?: string[];
@@ -200,6 +201,13 @@ export class SearchEmbeddable
     const { searchSource } = this.savedSearch;
     const indexPattern = (searchScope.indexPattern = searchSource.getField('index'))!;
 
+    if (!this.savedSearch.sort || !this.savedSearch.sort.length) {
+      this.savedSearch.sort = getDefaultSort(
+        indexPattern,
+        getServices().uiSettings.get(SORT_DEFAULT_ORDER_SETTING, 'desc')
+      );
+    }
+
     const timeRangeSearchSource = searchSource.create();
     timeRangeSearchSource.setField('filter', () => {
       if (!this.searchScope || !this.input.timeRange) return;
@@ -341,7 +349,14 @@ export class SearchEmbeddable
     // If there is column or sort data on the panel, that means the original columns or sort settings have
     // been overridden in a dashboard.
     searchScope.columns = this.input.columns || this.savedSearch.columns;
-    searchScope.sort = this.input.sort || this.savedSearch.sort;
+    const savedSearchSort =
+      this.savedSearch.sort && this.savedSearch.sort.length
+        ? this.savedSearch.sort
+        : getDefaultSort(
+            this.searchScope?.indexPattern,
+            getServices().uiSettings.get(SORT_DEFAULT_ORDER_SETTING, 'desc')
+          );
+    searchScope.sort = this.input.sort || savedSearchSort;
     searchScope.sharedItemTitle = this.panelTitle;
 
     if (forceFetch || isFetchRequired) {

src/plugins/discover/public/application/helpers/get_sharing_data.test.ts

@@ -21,6 +21,7 @@ import { getSharingData } from './get_sharing_data';
 import { IUiSettingsClient } from 'kibana/public';
 import { createSearchSourceMock } from '../../../../data/common/search/search_source/mocks';
 import { indexPatternMock } from '../../__mocks__/index_pattern';
+import { SORT_DEFAULT_ORDER_SETTING } from '../../../common';
 
 describe('getSharingData', () => {
   test('returns valid data for sharing', async () => {
@@ -29,7 +30,10 @@ describe('getSharingData', () => {
       searchSourceMock,
       { columns: [] },
       ({
-        get: () => {
+        get: (key: string) => {
+          if (key === SORT_DEFAULT_ORDER_SETTING) {
+            return 'desc';
+          }
           return false;
         },
       } as unknown) as IUiSettingsClient,
@@ -57,7 +61,13 @@ describe('getSharingData', () => {
               },
             },
             "script_fields": Object {},
-            "sort": Array [],
+            "sort": Array [
+              Object {
+                "_score": Object {
+                  "order": "desc",
+                },
+              },
+            ],
             "stored_fields": undefined,
           },
           "index": "the-index-pattern-title",

test/functional/apps/discover/_shared_links.ts

@@ -90,7 +90,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
             ":(from:'2015-09-19T06:31:44.000Z',to:'2015-09" +
             "-23T18:31:44.000Z'))&_a=(columns:!(_source),filters:!(),index:'logstash-" +
             "*',interval:auto,query:(language:kuery,query:'')" +
-            ',sort:!())';
+            ",sort:!(!('@timestamp',desc)))";
           const actualUrl = await PageObjects.share.getSharedUrl();
           // strip the timestamp out of each URL
           expect(actualUrl.replace(/_t=\d{13}/, '_t=TIMESTAMP')).to.be(