Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.12] [Discover] Fix index pattern switch behavior (#92131) #92456

Merged
merged 1 commit into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { IndexPatternRef } from './types';
import { ChangeIndexPattern } from './change_indexpattern';
import { getSwitchIndexPatternAppState } from '../../helpers/get_switch_index_pattern_app_state';
import { SortPairArr } from '../../angular/doc_table/lib/get_sort';
import { MODIFY_COLUMNS_ON_SWITCH } from '../../../../common';
import { MODIFY_COLUMNS_ON_SWITCH, SORT_DEFAULT_ORDER_SETTING } from '../../../../common';
import { AppState } from '../../angular/discover_state';
export interface DiscoverIndexPatternProps {
/**
Expand All @@ -46,10 +46,6 @@ export interface DiscoverIndexPatternProps {
* Discover App state
*/
state: AppState;
/**
* Read from the Fields API
*/
useNewFieldsApi?: boolean;
}

/**
Expand All @@ -62,7 +58,6 @@ export function DiscoverIndexPattern({
indexPatterns,
state,
setAppState,
useNewFieldsApi,
}: DiscoverIndexPatternProps) {
const options: IndexPatternRef[] = (indexPatternList || []).map((entity) => ({
id: entity.id,
Expand All @@ -80,12 +75,12 @@ export function DiscoverIndexPattern({
state.columns || [],
(state.sort || []) as SortPairArr[],
config.get(MODIFY_COLUMNS_ON_SWITCH),
useNewFieldsApi
config.get(SORT_DEFAULT_ORDER_SETTING)
);
setAppState(nextAppState);
}
},
[selectedIndexPattern, state, config, indexPatterns, setAppState, useNewFieldsApi]
[selectedIndexPattern, state, config, indexPatterns, setAppState]
);

const [selected, setSelected] = useState({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ export function DiscoverSidebar({
indexPatterns={indexPatterns}
state={state}
setAppState={setAppState}
useNewFieldsApi={useNewFieldsApi}
/>
</section>
);
Expand Down Expand Up @@ -195,7 +194,6 @@ export function DiscoverSidebar({
indexPatterns={indexPatterns}
state={state}
setAppState={setAppState}
useNewFieldsApi={useNewFieldsApi}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
indexPatterns={props.indexPatterns}
state={props.state}
setAppState={props.setAppState}
useNewFieldsApi={props.useNewFieldsApi}
/>
</section>
<EuiSpacer size="s" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,33 @@
*/

import { getSwitchIndexPatternAppState } from './get_switch_index_pattern_app_state';
import { IIndexPatternFieldList, IndexPattern } from '../../../../data/common/index_patterns';
import { IndexPattern } from '../../../../data/common/index_patterns';

const currentIndexPattern: IndexPattern = {
id: 'prev',
getFieldByName(name) {
return this.fields.getByName(name);
},
fields: {
getByName: (name: string) => {
const fields = [
{ name: 'category', sortable: true },
{ name: 'name', sortable: true },
] as IIndexPatternFieldList;
return fields.find((field) => field.name === name);
/**
* Helper function returning an index pattern
*/
const getIndexPattern = (id: string, timeFieldName: string, fields: string[]) => {
return {
id,
timeFieldName,
getFieldByName(name) {
return this.fields.getByName(name);
},
},
} as IndexPattern;

const nextIndexPattern = {
id: 'next',
getFieldByName(name) {
return this.fields.getByName(name);
},
fields: {
getByName: (name: string) => {
const fields = [{ name: 'category', sortable: true }] as IIndexPatternFieldList;
return fields.find((field) => field.name === name);
fields: {
getByName: (name: string) => {
return fields
.map((field) => ({
name: field,
sortable: true,
}))
.find((field) => field.name === name);
},
},
},
} as IndexPattern;
} as IndexPattern;
};

const currentIndexPattern = getIndexPattern('curr', '', ['category', 'name']);
const nextIndexPattern = getIndexPattern('next', '', ['category']);

describe('Discover getSwitchIndexPatternAppState', () => {
test('removing fields that are not part of the next index pattern, keeping unknown fields ', async () => {
Expand All @@ -59,10 +56,10 @@ describe('Discover getSwitchIndexPatternAppState', () => {
['name', 'asc'],
]
);
expect(result.columns).toEqual(['_source']);
expect(result.columns).toEqual([]);
expect(result.sort).toEqual([['category', 'desc']]);
});
test('removing sorted by fields that without modifying columns', async () => {
test('removing sorted by fields not available in the next index pattern without modifying columns', async () => {
const result = getSwitchIndexPatternAppState(
currentIndexPattern,
nextIndexPattern,
Expand All @@ -76,4 +73,29 @@ describe('Discover getSwitchIndexPatternAppState', () => {
expect(result.columns).toEqual(['name']);
expect(result.sort).toEqual([['category', 'desc']]);
});
test('keep sorting by timefield when switching between index patterns with different timeFields', async () => {
const current = getIndexPattern('a', 'timeFieldA', ['timeFieldA']);
const next = getIndexPattern('b', 'timeFieldB', ['timeFieldB']);

const result = getSwitchIndexPatternAppState(current, next, [], [['timeFieldA', 'desc']]);
expect(result.columns).toEqual([]);
expect(result.sort).toEqual([['timeFieldB', 'desc']]);
});
test('remove sorting by timefield when switching to an index pattern without timefield that contains the timefield column', async () => {
// Why: timefield column is prepended, keeping the sort, user would need to add the column to remove sorting in legacy grid
const current = getIndexPattern('a', 'timeFieldA', ['timeFieldA']);
const next = getIndexPattern('b', '', ['timeFieldA']);

const result = getSwitchIndexPatternAppState(current, next, [], [['timeFieldA', 'desc']]);
expect(result.columns).toEqual([]);
expect(result.sort).toEqual([]);
});
test('add sorting by timefield when switching from an index pattern without timefield to an indexpattern with timefield', async () => {
const current = getIndexPattern('b', '', ['timeFieldA']);
const next = getIndexPattern('a', 'timeFieldA', ['timeFieldA']);

const result = getSwitchIndexPatternAppState(current, next, [], []);
expect(result.columns).toEqual([]);
expect(result.sort).toEqual([['timeFieldA', 'desc']]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,28 @@ export function getSwitchIndexPatternAppState(
currentColumns: string[],
currentSort: SortPairArr[],
modifyColumns: boolean = true,
useNewFieldsApi: boolean = false
sortDirection: string = 'desc'
) {
const nextColumns = modifyColumns
? currentColumns.filter(
(column) =>
nextIndexPattern.fields.getByName(column) || !currentIndexPattern.fields.getByName(column)
)
: currentColumns;
const nextSort = getSortArray(currentSort, nextIndexPattern);
const defaultColumns = useNewFieldsApi ? [] : ['_source'];
const columns = nextColumns.length ? nextColumns : defaultColumns;
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) => {
return nextIndexPattern.timeFieldName || value[0] !== currentIndexPattern.timeFieldName;
});
return {
index: nextIndexPattern.id,
columns,
sort: nextSort,
sort:
nextIndexPattern.timeFieldName && !nextSort.length
? [[nextIndexPattern.timeFieldName, sortDirection]]
: nextSort,
};
}