Allow sorting on multiple columns in Discover

src/legacy/core_plugins/kibana/public/discover/controllers/discover.js

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

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

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

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

@@ -59,7 +59,7 @@ function getMockProps(props = {}) {
     indexPattern: getMockIndexPattern(),
     hideTimeColumn: false,
     columns: ['first', 'middle', 'last'],
-    sortOrder: ['time', 'asc'] as SortOrder,
+    sortOrder: [['time', 'asc']] as SortOrder[],
     isShortDots: true,
     onRemoveColumn: jest.fn(),
     onChangeSortOrder: jest.fn(),
@@ -89,7 +89,7 @@ describe('TableHeader with time column', () => {
 
   test('time column is sortable with button, cycling sort direction', () => {
     findTestSubject(wrapper, 'docTableHeaderFieldSort_time').simulate('click');
-    expect(props.onChangeSortOrder).toHaveBeenCalledWith('time', 'desc');
+    expect(props.onChangeSortOrder).toHaveBeenCalledWith([['time', 'desc']]);
   });
 
   test('time column is not removeable, no button displayed', () => {

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

@@ -28,10 +28,10 @@ interface Props {
   hideTimeColumn: boolean;
   indexPattern: IndexPattern;
   isShortDots: boolean;
-  onChangeSortOrder?: (name: string, direction: 'asc' | 'desc') => void;
+  onChangeSortOrder?: (sortOrder: SortOrder[]) => void;
   onMoveColumn?: (name: string, index: number) => void;
   onRemoveColumn?: (name: string) => void;
-  sortOrder: SortOrder;
+  sortOrder: SortOrder[];
 }
 
 export function TableHeader({
@@ -45,7 +45,6 @@ export function TableHeader({
   sortOrder,
 }: Props) {
   const displayedColumns = getDisplayedColumns(columns, indexPattern, hideTimeColumn, isShortDots);
-  const [currColumnName, currDirection = 'asc'] = sortOrder;
 
   return (
     <tr data-test-subj="docTableHeader" className="kbnDocTableHeader">
@@ -55,7 +54,7 @@ export function TableHeader({
           <TableHeaderColumn
             key={col.name}
             {...col}
-            sortDirection={col.name === currColumnName ? currDirection : ''}
+            sortOrder={sortOrder}
             onMoveColumn={onMoveColumn}
             onRemoveColumn={onRemoveColumn}
             onChangeSortOrder={onChangeSortOrder}

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

@@ -22,6 +22,7 @@ import { i18n } from '@kbn/i18n';
 import { EuiToolTip } from '@elastic/eui';
 // @ts-ignore
 import { shortenDottedString } from '../../../../../common/utils/shorten_dotted_string';
+import { SortOrder } from './helpers';
 
 interface Props {
   colLeftIdx: number; // idx of the column to the left, -1 if moving is not possible
@@ -30,12 +31,18 @@ interface Props {
   isRemoveable: boolean;
   isSortable: boolean;
   name: string;
-  onChangeSortOrder?: (name: string, direction: 'asc' | 'desc') => void;
+  onChangeSortOrder?: (sortOrder: SortOrder[]) => void;
   onMoveColumn?: (name: string, idx: number) => void;
   onRemoveColumn?: (name: string) => void;
-  sortDirection: 'asc' | 'desc' | ''; // asc|desc -> field is sorted in this direction, else ''
+  sortOrder: SortOrder[];
 }
 
+const sortDirectionToIcon = {
+  desc: 'fa fa-sort-down',
+  asc: 'fa fa-sort-up',
+  '': 'fa fa-sort',
+};
+
 export function TableHeaderColumn({
   colLeftIdx,
   colRightIdx,
@@ -46,38 +53,76 @@ export function TableHeaderColumn({
   onChangeSortOrder,
   onMoveColumn,
   onRemoveColumn,
-  sortDirection,
+  sortOrder,
 }: Props) {
-  const btnSortIcon = sortDirection === 'desc' ? 'fa fa-sort-down' : 'fa fa-sort-up';
+  const [, sortDirection = ''] = sortOrder.find(sortPair => name === sortPair[0]) || [];
+  const currentSortWithoutColumn = sortOrder.filter(pair => pair[0] !== name);
+  const currentColumnSort = sortOrder.find(pair => pair[0] === name);
+  const currentColumnSortDirection = (currentColumnSort && currentColumnSort[1]) || '';
+
+  const btnSortIcon = sortDirectionToIcon[sortDirection];
   const btnSortClassName =
     sortDirection !== '' ? btnSortIcon : `kbnDocTableHeader__sortChange ${btnSortIcon}`;
 
+  const handleChangeSortOrder = () => {
+    if (!onChangeSortOrder) return;
+
+    // Cycle goes Unsorted -> Asc -> Desc -> Unsorted
+    if (currentColumnSort === undefined) {
+      onChangeSortOrder([[name, 'asc'], ...currentSortWithoutColumn]);
+    } else if (currentColumnSortDirection === 'asc') {
+      onChangeSortOrder([[name, 'desc'], ...currentSortWithoutColumn]);
+    } else if (currentColumnSortDirection === 'desc' && currentSortWithoutColumn.length === 0) {
+      // 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.
+      onChangeSortOrder([[name, 'asc']]);
+    } else {
+      onChangeSortOrder(currentSortWithoutColumn);
+    }
+  };
+
+  const getAriaLabel = () => {
+    const sortAscendingMessage = i18n.translate(
+      'kbn.docTable.tableHeader.sortByColumnAscendingAriaLabel',
+      {
+        defaultMessage: 'Sort {columnName} ascending',
+        values: { columnName: name },
+      }
+    );
+    const sortDescendingMessage = i18n.translate(
+      'kbn.docTable.tableHeader.sortByColumnDescendingAriaLabel',
+      {
+        defaultMessage: 'Sort {columnName} descending',
+        values: { columnName: name },
+      }
+    );
+    const stopSortingMessage = i18n.translate(
+      'kbn.docTable.tableHeader.sortByColumnUnsortedAriaLabel',
+      {
+        defaultMessage: 'Stop sorting on {columnName}',
+        values: { columnName: name },
+      }
+    );
+
+    if (currentColumnSort === undefined) {
+      return sortAscendingMessage;
+    } else if (sortDirection === 'asc') {
+      return sortDescendingMessage;
+    } else if (sortDirection === 'desc' && currentSortWithoutColumn.length === 0) {
+      return sortAscendingMessage;
+    } else {
+      return stopSortingMessage;
+    }
+  };
+
   // action buttons displayed on the right side of the column name
   const buttons = [
     // Sort Button
     {
       active: isSortable && typeof onChangeSortOrder === 'function',
-      ariaLabel:
-        sortDirection === 'asc'
-          ? i18n.translate('kbn.docTable.tableHeader.sortByColumnDescendingAriaLabel', {
-              defaultMessage: 'Sort {columnName} descending',
-              values: { columnName: name },
-            })
-          : i18n.translate('kbn.docTable.tableHeader.sortByColumnAscendingAriaLabel', {
-              defaultMessage: 'Sort {columnName} ascending',
-              values: { columnName: name },
-            }),
+      ariaLabel: getAriaLabel(),
       className: btnSortClassName,
-      onClick: () => {
-        /**
-         * cycle sorting direction
-         * asc -> desc, desc -> asc, default: asc
-         */
-        if (typeof onChangeSortOrder === 'function') {
-          const newDirection = sortDirection === 'asc' ? 'desc' : 'asc';
-          onChangeSortOrder(name, newDirection);
-        }
-      },
+      onClick: handleChangeSortOrder,
       testSubject: `docTableHeaderFieldSort_${name}`,
       tooltip: i18n.translate('kbn.docTable.tableHeader.sortByColumnTooltip', {
         defaultMessage: 'Sort by {columnName}',

src/legacy/core_plugins/kibana/public/discover/doc_table/doc_table.html

@@ -46,7 +46,6 @@
           filters="filters"
           class="kbnDocTable__row"
           on-add-column="onAddColumn"
-          on-change-sort-order="onChangeSortOrder"
           on-remove-column="onRemoveColumn"
         ></tr>
       </tbody>
@@ -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>

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

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

src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts

@@ -45,11 +45,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;
@@ -201,8 +201,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 });
     };
 
@@ -263,6 +263,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;
 
     if (

test/functional/apps/discover/_shared_links.js

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