From f062a53a17197d388189ac811eca714911dfbb5d Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Mon, 25 Nov 2024 15:47:21 +0800 Subject: [PATCH 1/3] hide time filter when query assistant is active Signed-off-by: Yulong Ruan --- .../language_service/language_service.ts | 22 +++++++++- .../public/ui/query_editor/_query_editor.scss | 2 +- .../ui/query_editor/query_editor_top_row.tsx | 16 +++++--- .../query_enhancements/public/plugin.tsx | 2 +- .../public/query_assist/_index.scss | 4 ++ .../query_assist/utils/create_extension.tsx | 40 ++++++++++++++++++- .../public/search/ppl_search_interceptor.ts | 13 +++++- 7 files changed, 87 insertions(+), 12 deletions(-) diff --git a/src/plugins/data/public/query/query_string/language_service/language_service.ts b/src/plugins/data/public/query/query_string/language_service/language_service.ts index 7d3a272d1fef..1c522117e50c 100644 --- a/src/plugins/data/public/query/query_string/language_service/language_service.ts +++ b/src/plugins/data/public/query/query_string/language_service/language_service.ts @@ -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'; @@ -19,6 +22,7 @@ import { luceneLanguageReference } from './lib/lucene_language_reference'; export class LanguageService { private languages: Map = new Map(); + private languages$: BehaviorSubject> = new BehaviorSubject(new Map()); private queryEditorExtensionMap: Record; constructor( @@ -54,14 +58,30 @@ export class LanguageService { ); } - 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) { + const language = this.languages.get(languageId); + if (language) { + this.setLanguage({ ...language, ...config }); + } } 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)))); + } + public getLanguages(): LanguageConfig[] { return Array.from(this.languages.values()); } diff --git a/src/plugins/data/public/ui/query_editor/_query_editor.scss b/src/plugins/data/public/ui/query_editor/_query_editor.scss index d6577d33fdd3..6d02990c356a 100644 --- a/src/plugins/data/public/ui/query_editor/_query_editor.scss +++ b/src/plugins/data/public/ui/query_editor/_query_editor.scss @@ -152,7 +152,7 @@ .osdQueryEditor__querycontrols { .osdQueryEditor__extensionQueryControls { display: flex; - padding: 0 $euiSizeS 0 $euiSizeXS; + padding: 0 $euiSizeS 0 $euiSizeS; border-right: $euiBorderThin; } } diff --git a/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx b/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx index ea15fbfeeaa1..cc8f3731fe75 100644 --- a/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx +++ b/src/plugins/data/public/ui/query_editor/query_editor_top_row.tsx @@ -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, @@ -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(); 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 @@ -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 diff --git a/src/plugins/query_enhancements/public/plugin.tsx b/src/plugins/query_enhancements/public/plugin.tsx index 715ba53f0153..8ef6febbb83d 100644 --- a/src/plugins/query_enhancements/public/plugin.tsx +++ b/src/plugins/query_enhancements/public/plugin.tsx @@ -39,6 +39,7 @@ export class QueryEnhancementsPlugin constructor(initializerContext: PluginInitializerContext) { this.config = initializerContext.config.get(); this.storage = new DataStorage(window.localStorage, 'discover.queryAssist.'); + setStorage(this.storage); } public setup( @@ -203,7 +204,6 @@ export class QueryEnhancementsPlugin core: CoreStart, deps: QueryEnhancementsPluginStartDependencies ): QueryEnhancementsPluginStart { - setStorage(this.storage); setData(deps.data); return {}; } diff --git a/src/plugins/query_enhancements/public/query_assist/_index.scss b/src/plugins/query_enhancements/public/query_assist/_index.scss index ee3543d870c0..4787e19ed9fc 100644 --- a/src/plugins/query_enhancements/public/query_assist/_index.scss +++ b/src/plugins/query_enhancements/public/query_assist/_index.scss @@ -47,3 +47,7 @@ .osdQueryEditorExtensionComponent__query-assist { flex-grow: 1; } + +.osdQueryEditorExtensionQueryControls__assistant-query-actions { + padding-left: $euiSizeXS; +} diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx index 238ef40ff354..df1395215d46 100644 --- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx +++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.tsx @@ -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 { @@ -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 = new Map(); @@ -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(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( + 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, diff --git a/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts b/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts index 57152dbe98ea..1604c8347b02 100644 --- a/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts +++ b/src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts @@ -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) { From 3579c63e0986d649006996eacdd026f7c1e7aaca Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Mon, 25 Nov 2024 19:54:47 +0800 Subject: [PATCH 2/3] add tests for query assistant editor extension Signed-off-by: Yulong Ruan --- .../language_service/language_service.mock.ts | 3 + .../utils/create_extension.test.tsx | 72 +++++++++++++++++-- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/plugins/data/public/query/query_string/language_service/language_service.mock.ts b/src/plugins/data/public/query/query_string/language_service/language_service.mock.ts index e481932883ca..c12b6867f2f1 100644 --- a/src/plugins/data/public/query/query_string/language_service/language_service.mock.ts +++ b/src/plugins/data/public/query/query_string/language_service/language_service.mock.ts @@ -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'; @@ -47,9 +48,11 @@ const createSetupLanguageServiceMock = (): jest.Mocked 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), diff --git a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx index fc790cc79c11..6280e47bd4ef 100644 --- a/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx +++ b/src/plugins/query_enhancements/public/query_assist/utils/create_extension.test.tsx @@ -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'; @@ -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: { @@ -60,6 +70,7 @@ describe('CreateExtension', () => { }; afterEach(() => { jest.clearAllMocks(); + jest.restoreAllMocks(); clearCache(); }); @@ -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(` @@ -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 () => { @@ -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)); + }); }); From 34790b370832ad7accc06597d248167001b5256a Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 12:26:19 +0000 Subject: [PATCH 3/3] Changeset file for PR #8921 created/updated --- changelogs/fragments/8921.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/8921.yml diff --git a/changelogs/fragments/8921.yml b/changelogs/fragments/8921.yml new file mode 100644 index 000000000000..b3ca28b881d4 --- /dev/null +++ b/changelogs/fragments/8921.yml @@ -0,0 +1,2 @@ +feat: +- Hide time filter when query assistant input is expanded ([#8921](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8921)) \ No newline at end of file