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][Lens] Removes the dataview dependency from the text based mode #158531

Merged
merged 20 commits into from
May 31, 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
30 changes: 29 additions & 1 deletion packages/kbn-text-based-editor/src/editor_footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ interface EditorFooterProps {
lines: number;
containerCSS: Interpolation<Theme>;
errors?: MonacoError[];
detectTimestamp: boolean;
onErrorClick: (error: MonacoError) => void;
refreshErrors: () => void;
}
Expand All @@ -40,6 +41,7 @@ export const EditorFooter = memo(function EditorFooter({
lines,
containerCSS,
errors,
detectTimestamp,
onErrorClick,
refreshErrors,
}: EditorFooterProps) {
Expand All @@ -54,7 +56,7 @@ export const EditorFooter = memo(function EditorFooter({
>
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="s" responsive={false} alignItems="center">
<EuiFlexItem grow={false} style={{ marginRight: '16px' }}>
<EuiFlexItem grow={false} style={{ marginRight: '8px' }}>
<EuiText size="xs" color="subdued" data-test-subj="TextBasedLangEditor-footer-lines">
<p>
{i18n.translate('textBasedEditor.query.textBasedLanguagesEditor.lineCount', {
Expand All @@ -64,6 +66,32 @@ export const EditorFooter = memo(function EditorFooter({
</p>
</EuiText>
</EuiFlexItem>
<EuiFlexItem grow={false} style={{ marginRight: '16px' }}>
stratoula marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup gutterSize="xs" responsive={false} alignItems="center">
<EuiFlexItem grow={false}>
<EuiIcon type="calendar" color="subdued" size="s" />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText size="xs" color="subdued" data-test-subj="TextBasedLangEditor-date-info">
<p>
{detectTimestamp
? i18n.translate(
'textBasedEditor.query.textBasedLanguagesEditor.timestampDetected',
{
defaultMessage: '@timestamp detected',
}
)
: i18n.translate(
'textBasedEditor.query.textBasedLanguagesEditor.timestampNotDetected',
{
defaultMessage: '@timestamp not detected',
}
)}
</p>
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
{errors && errors.length > 0 && (
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="xs" responsive={false} alignItems="center">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,33 @@ describe('TextBasedLanguagesEditor', () => {
});
});

it('should render the date info with no @timestamp detected', async () => {
const newProps = {
...props,
isCodeEditorExpanded: true,
};
await act(async () => {
const component = mount(renderTextBasedLanguagesEditorComponent({ ...newProps }));
expect(
component.find('[data-test-subj="TextBasedLangEditor-date-info"]').at(0).text()
).toStrictEqual('@timestamp not detected');
});
});

it('should render the date info with @timestamp detected if detectTimestamp is true', async () => {
const newProps = {
...props,
isCodeEditorExpanded: true,
detectTimestamp: true,
};
await act(async () => {
const component = mount(renderTextBasedLanguagesEditorComponent({ ...newProps }));
expect(
component.find('[data-test-subj="TextBasedLangEditor-date-info"]').at(0).text()
).toStrictEqual('@timestamp detected');
});
});

it('should render the errors badge for the inline mode by default if errors are provides', async () => {
const newProps = {
...props,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export interface TextBasedLanguagesEditorProps {
onTextLangQuerySubmit: () => void;
expandCodeEditor: (status: boolean) => void;
isCodeEditorExpanded: boolean;
detectTimestamp?: boolean;
errors?: Error[];
isDisabled?: boolean;
isDarkMode?: boolean;
Expand Down Expand Up @@ -87,6 +88,7 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({
onTextLangQuerySubmit,
expandCodeEditor,
isCodeEditorExpanded,
detectTimestamp = false,
errors,
isDisabled,
isDarkMode,
Expand Down Expand Up @@ -537,6 +539,7 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({
errors={editorErrors}
onErrorClick={onErrorClick}
refreshErrors={onTextLangQuerySubmit}
detectTimestamp={detectTimestamp}
/>
)}
</div>
Expand Down Expand Up @@ -608,6 +611,7 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({
errors={editorErrors}
onErrorClick={onErrorClick}
refreshErrors={onTextLangQuerySubmit}
detectTimestamp={detectTimestamp}
/>
)}
{isCodeEditorExpanded && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ export const DiscoverTopNav = ({
const dataView = useInternalStateSelector((state) => state.dataView!);
const savedDataViews = useInternalStateSelector((state) => state.savedDataViews);
const savedSearch = useSavedSearchInitial();
const showDatePicker = useMemo(
() => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP,
[dataView]
);
const showDatePicker = useMemo(() => {
// always show the timepicker for text based languages
return (
isPlainRecord ||
(!isPlainRecord && dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP)
);
}, [dataView, isPlainRecord]);
const services = useDiscoverServices();
const { dataViewEditor, navigation, dataViewFieldEditor, data, uiSettings, dataViews } = services;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { DiscoverMainProvider } from '../services/discover_state_provider';
import { DiscoverAppState } from '../services/discover_app_state_container';
import { DiscoverStateContainer } from '../services/discover_state';
import { VIEW_MODE } from '@kbn/saved-search-plugin/public';
import { dataViewAdHoc } from '../../../__mocks__/data_view_complex';

function getHookProps(
query: AggregateQuery | Query | undefined,
Expand Down Expand Up @@ -84,6 +85,7 @@ const renderHookWithContext = (
appState?: DiscoverAppState
) => {
const props = getHookProps(query, useDataViewsService ? getDataViewsService() : undefined);
props.stateContainer.actions.setDataView(dataViewMock);
if (appState) {
props.stateContainer.appState.getState = jest.fn(() => {
return appState;
Expand All @@ -98,7 +100,7 @@ const renderHookWithContext = (

describe('useTextBasedQueryLanguage', () => {
test('a text based query should change state when loading and finished', async () => {
const { replaceUrlState, stateContainer } = renderHookWithContext(false);
const { replaceUrlState, stateContainer } = renderHookWithContext(true);

await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));
expect(replaceUrlState).toHaveBeenCalledWith({ index: 'the-data-view-id' });
Expand Down Expand Up @@ -191,11 +193,7 @@ describe('useTextBasedQueryLanguage', () => {
query: { sql: 'SELECT field1 from the-data-view-title WHERE field1=1' },
});

await waitFor(() => {
expect(replaceUrlState).toHaveBeenCalledWith({
index: 'the-data-view-id',
});
});
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0));
});
test('if its not a text based query coming along, it should be ignored', async () => {
const { replaceUrlState, stateContainer } = renderHookWithContext(false);
Expand Down Expand Up @@ -270,7 +268,7 @@ describe('useTextBasedQueryLanguage', () => {
],
query: { sql: 'SELECT field1 from the-data-view-title' },
});
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2));
davismcphee marked this conversation as resolved.
Show resolved Hide resolved
expect(replaceUrlState).toHaveBeenCalledWith({
columns: ['field1'],
});
Expand All @@ -288,7 +286,7 @@ describe('useTextBasedQueryLanguage', () => {
fetchStatus: FetchStatus.LOADING,
query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' },
});
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(0));
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));
documents$.next({
recordRawType: RecordRawType.PLAIN,
fetchStatus: FetchStatus.COMPLETE,
Expand All @@ -301,7 +299,7 @@ describe('useTextBasedQueryLanguage', () => {
],
query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' },
});
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(2));
stateContainer.appState.getState = jest.fn(() => {
return { columns: ['field1', 'field2'], index: 'the-data-view-id' };
});
Expand Down Expand Up @@ -344,17 +342,10 @@ describe('useTextBasedQueryLanguage', () => {
});

test('changing a text based query with an index pattern that not corresponds to a dataview should return results', async () => {
const dataViewsCreateMock = discoverServiceMock.dataViews.create as jest.Mock;
dataViewsCreateMock.mockImplementation(() => ({
...dataViewMock,
}));
const dataViewsService = {
...discoverServiceMock.dataViews,
create: dataViewsCreateMock,
};
const props = getHookProps(query, dataViewsService);
const props = getHookProps(query, discoverServiceMock.dataViews);
const { stateContainer, replaceUrlState } = props;
const documents$ = stateContainer.dataState.data$.documents$;
props.stateContainer.actions.setDataView(dataViewMock);

renderHook(() => useTextBasedQueryLanguage(props), { wrapper: getHookContext(stateContainer) });

Expand All @@ -374,6 +365,7 @@ describe('useTextBasedQueryLanguage', () => {
],
query: { sql: 'SELECT field1 from the-data-view-*' },
});
props.stateContainer.actions.setDataView(dataViewAdHoc);
await waitFor(() => expect(replaceUrlState).toHaveBeenCalledTimes(1));

await waitFor(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export function useTextBasedQueryLanguage({
columns: [],
query: undefined,
});
const indexTitle = useRef<string>('');
const savedSearch = useSavedSearchInitial();

const cleanup = useCallback(() => {
Expand Down Expand Up @@ -80,36 +81,24 @@ export function useTextBasedQueryLanguage({
}
}
const indexPatternFromQuery = getIndexPatternFromSQLQuery(query.sql);
const internalState = stateContainer.internalState.getState();
const dataViewList = [...internalState.savedDataViews, ...internalState.adHocDataViews];
let dataViewObj = dataViewList.find(({ title }) => title === indexPatternFromQuery);

// no dataview found but the index pattern is valid
// create an adhoc instance instead
if (!dataViewObj) {
dataViewObj = await dataViews.create({
title: indexPatternFromQuery,
});
stateContainer.internalState.transitions.setAdHocDataViews([dataViewObj]);

if (dataViewObj.fields.getByName('@timestamp')?.type === 'date') {
dataViewObj.timeFieldName = '@timestamp';
} else if (dataViewObj.fields.getByType('date')?.length) {
const dateFields = dataViewObj.fields.getByType('date');
dataViewObj.timeFieldName = dateFields[0].name;
}
}
const dataViewObj = stateContainer.internalState.getState().dataView!;

// don't set the columns on initial fetch, to prevent overwriting existing state
const addColumnsToState = Boolean(
nextColumns.length && (!initialFetch || !stateColumns?.length)
);
// no need to reset index to state if it hasn't changed
const addDataViewToState = Boolean(dataViewObj.id !== index);
if (!addColumnsToState && !addDataViewToState) {
const addDataViewToState = Boolean(dataViewObj?.id !== index) || initialFetch;
const queryChanged = indexPatternFromQuery !== indexTitle.current;
if (!addColumnsToState && !queryChanged) {
return;
}

if (queryChanged) {
indexTitle.current = indexPatternFromQuery;
}

const nextState = {
...(addDataViewToState && { index: dataViewObj.id }),
...(addColumnsToState && { columns: nextColumns }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
isEqualState,
} from '../../services/discover_app_state_container';
import { addLog } from '../../../../utils/add_log';
import { isTextBasedQuery } from '../../utils/is_text_based_query';
import { FetchStatus } from '../../../types';
import { loadAndResolveDataView } from '../../utils/resolve_data_view';

Expand Down Expand Up @@ -61,7 +62,7 @@ export const buildStateSubscribe =
// NOTE: this is also called when navigating from discover app to context app
if (nextState.index && dataViewChanged) {
const { dataView: nextDataView, fallback } = await loadAndResolveDataView(
{ id: nextState.index, savedSearch },
{ id: nextState.index, savedSearch, isTextBasedQuery: isTextBasedQuery(nextState?.query) },
{ internalStateContainer: internalState, services }
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import { RequestAdapter } from '@kbn/inspector-plugin/common';
import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { AggregateQuery, Query } from '@kbn/es-query';
import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types';
import { DataView } from '@kbn/data-views-plugin/common';
import { getDataViewByTextBasedQueryLang } from '../utils/get_data_view_by_text_based_query_lang';
import { isTextBasedQuery } from '../utils/is_text_based_query';
import { getRawRecordType } from '../utils/get_raw_record_type';
import { DiscoverAppState } from './discover_app_state_container';
import { DiscoverServices } from '../../../build_services';
Expand Down Expand Up @@ -129,11 +132,13 @@ export function getDataStateContainer({
searchSessionManager,
getAppState,
getSavedSearch,
setDataView,
}: {
services: DiscoverServices;
searchSessionManager: DiscoverSearchSessionManager;
getAppState: () => DiscoverAppState;
getSavedSearch: () => SavedSearch;
setDataView: (dataView: DataView) => void;
}): DiscoverDataStateContainer {
const { data, uiSettings, toastNotifications } = services;
const { timefilter } = data.query.timefilter;
Expand Down Expand Up @@ -226,7 +231,17 @@ export function getDataStateContainer({
};
}

const fetchQuery = (resetQuery?: boolean) => {
const fetchQuery = async (resetQuery?: boolean) => {
const query = getAppState().query;
const currentDataView = getSavedSearch().searchSource.getField('index');

if (isTextBasedQuery(query)) {
const nextDataView = await getDataViewByTextBasedQueryLang(query, currentDataView, services);
if (nextDataView !== currentDataView) {
setDataView(nextDataView);
}
}

if (resetQuery) {
refetch$.next('reset');
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ const startSync = (appState: DiscoverAppStateContainer) => {
async function getState(url: string = '/', savedSearch?: SavedSearch) {
const nextHistory = createBrowserHistory();
nextHistory.push(url);

discoverServiceMock.dataViews.create = jest.fn().mockReturnValue({
...dataViewMock,
isPersisted: () => false,
id: 'ad-hoc-id',
title: 'test',
});

const nextState = getDiscoverStateContainer({
services: discoverServiceMock,
history: nextHistory,
Expand Down Expand Up @@ -635,12 +643,6 @@ describe('Test discover state actions', () => {
});

test('onCreateDefaultAdHocDataView', async () => {
discoverServiceMock.dataViews.create = jest.fn().mockReturnValue({
...dataViewMock,
isPersisted: () => false,
id: 'ad-hoc-id',
title: 'test',
});
const { state } = await getState('/', savedSearchMock);
await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id });
const unsubscribe = state.actions.initializeAndSync();
Expand Down
Loading