Migrate legacy sort arrays on saved searches (#43038)

With my multi sort PR I changed the sort property on saved searches to contain a nested array. Discover and Dashboard were backwards compatible with the old format but it turns out the nested array caused issues for CSV export. Instead of trying to support single and two dimension arrays everywhere, this PR simply adds a migration for saved searches in 7.4 and updates our sample data sets so that we can always expect sort objects to be two dimensional arrays. I also cleaned up the backwards compatibility code in Discover and Dashboard.

src/legacy/core_plugins/kibana/migrations/migrations.js

@@ -365,6 +365,24 @@ function replaceMovAvgToMovFn(doc, logger) {
   return doc;
 }
 
+function migrateSearchSortToNestedArray(doc) {
+  const sort = get(doc, 'attributes.sort');
+  if (!sort) return doc;
+
+  // Don't do anything if we already have a two dimensional array
+  if (Array.isArray(sort) && sort.length > 0 && Array.isArray(sort[0])) {
+    return doc;
+  }
+
+  return {
+    ...doc,
+    attributes: {
+      ...doc.attributes,
+      sort: [doc.attributes.sort],
+    }
+  };
+}
+
 const executeMigrations720 = flow(
   migratePercentileRankAggregation,
   migrateDateHistogramAggregation
@@ -376,6 +394,10 @@ const executeMigrations730 = flow(
   replaceMovAvgToMovFn
 );
 
+const executeSearchMigrations740 = flow(
+  migrateSearchSortToNestedArray,
+);
+
 export const migrations = {
   'index-pattern': {
     '6.5.0': doc => {
@@ -530,5 +552,6 @@ export const migrations = {
       migrateIndexPattern(doc);
       return doc;
     },
+    '7.4.0': executeSearchMigrations740,
   },
 };

src/legacy/core_plugins/kibana/migrations/migrations.test.js

@@ -1788,4 +1788,56 @@ Object {
       /* eslint-enable max-len */
     });
   });
+
+  describe('7.4.0', function () {
+    const migration = migrations.search['7.4.0'];
+
+    test('transforms one dimensional sort arrays into two dimensional arrays', () => {
+      const doc = {
+        id: '123',
+        type: 'search',
+        attributes: {
+          sort: ['bytes', 'desc'],
+        },
+      };
+
+      const expected = {
+        id: '123',
+        type: 'search',
+        attributes: {
+          sort: [['bytes', 'desc']],
+        },
+      };
+
+      const migratedDoc = migration(doc);
+
+      expect(migratedDoc).toEqual(expected);
+    });
+
+    test('doesn\'t modify search docs that already have two dimensional sort arrays', () => {
+      const doc = {
+        id: '123',
+        type: 'search',
+        attributes: {
+          sort: [['bytes', 'desc']],
+        },
+      };
+
+      const migratedDoc = migration(doc);
+
+      expect(migratedDoc).toEqual(doc);
+    });
+
+    test('doesn\'t modify search docs that have no sort array', () => {
+      const doc = {
+        id: '123',
+        type: 'search',
+        attributes: {},
+      };
+
+      const migratedDoc = migration(doc);
+
+      expect(migratedDoc).toEqual(doc);
+    });
+  });
 });

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

@@ -38,11 +38,15 @@ describe('docTable', function () {
       expect(getSort).to.be.a(Function);
     });
 
-    it('should return an array of objects if passed a 2 item array', function () {
-      expect(getSort(['bytes', 'desc'], indexPattern)).to.eql([{ bytes: 'desc' }]);
+    it('should return an array of objects', 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 passthrough arrays of objects', () => {
+      expect(getSort([{ bytes: 'desc' }], indexPattern)).to.eql([{ bytes: 'desc' }]);
     });
 
     it('should sort by the default when passed an unsortable field', function () {
@@ -74,7 +78,26 @@ describe('docTable', function () {
     });
 
     it('should return an array of arrays for sortable fields', function () {
-      expect(getSort.array(['bytes', 'desc'], indexPattern)).to.eql([[ 'bytes', 'desc' ]]);
+      expect(getSort.array([['bytes', 'desc']], indexPattern)).to.eql([[ 'bytes', 'desc' ]]);
+    });
+
+    it('should return an array of arrays from an array of elasticsearch sort objects', 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']]);
+
+      delete indexPattern.timeFieldName;
+      expect(getSort.array([{ 'non-sortable': 'asc' }], indexPattern)).to.eql([[ '_score', 'desc' ]]);
+    });
+
+    it('should sort by the default when passed an empty sort', () => {
+      expect(getSort.array([], indexPattern)).to.eql([['time', 'desc']]);
+
+      delete indexPattern.timeFieldName;
+      expect(getSort.array([], indexPattern)).to.eql([[ '_score', 'desc' ]]);
     });
   });
 });

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

@@ -29,24 +29,25 @@ function createSortObject(sortPair, indexPattern) {
     const [ field, direction ] = sortPair;
     return { [field]: direction };
   }
+  else if (_.isPlainObject(sortPair) && isSortable(Object.keys(sortPair)[0], indexPattern)) {
+    return sortPair;
+  }
   else {
     return undefined;
   }
 }
 
 /**
  * Take a sorting array and make it into an object
- * @param {array} sort 2 item array [fieldToSort, directionToSort]
+ * @param {array} sort two dimensional array [[fieldToSort, directionToSort]]
+ *  or an array of objects [{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') {
 
   let sortObjects;
   if (Array.isArray(sort)) {
-    if (sort.length > 0 && !Array.isArray(sort[0])) {
-      sort = [sort];
-    }
     sortObjects = _.compact(sort.map((sortPair) => createSortObject(sortPair, indexPattern)));
   }
 

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[] | string[][];
+  sort?: string[][];
   searchSource?: SearchSource;
   sharedItemTitle?: string;
   inspectorAdapters?: Adapters;
-  setSortOrder?: (sortPair: [string, string]) => void;
+  setSortOrder?: (sortPair: [[string, string]]) => void;
   removeColumn?: (column: string) => void;
   addColumn?: (column: string) => void;
   moveColumn?: (column: string, index: number) => void;
@@ -264,9 +264,6 @@ 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 (

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

@@ -34,7 +34,7 @@ export interface SearchInput extends EmbeddableInput {
   filters?: Filter[];
   hidePanelTitles?: boolean;
   columns?: string[];
-  sort?: string[];
+  sort?: string[][];
 }
 
 export interface SearchOutput extends EmbeddableOutput {

src/legacy/core_plugins/kibana/public/discover/types.d.ts

@@ -25,7 +25,7 @@ export interface SavedSearch {
   searchSource: SearchSource;
   description?: string;
   columns: string[];
-  sort: string[];
+  sort: string[][];
   destroy: () => void;
 }
 export interface SavedSearchLoader {

src/legacy/server/sample_data/data_sets/ecommerce/saved_objects.js

@@ -212,10 +212,10 @@ export const getSavedObjects = () => [
         "taxful_total_price",
         "total_quantity"
       ],
-      "sort": [
+      "sort": [[
         "order_date",
         "desc"
-      ],
+      ]],
       "version": 1,
       "kibanaSavedObjectMeta": {
         "searchSourceJSON": "{\"index\":\"ff959d40-b880-11e8-a6d9-e546fe2bba5f\",\"highlightAll\":true,\"version\":true,\"query\":{\"query\":\"\",\"language\":\"kuery\"},\"filter\":[]}"

src/legacy/server/sample_data/data_sets/flights/saved_objects.js

@@ -84,10 +84,10 @@ export const getSavedObjects = () => [
         "Cancelled",
         "FlightDelayType"
       ],
-      "sort": [
+      "sort": [[
         "timestamp",
         "desc"
-      ],
+      ]],
       "version": 1,
       "kibanaSavedObjectMeta": {
         "searchSourceJSON": "{\"index\":\"d3d7af60-4c81-11e8-b3d7-01146121b73d\",\"highlightAll\":true,\"version\":true,\"query\":{\"language\":\"kuery\",\"query\":\"\"},\"filter\":[]}"