Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,8 @@ function discoverController(
$scope.$watchCollection('state.sort', function (sort) {
if (!sort) return;

// get the current sort from {key: val} to ["key", "val"];
const currentSort = _.pairs($scope.searchSource.getField('sort')).pop();
// get the current sort from searchSource as array of arrays
const currentSort = getSort.array($scope.searchSource.getField('sort'), $scope.indexPattern);

// if the searchSource doesn't know, tell it so
if (!angular.equals(sort, currentSort)) $scope.fetch();
Expand Down Expand Up @@ -833,8 +833,8 @@ function discoverController(
.setField('filter', queryFilter.getFilters());
});

$scope.setSortOrder = function setSortOrder(columnName, direction) {
$scope.state.sort = [columnName, direction];
$scope.setSortOrder = function setSortOrder(sortPair) {
$scope.state.sort = sortPair;
};

// TODO: On array fields, negating does not negate the combination, rather all terms
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import ngMock from 'ng_mock';
import { getSort } from '../../lib/get_sort';
import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';

const defaultSort = { time: 'desc' };
const defaultSort = [{ time: 'desc' }];
let indexPattern;

describe('docTable', function () {
Expand All @@ -38,19 +38,19 @@ describe('docTable', function () {
expect(getSort).to.be.a(Function);
});

it('should return an object if passed a 2 item array', function () {
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql({ bytes: 'desc' });
it('should return an array of objects if passed a 2 item array', function () {
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql([{ bytes: 'desc' }]);

delete indexPattern.timeFieldName;
expect(getSort(['bytes', 'desc'], indexPattern)).to.eql({ bytes: 'desc' });
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);

delete indexPattern.timeFieldName;
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql({ _score: 'desc' });
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql([{ _score: 'desc' }]);
});

it('should sort in reverse chrono order otherwise on time based patterns', function () {
Expand All @@ -62,9 +62,9 @@ describe('docTable', function () {
it('should sort by score 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([{ _score: 'desc' }]);
expect(getSort(['foo'], indexPattern)).to.eql([{ _score: 'desc' }]);
expect(getSort({ foo: 'bar' }, indexPattern)).to.eql([{ _score: 'desc' }]);
});
});

Expand All @@ -73,8 +73,8 @@ describe('docTable', function () {
expect(getSort.array).to.be.a(Function);
});

it('should return an array for sortable fields', function () {
expect(getSort.array(['bytes', 'desc'], indexPattern)).to.eql([ 'bytes', 'desc' ]);
it('should return an array of arrays for sortable fields', function () {
expect(getSort.array(['bytes', 'desc'], indexPattern)).to.eql([[ 'bytes', 'desc' ]]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -174,28 +174,29 @@ describe('Doc Table', function () {
$scope.sortOrder = [];
$scope.cycleSortOrder(SORTABLE_FIELDS[0]);
expect($scope.onChangeSortOrder.callCount).to.be(1);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([SORTABLE_FIELDS[0], 'asc']);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([[[SORTABLE_FIELDS[0], 'asc']]]);
});

it('should call onChangeSortOrder with ascending order for a sortable field already sorted by in descending order', function () {
$scope.sortOrder = [SORTABLE_FIELDS[0], 'desc'];
$scope.sortOrder = [[SORTABLE_FIELDS[0], 'desc']];
$scope.cycleSortOrder(SORTABLE_FIELDS[0]);
expect($scope.onChangeSortOrder.callCount).to.be(1);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([SORTABLE_FIELDS[0], 'asc']);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([[[SORTABLE_FIELDS[0], 'asc']]]);
});

it('should call onChangeSortOrder with ascending order for a sortable field when already sorted by an different field', function () {
$scope.sortOrder = [SORTABLE_FIELDS[1], 'asc'];
it('should call onChangeSortOrder with ascending order for a sortable field in addition to an existing sort on a ' +
'different field', function () {
$scope.sortOrder = [[SORTABLE_FIELDS[1], 'asc']];
$scope.cycleSortOrder(SORTABLE_FIELDS[0]);
expect($scope.onChangeSortOrder.callCount).to.be(1);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([SORTABLE_FIELDS[0], 'asc']);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([[[SORTABLE_FIELDS[0], 'asc'], [SORTABLE_FIELDS[1], 'asc']]]);
});

it('should call onChangeSortOrder with descending order for a sortable field already sorted by in ascending order', function () {
$scope.sortOrder = [SORTABLE_FIELDS[0], 'asc'];
$scope.sortOrder = [[SORTABLE_FIELDS[0], 'asc']];
$scope.cycleSortOrder(SORTABLE_FIELDS[0]);
expect($scope.onChangeSortOrder.callCount).to.be(1);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([SORTABLE_FIELDS[0], 'desc']);
expect($scope.onChangeSortOrder.firstCall.args).to.eql([[[SORTABLE_FIELDS[0], 'desc']]]);
});

it('should not call onChangeSortOrder for an unsortable field', function () {
Expand Down Expand Up @@ -223,17 +224,17 @@ describe('Doc Table', function () {
expect($scope.headerClass(UNSORTABLE_FIELDS[0])).to.be(undefined);
});

it('should return list including fa-sort-up for a sortable field not currently sorted by', function () {
expect($scope.headerClass(SORTABLE_FIELDS[0])).to.contain('fa-sort-up');
it('should return list including fa-sort for a sortable field not currently sorted by', function () {
expect($scope.headerClass(SORTABLE_FIELDS[0])).to.contain('fa-sort');
});

it('should return list including fa-sort-up for a sortable field currently sorted by in ascending order', function () {
$scope.sortOrder = [SORTABLE_FIELDS[0], 'asc'];
$scope.sortOrder = [[SORTABLE_FIELDS[0], 'asc']];
expect($scope.headerClass(SORTABLE_FIELDS[0])).to.contain('fa-sort-up');
});

it('should return list including fa-sort-down for a sortable field currently sorted by in descending order', function () {
$scope.sortOrder = [SORTABLE_FIELDS[0], 'desc'];
$scope.sortOrder = [[SORTABLE_FIELDS[0], 'desc']];
expect($scope.headerClass(SORTABLE_FIELDS[0])).to.contain('fa-sort-down');
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,12 @@ module.directive('kbnTableHeader', function () {
$scope.headerClass = function (column) {
if (!$scope.isSortableColumn(column)) return;

const sortOrder = $scope.sortOrder;
const defaultClass = ['fa', 'fa-sort-up', 'kbnDocTableHeader__sortChange'];
const defaultClass = ['fa', 'fa-sort', 'kbnDocTableHeader__sortChange'];
const sortOrder = $scope.sortOrder || [];
const columnSortOrder = sortOrder.find((sortPair) => column === sortPair[0]);

if (!sortOrder || column !== sortOrder[0]) return defaultClass;
return ['fa', sortOrder[1] === 'asc' ? 'fa-sort-up' : 'fa-sort-down'];
if (!columnSortOrder) return defaultClass;
return ['fa', columnSortOrder[1] === 'asc' ? 'fa-sort-up' : 'fa-sort-down'];
};

$scope.moveColumnLeft = function moveLeft(columnName) {
Expand Down Expand Up @@ -118,14 +119,23 @@ module.directive('kbnTableHeader', function () {
return;
}

const [currentColumnName, currentDirection = 'asc'] = $scope.sortOrder;
const newDirection = (
(columnName === currentColumnName && currentDirection === 'asc')
? 'desc'
: 'asc'
);
const sortPair = $scope.sortOrder.find((pair) => pair[0] === columnName);

$scope.onChangeSortOrder(columnName, newDirection);
// Cycle goes Unsorted -> Asc -> Desc -> Unsorted
if (sortPair === undefined) {
$scope.onChangeSortOrder([[columnName, 'asc'], ...$scope.sortOrder]);
}
else if (sortPair[1] === 'asc') {
$scope.onChangeSortOrder([[columnName, 'desc'], ...$scope.sortOrder.filter((pair) => pair[0] !== columnName)]);
}
else if (sortPair[1] === 'desc' && $scope.sortOrder.length === 1) {
// If we're at the end of the cycle and this is the only existing sort, we switch
// back to ascending sort instead of removing it.
$scope.onChangeSortOrder([[columnName, 'asc']]);
}
else {
$scope.onChangeSortOrder($scope.sortOrder.filter((pair) => pair[0] !== columnName));
}
};

$scope.getAriaLabelForColumn = function getAriaLabelForColumn(name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
filters="filters"
class="kbnDocTable__row"
on-add-column="onAddColumn"
on-change-sort-order="onChangeSortOrder"
on-remove-column="onRemoveColumn"
></tr>
</tbody>
Expand Down Expand Up @@ -98,7 +97,6 @@
ng-class="{'kbnDocTable__row--highlight': row['$$_isAnchor']}"
data-test-subj="docTableRow{{ row['$$_isAnchor'] ? ' docTableAnchorRow' : ''}}"
on-add-column="onAddColumn"
on-change-sort-order="onChangeSortOrder"
on-remove-column="onRemoveColumn"
></tr>
</tbody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,49 @@

import _ from 'lodash';

function isSortable(field, indexPattern) {
return (indexPattern.fields.byName[field] && indexPattern.fields.byName[field].sortable);
}

function createSortObject(sortPair, indexPattern) {

if (Array.isArray(sortPair) && sortPair.length === 2 && isSortable(sortPair[0], indexPattern)) {
const [ field, direction ] = sortPair;
return { [field]: direction };
}
else {
return undefined;
}
}

/**
* Take a sorting array and make it into an object
* @param {array} 2 item array [fieldToSort, directionToSort]
* @param {array} sort 2 item array [fieldToSort, directionToSort]
* @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') {
const sortObj = {};
let field;
let direction;

function isSortable(field) {
return (indexPattern.fields.byName[field] && indexPattern.fields.byName[field].sortable);
let sortObjects;
if (Array.isArray(sort)) {
if (sort.length > 0 && !Array.isArray(sort[0])) {
sort = [sort];
}
sortObjects = _.compact(sort.map((sortPair) => createSortObject(sortPair, indexPattern)));
}

if (Array.isArray(sort) && sort.length === 2 && isSortable(sort[0])) {
// At some point we need to refactor the sorting logic, this array sucks.
field = sort[0];
direction = sort[1];
} else if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName)) {
field = indexPattern.timeFieldName;
direction = defaultSortOrder;
if (!_.isEmpty(sortObjects)) {
return sortObjects;
}

if (field) {
sortObj[field] = direction;
} else {
sortObj._score = 'desc';
else if (indexPattern.timeFieldName && isSortable(indexPattern.timeFieldName, indexPattern)) {
return [{ [indexPattern.timeFieldName]: defaultSortOrder }];
}
else {
return [{ _score: 'desc' }];
}



return sortObj;
}

getSort.array = function (sort, indexPattern, defaultSortOrder) {
return _(getSort(sort, indexPattern, defaultSortOrder)).pairs().pop();
return getSort(sort, indexPattern, defaultSortOrder).map((sortPair) => _(sortPair).pairs().pop());
};

Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ import { ISearchEmbeddable, SearchInput, SearchOutput } from './types';
interface SearchScope extends ng.IScope {
columns?: string[];
description?: string;
sort?: string[];
sort?: string[] | string[][];
searchSource?: SearchSource;
sharedItemTitle?: string;
inspectorAdapters?: Adapters;
setSortOrder?: (column: string, columnDirection: string) => void;
setSortOrder?: (sortPair: [string, string]) => void;
removeColumn?: (column: string) => void;
addColumn?: (column: string) => void;
moveColumn?: (column: string, index: number) => void;
Expand Down Expand Up @@ -195,8 +195,8 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>

this.pushContainerStateParamsToScope(searchScope);

searchScope.setSortOrder = (columnName, direction) => {
searchScope.sort = [columnName, direction];
searchScope.setSortOrder = sortPair => {
searchScope.sort = sortPair;
this.updateInput({ sort: searchScope.sort });
};

Expand Down Expand Up @@ -257,6 +257,9 @@ export class SearchEmbeddable extends Embeddable<SearchInput, SearchOutput>
// been overridden in a dashboard.
searchScope.columns = this.input.columns || this.savedSearch.columns;
searchScope.sort = this.input.sort || this.savedSearch.sort;
if (searchScope.sort.length && !Array.isArray(searchScope.sort[0])) {
searchScope.sort = [searchScope.sort];
}
searchScope.sharedItemTitle = this.panelTitle;

this.filtersSearchSource.setField('filter', this.input.filters);
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/discover/_shared_links.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,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:!(!(\'@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