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] Navigating from Field statistics tab to text based languages should return to the documents view #152572

Merged
merged 6 commits into from
Apr 27, 2023
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 @@ -14,21 +14,24 @@ import { useTextBasedQueryLanguage } from './use_text_based_query_language';
import { BehaviorSubject } from 'rxjs';
import { FetchStatus } from '../../types';
import { DataDocuments$, RecordRawType } from '../services/discover_data_state_container';
import { DiscoverAppState } from '../services/discover_app_state_container';
import { DataTableRecord } from '../../../types';
import { AggregateQuery, Query } from '@kbn/es-query';
import { dataViewMock } from '../../../__mocks__/data_view';
import { DataViewListItem } from '@kbn/data-views-plugin/common';
import { savedSearchMock } from '../../../__mocks__/saved_search';
import { getDiscoverStateMock } from '../../../__mocks__/discover_state.mock';
import { VIEW_MODE } from '@kbn/saved-search-plugin/common';

function getHookProps(
query: AggregateQuery | Query | undefined,
dataViewsService?: DataViewsContract
dataViewsService?: DataViewsContract,
appState?: Partial<DiscoverAppState>
) {
const replaceUrlState = jest.fn();
const stateContainer = getDiscoverStateMock({ isTimeBased: true });
stateContainer.appState.replaceUrlState = replaceUrlState;
stateContainer.appState.update({ columns: [] });
stateContainer.appState.update({ columns: [], ...appState });
stateContainer.internalState.transitions.setSavedDataViews([dataViewMock as DataViewListItem]);

const msgLoading = {
Expand Down Expand Up @@ -83,6 +86,20 @@ describe('useTextBasedQueryLanguage', () => {
});
});
});
test('should change viewMode to DOCUMENT_LEVEL if it was AGGREGATED_LEVEL', async () => {
const props = getHookProps(query, undefined, {
viewMode: VIEW_MODE.AGGREGATED_LEVEL,
});
const { replaceUrlState } = props;

renderHook(() => useTextBasedQueryLanguage(props));

await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));
expect(replaceUrlState).toHaveBeenCalledWith({
index: 'the-data-view-id',
viewMode: VIEW_MODE.DOCUMENT_LEVEL,
});
});
test('changing a text based query with different result columns should change state when loading and finished', async () => {
const props = getHookProps(query);
const { documents$, replaceUrlState } = props;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import {
} from '@kbn/es-query';
import { useCallback, useEffect, useRef } from 'react';
import type { DataViewsContract } from '@kbn/data-views-plugin/public';
import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { SavedSearch, VIEW_MODE } from '@kbn/saved-search-plugin/public';
import type { DiscoverStateContainer } from '../services/discover_state';
import type { DataDocuments$ } from '../services/discover_data_state_container';
import { getValidViewMode } from '../utils/get_valid_view_mode';
import { FetchStatus } from '../../types';

const MAX_NUM_OF_COLUMNS = 50;
Expand Down Expand Up @@ -57,7 +58,7 @@ export function useTextBasedQueryLanguage({
if (!query || next.fetchStatus === FetchStatus.ERROR) {
return;
}
const { columns: stateColumns, index } = stateContainer.appState.getState();
const { columns: stateColumns, index, viewMode } = stateContainer.appState.getState();
let nextColumns: string[] = [];
const isTextBasedQueryLang =
recordRawType === 'plain' && isOfAggregateQueryType(query) && 'sql' in query;
Expand Down Expand Up @@ -115,6 +116,9 @@ export function useTextBasedQueryLanguage({
const nextState = {
...(addDataViewToState && { index: dataViewObj.id }),
...(addColumnsToState && { columns: nextColumns }),
...(viewMode === VIEW_MODE.AGGREGATED_LEVEL && {
viewMode: getValidViewMode({ viewMode, isTextBasedQueryMode: true }),
}),
};
stateContainer.appState.replaceUrlState(nextState);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,3 @@ export function getRawRecordType(query?: Query | AggregateQuery) {

return RecordRawType.DOCUMENT;
}

export function isPlainRecord(query?: Query | AggregateQuery): query is AggregateQuery {
return getRawRecordType(query) === RecordRawType.PLAIN;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

import { getStateDefaults } from './get_state_defaults';
import { createSearchSourceMock } from '@kbn/data-plugin/public/mocks';
import { VIEW_MODE } from '@kbn/saved-search-plugin/common';
import { dataViewWithTimefieldMock } from '../../../__mocks__/data_view_with_timefield';
import { savedSearchMock } from '../../../__mocks__/saved_search';
import { savedSearchMock, savedSearchMockWithSQL } from '../../../__mocks__/saved_search';
import { dataViewMock } from '../../../__mocks__/data_view';
import { discoverServiceMock } from '../../../__mocks__/services';

Expand Down Expand Up @@ -75,4 +76,42 @@ describe('getStateDefaults', () => {
}
`);
});

test('should set view mode correctly', () => {
const actualForUndefinedViewMode = getStateDefaults({
services: discoverServiceMock,
savedSearch: {
...savedSearchMockWithSQL,
viewMode: undefined,
},
});
expect(actualForUndefinedViewMode.viewMode).toBeUndefined();

const actualForTextBasedWithInvalidViewMode = getStateDefaults({
services: discoverServiceMock,
savedSearch: {
...savedSearchMockWithSQL,
viewMode: VIEW_MODE.AGGREGATED_LEVEL,
},
});
expect(actualForTextBasedWithInvalidViewMode.viewMode).toBe(VIEW_MODE.DOCUMENT_LEVEL);

const actualForTextBasedWithValidViewMode = getStateDefaults({
services: discoverServiceMock,
savedSearch: {
...savedSearchMockWithSQL,
viewMode: VIEW_MODE.DOCUMENT_LEVEL,
},
});
expect(actualForTextBasedWithValidViewMode.viewMode).toBe(VIEW_MODE.DOCUMENT_LEVEL);

const actualForWithValidViewMode = getStateDefaults({
services: discoverServiceMock,
savedSearch: {
...savedSearchMock,
viewMode: VIEW_MODE.AGGREGATED_LEVEL,
},
});
expect(actualForWithValidViewMode.viewMode).toBe(VIEW_MODE.AGGREGATED_LEVEL);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
SEARCH_FIELDS_FROM_SOURCE,
SORT_DEFAULT_ORDER_SETTING,
} from '../../../../common';
import { isTextBasedQuery } from './is_text_based_query';
import { getValidViewMode } from './get_valid_view_mode';

function getDefaultColumns(savedSearch: SavedSearch, uiSettings: IUiSettingsClient) {
if (savedSearch.columns && savedSearch.columns.length > 0) {
Expand Down Expand Up @@ -81,7 +83,10 @@ export function getStateDefaults({
defaultState.rowHeight = savedSearch.rowHeight;
}
if (savedSearch.viewMode) {
defaultState.viewMode = savedSearch.viewMode;
defaultState.viewMode = getValidViewMode({
viewMode: savedSearch.viewMode,
isTextBasedQueryMode: isTextBasedQuery(query),
});
}
if (savedSearch.hideAggregatedPreview) {
defaultState.hideAggregatedPreview = savedSearch.hideAggregatedPreview;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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 { VIEW_MODE } from '@kbn/saved-search-plugin/common';
import { getValidViewMode } from './get_valid_view_mode';

describe('getValidViewMode', () => {
test('should work correctly for regular mode', () => {
expect(
getValidViewMode({
viewMode: undefined,
isTextBasedQueryMode: false,
})
).toBeUndefined();

expect(
getValidViewMode({
viewMode: VIEW_MODE.DOCUMENT_LEVEL,
isTextBasedQueryMode: false,
})
).toBe(VIEW_MODE.DOCUMENT_LEVEL);

expect(
getValidViewMode({
viewMode: VIEW_MODE.AGGREGATED_LEVEL,
isTextBasedQueryMode: false,
})
).toBe(VIEW_MODE.AGGREGATED_LEVEL);
});

test('should work correctly for text-based mode', () => {
expect(
getValidViewMode({
viewMode: undefined,
isTextBasedQueryMode: true,
})
).toBeUndefined();

expect(
getValidViewMode({
viewMode: VIEW_MODE.DOCUMENT_LEVEL,
isTextBasedQueryMode: true,
})
).toBe(VIEW_MODE.DOCUMENT_LEVEL);

expect(
getValidViewMode({
viewMode: VIEW_MODE.AGGREGATED_LEVEL,
isTextBasedQueryMode: true,
})
).toBe(VIEW_MODE.DOCUMENT_LEVEL);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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 { VIEW_MODE } from '@kbn/saved-search-plugin/public';

/**
* Returns a valid view mode
* @param viewMode
* @param isTextBasedQueryMode
*/
export const getValidViewMode = ({
viewMode,
isTextBasedQueryMode,
}: {
viewMode?: VIEW_MODE;
isTextBasedQueryMode: boolean;
}): VIEW_MODE | undefined => {
if (viewMode === VIEW_MODE.AGGREGATED_LEVEL && isTextBasedQueryMode) {
// only this mode is supported for text-based languages
return VIEW_MODE.DOCUMENT_LEVEL;
}

return viewMode;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* 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 { isTextBasedQuery } from './is_text_based_query';

describe('isTextBasedQuery', () => {
it('should work correctly', () => {
expect(isTextBasedQuery({ query: '', language: 'lucene' })).toEqual(false);
expect(isTextBasedQuery({ sql: 'SELECT * from foo' })).toEqual(true);
expect(isTextBasedQuery()).toEqual(false);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* 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 type { AggregateQuery, Query } from '@kbn/es-query';
import { RecordRawType } from '../services/discover_data_state_container';
import { getRawRecordType } from './get_raw_record_type';

/**
* Checks if the query is of AggregateQuery type
* @param query
*/
export function isTextBasedQuery(query?: Query | AggregateQuery): query is AggregateQuery {
return getRawRecordType(query) === RecordRawType.PLAIN;
}
19 changes: 14 additions & 5 deletions src/plugins/discover/public/embeddable/saved_search_embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { METRIC_TYPE } from '@kbn/analytics';
import { VIEW_MODE } from '../../common/constants';
import { getSortForEmbeddable, SortPair } from '../utils/sorting';
import { RecordRawType } from '../application/main/services/discover_data_state_container';
import { buildDataTableRecord } from '../utils/build_data_record';
import { DataTableRecord, EsHitRecord } from '../types';
import { ISearchEmbeddable, SearchInput, SearchOutput } from './types';
Expand All @@ -59,7 +58,8 @@ import { DiscoverGridSettings } from '../components/discover_grid/types';
import { DocTableProps } from '../components/doc_table/doc_table_wrapper';
import { updateSearchSource } from './utils/update_search_source';
import { FieldStatisticsTable } from '../application/main/components/field_stats_table';
import { getRawRecordType } from '../application/main/utils/get_raw_record_type';
import { isTextBasedQuery } from '../application/main/utils/is_text_based_query';
import { getValidViewMode } from '../application/main/utils/get_valid_view_mode';
import { fetchSql } from '../application/main/utils/fetch_sql';
import { ADHOC_DATA_VIEW_RENDER_EVENT } from '../constants';

Expand Down Expand Up @@ -169,6 +169,11 @@ export class SavedSearchEmbeddable
return true;
}

private isTextBasedSearch = (savedSearch: SavedSearch): boolean => {
const query = savedSearch.searchSource.getField('query');
return isTextBasedQuery(query);
};

private fetch = async () => {
const searchSessionId = this.input.searchSessionId;
const useNewFieldsApi = !this.services.uiSettings.get(SEARCH_FIELDS_FROM_SOURCE, false);
Expand Down Expand Up @@ -229,8 +234,7 @@ export class SavedSearchEmbeddable

const query = this.savedSearch.searchSource.getField('query');
const dataView = this.savedSearch.searchSource.getField('index')!;
const recordRawType = getRawRecordType(query);
const useSql = recordRawType === RecordRawType.PLAIN;
const useSql = this.isTextBasedSearch(this.savedSearch);

try {
// Request SQL data
Expand Down Expand Up @@ -515,9 +519,14 @@ export class SavedSearchEmbeddable
return;
}

const viewMode = getValidViewMode({
viewMode: this.savedSearch.viewMode,
isTextBasedQueryMode: this.isTextBasedSearch(this.savedSearch),
});

if (
this.services.uiSettings.get(SHOW_FIELD_STATISTICS) === true &&
this.savedSearch.viewMode === VIEW_MODE.AGGREGATED_LEVEL &&
viewMode === VIEW_MODE.AGGREGATED_LEVEL &&
searchProps.services &&
searchProps.dataView &&
Array.isArray(searchProps.columns)
Expand Down