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

Discover: Add handling for source column #91815

Merged
merged 2 commits into from
Feb 18, 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
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: else is redundant

// 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