Skip to content

Commit

Permalink
Discover: Add handling for source column (elastic#91815)
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 committed Feb 19, 2021
1 parent dcf6336 commit d33b58e
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 15 deletions.
1 change: 1 addition & 0 deletions src/plugins/discover/public/application/angular/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ function ContextAppRouteController($routeParams, $scope, $route) {
storeInSessionStorage: getServices().uiSettings.get('state:storeInSessionStorage'),
history: getServices().history(),
toasts: getServices().core.notifications.toasts,
uiSettings: getServices().core.uiSettings,
});
this.state = { ...appState.getState() };
this.anchorId = $routeParams.id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
* Side Public License, v 1.
*/

import { IUiSettingsClient } from 'kibana/public';
import { getState } from './context_state';
import { createBrowserHistory, History } from 'history';
import { FilterManager, Filter } from '../../../../data/public';
import { coreMock } from '../../../../../core/public/mocks';
import { SEARCH_FIELDS_FROM_SOURCE } from '../../../common';
const setupMock = coreMock.createSetup();

describe('Test Discover Context State', () => {
Expand All @@ -23,6 +25,10 @@ describe('Test Discover Context State', () => {
defaultStepSize: '4',
timeFieldName: 'time',
history,
uiSettings: {
get: <T>(key: string) =>
((key === SEARCH_FIELDS_FROM_SOURCE ? true : ['_source']) as unknown) as T,
} as IUiSettingsClient,
});
state.startSync();
});
Expand Down
30 changes: 23 additions & 7 deletions src/plugins/discover/public/application/angular/context_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import _ from 'lodash';
import { History } from 'history';
import { NotificationsStart } from 'kibana/public';
import { NotificationsStart, IUiSettingsClient } from 'kibana/public';
import {
createStateContainer,
createKbnUrlStateStorage,
Expand All @@ -17,6 +17,7 @@ import {
withNotifyOnErrors,
} from '../../../../kibana_utils/public';
import { esFilters, FilterManager, Filter, Query } from '../../../../data/public';
import { handleSourceColumnState } from './helpers';

export interface AppState {
/**
Expand Down Expand Up @@ -73,6 +74,11 @@ interface GetStateParams {
* kbnUrlStateStorage will use it notifying about inner errors
*/
toasts?: NotificationsStart['toasts'];

/**
* core ui settings service
*/
uiSettings: IUiSettingsClient;
}

interface GetStateReturn {
Expand Down Expand Up @@ -123,6 +129,7 @@ export function getState({
storeInSessionStorage = false,
history,
toasts,
uiSettings,
}: GetStateParams): GetStateReturn {
const stateStorage = createKbnUrlStateStorage({
useHash: storeInSessionStorage,
Expand All @@ -134,7 +141,12 @@ export function getState({
const globalStateContainer = createStateContainer<GlobalState>(globalStateInitial);

const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState;
const appStateInitial = createInitialAppState(defaultStepSize, timeFieldName, appStateFromUrl);
const appStateInitial = createInitialAppState(
defaultStepSize,
timeFieldName,
appStateFromUrl,
uiSettings
);
const appStateContainer = createStateContainer<AppState>(appStateInitial);

const { start, stop } = syncStates([
Expand Down Expand Up @@ -257,7 +269,8 @@ function getFilters(state: AppState | GlobalState): Filter[] {
function createInitialAppState(
defaultSize: string,
timeFieldName: string,
urlState: AppState
urlState: AppState,
uiSettings: IUiSettingsClient
): AppState {
const defaultState = {
columns: ['_source'],
Expand All @@ -270,8 +283,11 @@ function createInitialAppState(
return defaultState;
}

return {
...defaultState,
...urlState,
};
return handleSourceColumnState(
{
...defaultState,
...urlState,
},
uiSettings
);
}
3 changes: 2 additions & 1 deletion src/plugins/discover/public/application/angular/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ app.config(($routeProvider) => {
const history = getHistory();
const savedSearchId = $route.current.params.id;
return data.indexPatterns.ensureDefaultIndexPattern(history).then(() => {
const { appStateContainer } = getState({ history });
const { appStateContainer } = getState({ history, uiSettings: config });
const { index } = appStateContainer.getState();
return Promise.props({
ip: loadIndexPattern(index, data.indexPatterns, config),
Expand Down Expand Up @@ -195,6 +195,7 @@ function discoverController($route, $scope, Promise) {
storeInSessionStorage: config.get('state:storeInSessionStorage'),
history,
toasts: core.notifications.toasts,
uiSettings: config,
});

const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { IUiSettingsClient } from 'kibana/public';
import {
getState,
GetStateReturn,
Expand All @@ -14,18 +15,25 @@ import {
import { createBrowserHistory, History } from 'history';
import { dataPluginMock } from '../../../../data/public/mocks';
import { SavedSearch } from '../../saved_searches';
import { SEARCH_FIELDS_FROM_SOURCE } from '../../../common';

let history: History;
let state: GetStateReturn;
const getCurrentUrl = () => history.createHref(history.location);

const uiSettingsMock = {
get: <T>(key: string) =>
((key === SEARCH_FIELDS_FROM_SOURCE ? true : ['_source']) as unknown) as T,
} as IUiSettingsClient;

describe('Test discover state', () => {
beforeEach(async () => {
history = createBrowserHistory();
history.push('/');
state = getState({
getStateDefaults: () => ({ index: 'test' }),
history,
uiSettings: uiSettingsMock,
});
await state.replaceUrlAppState({});
await state.startSync();
Expand Down Expand Up @@ -81,6 +89,7 @@ describe('Test discover state with legacy migration', () => {
state = getState({
getStateDefaults: () => ({ index: 'test' }),
history,
uiSettings: uiSettingsMock,
});
expect(state.appStateContainer.getState()).toMatchInlineSnapshot(`
Object {
Expand All @@ -106,6 +115,7 @@ describe('createSearchSessionRestorationDataProvider', () => {
data: mockDataPlugin,
appStateContainer: getState({
history: createBrowserHistory(),
uiSettings: uiSettingsMock,
}).appStateContainer,
getSavedSearch: () => mockSavedSearch,
});
Expand Down
21 changes: 16 additions & 5 deletions src/plugins/discover/public/application/angular/discover_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { isEqual } from 'lodash';
import { i18n } from '@kbn/i18n';
import { History } from 'history';
import { NotificationsStart } from 'kibana/public';
import { NotificationsStart, IUiSettingsClient } from 'kibana/public';
import {
createKbnUrlStateStorage,
createStateContainer,
Expand All @@ -30,6 +30,7 @@ import { migrateLegacyQuery } from '../helpers/migrate_legacy_query';
import { DiscoverGridSettings } from '../components/discover_grid/types';
import { DISCOVER_APP_URL_GENERATOR, DiscoverUrlGeneratorState } from '../../url_generator';
import { SavedSearch } from '../../saved_searches';
import { handleSourceColumnState } from './helpers';

export interface AppState {
/**
Expand Down Expand Up @@ -90,6 +91,11 @@ interface GetStateParams {
* kbnUrlStateStorage will use it notifying about inner errors
*/
toasts?: NotificationsStart['toasts'];

/**
* core ui settings service
*/
uiSettings: IUiSettingsClient;
}

export interface GetStateReturn {
Expand Down Expand Up @@ -149,6 +155,7 @@ export function getState({
storeInSessionStorage = false,
history,
toasts,
uiSettings,
}: GetStateParams): GetStateReturn {
const defaultAppState = getStateDefaults ? getStateDefaults() : {};
const stateStorage = createKbnUrlStateStorage({
Expand All @@ -163,10 +170,14 @@ export function getState({
appStateFromUrl.query = migrateLegacyQuery(appStateFromUrl.query);
}

let initialAppState = {
...defaultAppState,
...appStateFromUrl,
};
let initialAppState = handleSourceColumnState(
{
...defaultAppState,
...appStateFromUrl,
},
uiSettings
);
// todo filter source depending on fields fetchinbg flag (if no columns remain and source fetching is enabled, use default columns)
let previousAppState: AppState;
const appStateContainer = createStateContainer<AppState>(initialAppState);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@

export { buildPointSeriesData } from './point_series';
export { formatRow } from './row_formatter';
export { handleSourceColumnState } from './state_helpers';
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { IUiSettingsClient } from 'src/core/public';
import { SEARCH_FIELDS_FROM_SOURCE, DEFAULT_COLUMNS_SETTING } from '../../../../common';

/**
* Makes sure the current state is not referencing the source column when using the fields api
* @param state
* @param uiSettings
*/
export function handleSourceColumnState<TState extends { columns?: string[] }>(
state: TState,
uiSettings: IUiSettingsClient
): TState {
if (!state.columns) {
return state;
}
const useNewFieldsApi = !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE);
const defaultColumns = uiSettings.get(DEFAULT_COLUMNS_SETTING);
if (useNewFieldsApi) {
// if fields API is used, filter out the source column
return {
...state,
columns: state.columns.filter((column) => column !== '_source'),
};
} else if (state.columns.length === 0) {
// if _source fetching is used and there are no column, switch back to default columns
// this can happen if the fields API was previously used
return {
...state,
columns: [...defaultColumns],
};
}

return state;
}
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ export const DiscoverGrid = ({
<DiscoverGridFlyout
indexPattern={indexPattern}
hit={expandedDoc}
columns={columns}
// if default columns are used, dont make them part of the URL - the context state handling will take care to restore them
columns={defaultColumns ? [] : columns}
onFilter={onFilter}
onRemoveColumn={onRemoveColumn}
onAddColumn={onAddColumn}
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/discover/_shared_links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
'/app/discover?_t=1453775307251#' +
'/?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time' +
":(from:'2015-09-19T06:31:44.000Z',to:'2015-09" +
"-23T18:31:44.000Z'))&_a=(columns:!(_source),filters:!(),index:'logstash-" +
"-23T18:31:44.000Z'))&_a=(columns:!(),filters:!(),index:'logstash-" +
"*',interval:auto,query:(language:kuery,query:'')" +
",sort:!(!('@timestamp',desc)))";
const actualUrl = await PageObjects.share.getSharedUrl();
Expand Down

0 comments on commit d33b58e

Please sign in to comment.