From 79201f5d8e1380fb092d643889e2eda0bdb08c20 Mon Sep 17 00:00:00 2001 From: Matt Bargar Date: Tue, 13 Aug 2019 19:39:23 -0400 Subject: [PATCH] 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. --- .../kibana/migrations/migrations.js | 23 ++++++++ .../kibana/migrations/migrations.test.js | 52 +++++++++++++++++++ .../doc_table/__tests__/lib/get_sort.js | 31 +++++++++-- .../public/discover/doc_table/lib/get_sort.js | 9 ++-- .../discover/embeddable/search_embeddable.ts | 7 +-- .../public/discover/embeddable/types.ts | 2 +- .../kibana/public/discover/types.d.ts | 2 +- .../data_sets/ecommerce/saved_objects.js | 4 +- .../data_sets/flights/saved_objects.js | 4 +- 9 files changed, 115 insertions(+), 19 deletions(-) diff --git a/src/legacy/core_plugins/kibana/migrations/migrations.js b/src/legacy/core_plugins/kibana/migrations/migrations.js index 40e3e8a5e88d0..bd4340dbadb0f 100644 --- a/src/legacy/core_plugins/kibana/migrations/migrations.js +++ b/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, }, }; diff --git a/src/legacy/core_plugins/kibana/migrations/migrations.test.js b/src/legacy/core_plugins/kibana/migrations/migrations.test.js index 37a9006f9eeb9..0776b6028641f 100644 --- a/src/legacy/core_plugins/kibana/migrations/migrations.test.js +++ b/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); + }); + }); }); diff --git a/src/legacy/core_plugins/kibana/public/discover/doc_table/__tests__/lib/get_sort.js b/src/legacy/core_plugins/kibana/public/discover/doc_table/__tests__/lib/get_sort.js index b8b962b9f92d7..18d4d1298d83d 100644 --- a/src/legacy/core_plugins/kibana/public/discover/doc_table/__tests__/lib/get_sort.js +++ b/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' ]]); }); }); }); diff --git a/src/legacy/core_plugins/kibana/public/discover/doc_table/lib/get_sort.js b/src/legacy/core_plugins/kibana/public/discover/doc_table/lib/get_sort.js index 9aba887569dbf..0af57c158e21a 100644 --- a/src/legacy/core_plugins/kibana/public/discover/doc_table/lib/get_sort.js +++ b/src/legacy/core_plugins/kibana/public/discover/doc_table/lib/get_sort.js @@ -29,6 +29,9 @@ 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; } @@ -36,7 +39,8 @@ function createSortObject(sortPair, indexPattern) { /** * 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 */ @@ -44,9 +48,6 @@ 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))); } diff --git a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts index 576abc5427a92..8288e3d1d1d7a 100644 --- a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts +++ b/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 // 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 ( diff --git a/src/legacy/core_plugins/kibana/public/discover/embeddable/types.ts b/src/legacy/core_plugins/kibana/public/discover/embeddable/types.ts index 5791216acde41..43648d575014e 100644 --- a/src/legacy/core_plugins/kibana/public/discover/embeddable/types.ts +++ b/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 { diff --git a/src/legacy/core_plugins/kibana/public/discover/types.d.ts b/src/legacy/core_plugins/kibana/public/discover/types.d.ts index 5922e2f8532fa..6de969888f166 100644 --- a/src/legacy/core_plugins/kibana/public/discover/types.d.ts +++ b/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 { diff --git a/src/legacy/server/sample_data/data_sets/ecommerce/saved_objects.js b/src/legacy/server/sample_data/data_sets/ecommerce/saved_objects.js index ac7a5e80c45a1..87a1f1c9ece47 100644 --- a/src/legacy/server/sample_data/data_sets/ecommerce/saved_objects.js +++ b/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\":[]}" diff --git a/src/legacy/server/sample_data/data_sets/flights/saved_objects.js b/src/legacy/server/sample_data/data_sets/flights/saved_objects.js index 726beeeb83227..5157e19df593e 100644 --- a/src/legacy/server/sample_data/data_sets/flights/saved_objects.js +++ b/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\":[]}"