Skip to content

Commit

Permalink
[Discover] Fix timefield sorting when switching similar index patterns (
Browse files Browse the repository at this point in the history
#116145)

* [Discover] fix timefield sorting

* [Discover] resolve review comments

* [Discover] replace primary time field for sorting

* [Discover] improve solution

* Improve typing and time based checking

* Fix missing isTimeBased call

* [Discover] fix linting

* [Discover] replace time fields when switching between time based data views

* [Discover] apply suggestions

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Tim Roes <[email protected]>
  • Loading branch information
3 people authored and dmlemeshko committed Nov 29, 2021
1 parent 80a686e commit 343d395
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 8 deletions.
12 changes: 10 additions & 2 deletions src/plugins/data_views/common/data_views/data_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ interface SavedObjectBody {
type?: string;
}

/**
* An interface representing a data view that is time based.
*/
export interface TimeBasedDataView extends DataView {
timeFieldName: NonNullable<DataView['timeFieldName']>;
getTimeField: () => DataViewField;
}

export class DataView implements IIndexPattern {
public id?: string;
public title: string = '';
Expand Down Expand Up @@ -286,11 +294,11 @@ export class DataView implements IIndexPattern {
return [...this.fields.getAll().filter((field) => field.scripted)];
}

isTimeBased(): boolean {
isTimeBased(): this is TimeBasedDataView {
return !!this.timeFieldName && (!this.fields || !!this.getTimeField());
}

isTimeNanosBased(): boolean {
isTimeNanosBased(): this is TimeBasedDataView {
const timeField = this.getTimeField();
return !!(timeField && timeField.esTypes && timeField.esTypes.indexOf('date_nanos') !== -1);
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data_views/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export type {
export { DataViewType, IndexPatternType } from './types';
export type { IndexPatternsContract, DataViewsContract } from './data_views';
export { IndexPatternsService, DataViewsService } from './data_views';
export type { IndexPatternListItem, DataViewListItem } from './data_views';
export type { IndexPatternListItem, DataViewListItem, TimeBasedDataView } from './data_views';
export { IndexPattern, DataView } from './data_views';
export { DuplicateDataViewError, DataViewSavedObjectConflictError } from './errors';
export type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const getIndexPattern = (id: string, timeFieldName: string, fields: string[]) =>
return {
id,
timeFieldName,
isTimeBased() {
return !!timeFieldName;
},
getFieldByName(name) {
return this.fields.getByName(name);
},
Expand Down Expand Up @@ -98,4 +101,12 @@ describe('Discover getSwitchIndexPatternAppState', () => {
expect(result.columns).toEqual([]);
expect(result.sort).toEqual([['timeFieldA', 'desc']]);
});
test('should change sorting for similar index patterns', async () => {
const current = getIndexPattern('timebased', 'timefield', ['timefield']);
const next = getIndexPattern('timebased2', 'timefield2', ['timefield', 'timefield2']);

const result = getSwitchIndexPatternAppState(current, next, [], [['timefield', 'desc']]);
expect(result.columns).toEqual([]);
expect(result.sort).toEqual([['timefield2', 'desc']]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,32 @@ export function getSwitchIndexPatternAppState(
)
: currentColumns;
const columns = nextColumns.length ? nextColumns : [];

// when switching from an index pattern with timeField to an index pattern without timeField
// filter out sorting by timeField in case it is set. index patterns without timeField don't
// prepend this field in the table, so in legacy grid you would need to add this column to
// remove sorting
const nextSort = getSortArray(currentSort, nextIndexPattern).filter((value) => {
let nextSort = getSortArray(currentSort, nextIndexPattern).filter((value) => {
return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName;
});

if (nextIndexPattern.isTimeBased() && !nextSort.length) {
// set default sorting if it was not set
nextSort = [[nextIndexPattern.timeFieldName, sortDirection]];
} else if (
nextIndexPattern.isTimeBased() &&
currentIndexPattern.isTimeBased() &&
nextIndexPattern.timeFieldName !== currentIndexPattern.timeFieldName
) {
// switch time fields
nextSort = nextSort.map((cur) =>
cur[0] === currentIndexPattern.timeFieldName ? [nextIndexPattern.timeFieldName, cur[1]] : cur
);
}

return {
index: nextIndexPattern.id,
columns,
sort:
nextIndexPattern.timeFieldName && !nextSort.length
? [[nextIndexPattern.timeFieldName, sortDirection]]
: nextSort,
sort: nextSort,
};
}

0 comments on commit 343d395

Please sign in to comment.