[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
This commit is contained in:
Matthias Wilhelm 2020-01-24 06:23:53 +01:00 committed by GitHub
parent 1e91775a7f
commit ac0953e08b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 103 additions and 45 deletions

View file

@ -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([]);
});
});
});

View file

@ -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());
});

View file

@ -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'),
}
);
}

View file

@ -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;

View file

@ -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(),

View file

@ -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}

View file

@ -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',

View file

@ -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]];
}
}

View file

@ -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,7 +41,7 @@ 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)));
@ -49,15 +49,12 @@ export function getSort(sort, indexPattern, defaultSortOrder = 'desc') {
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()

View file

@ -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>) => {

View file

@ -266,7 +266,11 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>
searchSource.setField('size', getServices().uiSettings.get('discover:sampleSize'));
searchSource.setField(
'sort',
getSortForSearchSource(this.searchScope.sort, this.searchScope.indexPattern)
getSortForSearchSource(
this.searchScope.sort,
this.searchScope.indexPattern,
getServices().uiSettings.get('discover:sort:defaultOrder')
)
);
// Log request to inspector

View file

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