Skip to content

Commit a7258a9

Browse files
lukasolsonmattkime
authored andcommitted
Fix sort for rollup data views (elastic#214656)
## Summary Resolves elastic#213629. Since elastic#163784 we have included a `format` parameter in the `sort` that we send to Elasticsearch. This worked for everything except rollup data views, which break when the `format` parameter is provided. This restores the behavior prior to that PR (we still send the `sort` but don't include the `format` parameter). Ideally we would probably not send the timestamp field at all for rollup data views since we treat them as if they are non-time-based, but this would require a bit of a refactor, and rollups are deprecated anyway. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### Release notes Fixes opening a rollup data view in Discover. Co-authored-by: Matthew Kime <[email protected]>
1 parent 042d2b2 commit a7258a9

File tree

3 files changed

+38
-3
lines changed

3 files changed

+38
-3
lines changed

src/platform/plugins/shared/data_views/common/data_view.stub.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10+
import { DataViewType } from './types';
1011
import { stubFieldSpecMap, stubLogstashFieldSpecMap } from './field.stub';
1112
import { createStubDataView } from './data_views/data_view.stub';
1213
export {
@@ -23,6 +24,16 @@ export const stubDataView = createStubDataView({
2324
},
2425
});
2526

27+
export const stubRollupDataView = createStubDataView({
28+
spec: {
29+
id: 'logstash-*',
30+
fields: stubFieldSpecMap,
31+
title: 'logstash-*',
32+
timeFieldName: '@timestamp',
33+
type: DataViewType.ROLLUP,
34+
},
35+
});
36+
2637
export const stubIndexPattern = stubDataView;
2738

2839
export const stubDataViewWithoutTimeField = createStubDataView({

src/platform/plugins/shared/discover/common/utils/sorting/get_sort_for_search_source.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99

1010
import type { SortOrder } from '@kbn/saved-search-plugin/public';
1111
import { getSortForSearchSource } from './get_sort_for_search_source';
12-
import { stubDataView, stubDataViewWithoutTimeField } from '@kbn/data-plugin/common/stubs';
12+
import {
13+
stubDataView,
14+
stubDataViewWithoutTimeField,
15+
stubRollupDataView,
16+
} from '@kbn/data-plugin/common/stubs';
1317

1418
describe('getSortForSearchSource function', function () {
1519
test('should be a function', function () {
@@ -94,4 +98,24 @@ describe('getSortForSearchSource function', function () {
9498
})
9599
).toEqual([{ _score: 'asc' }]);
96100
});
101+
102+
test('should return an object including format when data view is not a rollup', function () {
103+
expect(
104+
getSortForSearchSource({
105+
sort: [['@timestamp', 'desc']],
106+
dataView: stubDataView,
107+
defaultSortDir: 'desc',
108+
})
109+
).toEqual([{ '@timestamp': { format: 'strict_date_optional_time', order: 'desc' } }]);
110+
});
111+
112+
test('should not return an object excluding format when data view is a rollup', function () {
113+
expect(
114+
getSortForSearchSource({
115+
sort: [['@timestamp', 'desc']],
116+
dataView: stubRollupDataView,
117+
defaultSortDir: 'desc',
118+
})
119+
).toEqual([{ '@timestamp': 'desc' }]);
120+
});
97121
});

src/platform/plugins/shared/discover/common/utils/sorting/get_sort_for_search_source.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
import type { DataView } from '@kbn/data-views-plugin/common';
10+
import { type DataView, DataViewType } from '@kbn/data-views-plugin/common';
1111
import type { EsQuerySortValue, SortDirection } from '@kbn/data-plugin/common';
1212
import type { SortOrder } from '@kbn/saved-search-plugin/public';
1313
import { getSort } from './get_sort';
@@ -50,7 +50,7 @@ export function getSortForSearchSource({
5050
const sortPairs = getSort(sort, dataView, false); // ES|QL request is not using search source
5151

5252
const sortForSearchSource = sortPairs.map((sortPair: Record<string, string>) => {
53-
if (timeFieldName && sortPair[timeFieldName]) {
53+
if (dataView.type !== DataViewType.ROLLUP && timeFieldName && sortPair[timeFieldName]) {
5454
return getESQuerySortForTimeField({
5555
sortDir: sortPair[timeFieldName] as SortDirection,
5656
timeFieldName,

0 commit comments

Comments
 (0)