Skip to content

Commit

Permalink
[Discover] Fix index pattern switch behavior (#92131) (#92455)
Browse files Browse the repository at this point in the history
  • Loading branch information
kertal committed Feb 23, 2021
1 parent ffa5cdf commit 23ca2f1
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 45 deletions.
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,
};
}

0 comments on commit 23ca2f1

Please sign in to comment.