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

hide time filter when query assistant is active #8921

Closed
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
2 changes: 2 additions & 0 deletions changelogs/fragments/8921.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Hide time filter when query assistant input is expanded ([#8921](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8921))
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { BehaviorSubject } from 'rxjs';
import { createEditor, DQLBody, SingleLineInput } from '../../../ui';
import { LanguageServiceContract } from './language_service';
import { LanguageConfig } from './types';
Expand Down Expand Up @@ -47,9 +48,11 @@ const createSetupLanguageServiceMock = (): jest.Mocked<LanguageServiceContract>
languages.set(language.id, language);
}),
getLanguage: jest.fn((id: string) => languages.get(id)),
getLanguage$: jest.fn((id: string) => new BehaviorSubject(languages.get(id))),
getLanguages: jest.fn(() => Array.from(languages.values())),
getDefaultLanguage: jest.fn(() => languages.get('kuery') || languages.values().next().value),
getQueryEditorExtensionMap: jest.fn().mockReturnValue({}),
updateLanguageConfig: jest.fn(),
resetUserQuery: jest.fn(),
getUserQueryLanguageBlocklist: jest.fn().mockReturnValue([]),
setUserQueryLanguageBlocklist: jest.fn().mockReturnValue(true),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { BehaviorSubject, of } from 'rxjs';
import { switchMap } from 'rxjs/operators';

import { LanguageConfig } from './types';
import { getDQLLanguageConfig, getLuceneLanguageConfig } from './lib';
import { ISearchInterceptor } from '../../../search';
Expand All @@ -19,6 +22,7 @@

export class LanguageService {
private languages: Map<string, LanguageConfig> = new Map();
private languages$: BehaviorSubject<Map<string, LanguageConfig>> = new BehaviorSubject(new Map());
private queryEditorExtensionMap: Record<string, QueryEditorExtensionConfig>;

constructor(
Expand Down Expand Up @@ -54,14 +58,30 @@
);
}

public registerLanguage(config: LanguageConfig): void {
private setLanguage(config: LanguageConfig) {
this.languages.set(config.id, config);
this.languages$.next(this.languages);
}

public registerLanguage(config: LanguageConfig): void {
this.setLanguage(config);
}

public updateLanguageConfig(languageId: string, config: Partial<LanguageConfig>) {
const language = this.languages.get(languageId);

Check warning on line 71 in src/plugins/data/public/query/query_string/language_service/language_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/language_service/language_service.ts#L71

Added line #L71 was not covered by tests
if (language) {
this.setLanguage({ ...language, ...config });

Check warning on line 73 in src/plugins/data/public/query/query_string/language_service/language_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/language_service/language_service.ts#L73

Added line #L73 was not covered by tests
}
}

public getLanguage(languageId: string): LanguageConfig | undefined {
return this.languages.get(languageId);
}

public getLanguage$(languageId: string) {
return this.languages$.pipe(switchMap((languages) => of(languages.get(languageId))));

Check warning on line 82 in src/plugins/data/public/query/query_string/language_service/language_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/language_service/language_service.ts#L82

Added line #L82 was not covered by tests
}

public getLanguages(): LanguageConfig[] {
return Array.from(this.languages.values());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
.osdQueryEditor__querycontrols {
.osdQueryEditor__extensionQueryControls {
display: flex;
padding: 0 $euiSizeS 0 $euiSizeXS;
padding: 0 $euiSizeS 0 $euiSizeS;
border-right: $euiBorderThin;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
import classNames from 'classnames';
import React, { useState } from 'react';
import { createPortal } from 'react-dom';
import { useObservable } from 'react-use';
import { of } from 'rxjs';
import {
DatasetSelector,
DatasetSelectorAppearance,
Expand Down Expand Up @@ -69,12 +71,19 @@ export interface QueryEditorTopRowProps {
// Needed for React.lazy
// eslint-disable-next-line import/no-default-export
export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
const queryLanguage = props.query && props.query.language;

const [isDateRangeInvalid, setIsDateRangeInvalid] = useState(false);
const [isQueryEditorFocused, setIsQueryEditorFocused] = useState(false);
const opensearchDashboards = useOpenSearchDashboards<IDataPluginServices>();
const { uiSettings, storage, appName, data } = opensearchDashboards.services;

const queryLanguage = props.query && props.query.language;
const language = useObservable(
queryLanguage
? data.query.queryString.getLanguageService().getLanguage$(queryLanguage)
: of(undefined)
);

const persistedLog: PersistedLog | undefined = React.useMemo(
() =>
queryLanguage && uiSettings && storage && appName
Expand Down Expand Up @@ -227,10 +236,7 @@ export default function QueryEditorTopRow(props: QueryEditorTopRowProps) {
function shouldRenderDatePicker(): boolean {
return (
Boolean((props.showDatePicker || props.showAutoRefreshOnly) ?? true) &&
!(
queryLanguage &&
data.query.queryString.getLanguageService().getLanguage(queryLanguage)?.hideDatePicker
) &&
!(queryLanguage && language?.hideDatePicker) &&
(props.query?.dataset
? data.query.queryString.getDatasetService().getType(props.query.dataset.type)?.meta
?.supportsTimeFilter !== false
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/query_enhancements/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export class QueryEnhancementsPlugin
constructor(initializerContext: PluginInitializerContext) {
this.config = initializerContext.config.get<ConfigSchema>();
this.storage = new DataStorage(window.localStorage, 'discover.queryAssist.');
setStorage(this.storage);
}

public setup(
Expand Down Expand Up @@ -203,7 +204,6 @@ export class QueryEnhancementsPlugin
core: CoreStart,
deps: QueryEnhancementsPluginStartDependencies
): QueryEnhancementsPluginStart {
setStorage(this.storage);
setData(deps.data);
return {};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,7 @@
.osdQueryEditorExtensionComponent__query-assist {
flex-grow: 1;
}

.osdQueryEditorExtensionQueryControls__assistant-query-actions {
padding-left: $euiSizeXS;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { firstValueFrom } from '@osd/std';
import { act, render, screen } from '@testing-library/react';
import { act, render, screen, waitFor } from '@testing-library/react';
import React from 'react';
import { of } from 'rxjs';
import { coreMock } from '../../../../../core/public/mocks';
Expand All @@ -13,6 +13,16 @@ import { dataPluginMock } from '../../../../data/public/mocks';
import { ConfigSchema } from '../../../common/config';
import { clearCache, createQueryAssistExtension } from './create_extension';
import { ResultStatus } from '../../../../discover/public';
import { languageServiceMock } from '../../../../data/public/query/query_string/language_service/language_service.mock';

const mockGet = jest.fn();
const mockSet = jest.fn();
jest.mock('../../services', () => ({
getStorage: () => ({
get: mockGet,
set: mockSet,
}),
}));

const coreSetupMock = coreMock.createSetup({
pluginStartDeps: {
Expand Down Expand Up @@ -60,6 +70,7 @@ describe('CreateExtension', () => {
};
afterEach(() => {
jest.clearAllMocks();
jest.restoreAllMocks();
clearCache();
});

Expand Down Expand Up @@ -89,7 +100,7 @@ describe('CreateExtension', () => {
});

it('creates data structure meta', async () => {
httpMock.get.mockResolvedValueOnce({ configuredLanguages: ['PPL'] });
httpMock.get.mockResolvedValue({ configuredLanguages: ['PPL'] });
const extension = createQueryAssistExtension(coreSetupMock, dataMock, config);
const meta = await extension.getDataStructureMeta?.('mock-data-source-id2');
expect(meta).toMatchInlineSnapshot(`
Expand All @@ -108,14 +119,17 @@ describe('CreateExtension', () => {
});

it('does not send multiple requests for the same data source', async () => {
httpMock.get.mockResolvedValueOnce({ configuredLanguages: ['PPL'] });
httpMock.get.mockResolvedValue({ configuredLanguages: ['PPL'] });
const extension = createQueryAssistExtension(coreSetupMock, dataMock, config);
const metas = await Promise.all(
Array.from({ length: 10 }, () => extension.getDataStructureMeta?.('mock-data-source-id2'))
);
metas.push(await extension.getDataStructureMeta?.('mock-data-source-id2'));
metas.forEach((meta) => expect(meta?.type).toBe('FEATURE'));
expect(httpMock.get).toBeCalledTimes(1);
const callsWithMockDataSourceId2 = httpMock.get.mock.calls.filter(
(args) => args[1].query.dataSourceId === 'mock-data-source-id2'
);
expect(callsWithMockDataSourceId2).toHaveLength(1);
});

it('should render the component if language is supported', async () => {
Expand Down Expand Up @@ -180,4 +194,54 @@ describe('CreateExtension', () => {

expect(screen.getByText('QueryAssistSummary')).toBeInTheDocument();
});

it('should hide date picker if query assistant is expanded', async () => {
// Query assistant NOT collapsed
mockGet.mockReturnValue(false);

const languageService = languageServiceMock.createSetupContract();
languageService.getLanguage.mockReturnValue({ id: 'PPL' });
queryStringMock.getLanguageService.mockReturnValue(languageService);

queryStringMock.getUpdates$.mockReturnValue(
of({ ...mockQueryWithIndexPattern, language: 'PPL' })
);

httpMock.get.mockResolvedValueOnce({ configuredLanguages: ['PPL'] });
createQueryAssistExtension(coreSetupMock, dataMock, config);
await waitFor(() => {
// language config updated
expect(languageService.updateLanguageConfig).toHaveBeenCalledWith('PPL', {
hideDatePicker: true,
});
});

// reset
queryStringMock.getUpdates$.mockReturnValue(of(mockQueryWithIndexPattern));
});

it('should NOT hide date picker if query assistant collapsed', async () => {
// Query assistant is collapsed
mockGet.mockReturnValue(true);

const languageService = languageServiceMock.createSetupContract();
languageService.getLanguage.mockReturnValue({ id: 'PPL' });
queryStringMock.getLanguageService.mockReturnValue(languageService);

queryStringMock.getUpdates$.mockReturnValue(
of({ ...mockQueryWithIndexPattern, language: 'PPL' })
);

httpMock.get.mockResolvedValueOnce({ configuredLanguages: ['PPL'] });
createQueryAssistExtension(coreSetupMock, dataMock, config);
await waitFor(() => {
// language config updated
expect(languageService.updateLanguageConfig).toHaveBeenCalledWith('PPL', {
hideDatePicker: false,
});
});

// reset
queryStringMock.getUpdates$.mockReturnValue(of(mockQueryWithIndexPattern));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { i18n } from '@osd/i18n';
import { HttpSetup } from 'opensearch-dashboards/public';
import React, { useEffect, useState } from 'react';
import { BehaviorSubject } from 'rxjs';
import { BehaviorSubject, combineLatest } from 'rxjs';
import { distinctUntilChanged, map, startWith, switchMap } from 'rxjs/operators';
import { DATA_STRUCTURE_META_TYPES, DEFAULT_DATA } from '../../../../data/common';
import {
Expand All @@ -26,6 +26,7 @@ import {
import { UsageCollectionSetup } from '../../../../usage_collection/public';
import { QueryAssistContext } from '../hooks/use_query_assist';
import { CoreSetup } from '../../../../../core/public';
import { getStorage } from '../../services';

const [getAvailableLanguagesForDataSource, clearCache] = (() => {
const availableLanguagesByDataSource: Map<string | undefined, string[]> = new Map();
Expand Down Expand Up @@ -99,9 +100,44 @@ export const createQueryAssistExtension = (
config: ConfigSchema['queryAssist'],
usageCollection?: UsageCollectionSetup
): QueryEditorExtensionConfig => {
const QUERY_ASSIST_COLLAPSED_STORAGE_KEY = 'query_input_collapsed';
const http: HttpSetup = core.http;
const isQueryAssistCollapsed$ = new BehaviorSubject<boolean>(false);

// read query assistant collapsed state from local storage, if not yet set to local storage,
// the default value is `false` which query assistant is expanded.
const isQueryAssistCollapsed$ = new BehaviorSubject<boolean>(
getStorage().get(QUERY_ASSIST_COLLAPSED_STORAGE_KEY) ?? false
);

const assistantEnabled$ = getAvailableLanguages$(http, data).pipe(
map((languages) => languages.length > 0)
);
const question$ = new BehaviorSubject('');

// persist the query assistant collapsed state to local storage
isQueryAssistCollapsed$.subscribe((val) => {
getStorage().set(QUERY_ASSIST_COLLAPSED_STORAGE_KEY, val);
});

combineLatest([
assistantEnabled$,
isQueryAssistCollapsed$,
data.query.queryString.getUpdates$(),
]).subscribe(([assistantEnabled, collapsed, query]) => {
const languageService = data.query.queryString.getLanguageService();
const pplLanguage = languageService.getLanguage('PPL');
if (!pplLanguage || !assistantEnabled || query.language !== 'PPL') {
return;
}
// Don't include time filter in the query from queryService if assistant is active(expanded)
// because the time filter will conflict with the time filter in PPL which generated by query assistant
if (!collapsed) {
languageService.updateLanguageConfig('PPL', { hideDatePicker: true });
} else {
languageService.updateLanguageConfig('PPL', { hideDatePicker: false });
}
});

return {
id: 'query-assist',
order: 1000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,21 @@ export class PPLSearchInterceptor extends SearchInterceptor {

private buildQuery() {
const query: Query = this.queryService.queryString.getQuery();
const language = this.queryService.queryString.getLanguageService().getLanguage('PPL');
const dataset = query.dataset;
if (!dataset || !dataset.timeFieldName) return query;

const [baseQuery, ...afterPipeParts] = query.query.split('|');
const afterPipe = afterPipeParts.length > 0 ? ` | ${afterPipeParts.join('|').trim()}` : '';
const timeFilter = this.getTimeFilter(dataset.timeFieldName);
return { ...query, query: baseQuery + timeFilter + afterPipe };

// If hideDatePicker is NOT set specifically to `true`, it should include time filter to the ppl query
const includeTimeFilter = language?.hideDatePicker !== true;
if (includeTimeFilter) {
const timeFilter = this.getTimeFilter(dataset.timeFieldName);
return { ...query, query: baseQuery + timeFilter + afterPipe };
}

return { ...query, query: baseQuery + afterPipe };
}

private getAggConfig(request: IOpenSearchDashboardsSearchRequest, query: Query) {
Expand Down
Loading