Skip to content

Commit

Permalink
[Discover] Change default sort handling (#85561)
Browse files Browse the repository at this point in the history
  • Loading branch information
kertal authored Dec 18, 2020
1 parent 122f1e1 commit 4bf2de7
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 14 deletions.
8 changes: 7 additions & 1 deletion src/plugins/discover/public/application/angular/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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 [];
}
}
Original file line number Diff line number Diff line change
@@ -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' }]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/discover/_shared_links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 4bf2de7

Please sign in to comment.