From a70a7b83b0165c66391433ba98d4c09ad5127902 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Wed, 30 Oct 2024 16:01:09 -0700 Subject: [PATCH] [data] updates query and lang if language is not supported by query data (#8749) * [data] updates query and lang if language is not supported by query data If the query currently has a language set that is not valid for that dataset type then update the language and get the initial query for that language and dataset and then send the query. The scenario we can see this is when we are on an index pattern, and DQL selected. Then from recent datasets select an S3 connection. The dataset updates but the language does not update so therefore the query is invalid. Issue: n/a Signed-off-by: Kawika Avilla * Changeset file for PR #8749 created/updated * update string in notification Signed-off-by: Kawika Avilla --------- Signed-off-by: Kawika Avilla Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> --- changelogs/fragments/8749.yml | 2 + .../query_string/query_string_manager.test.ts | 115 ++++++++++++++++++ .../query_string/query_string_manager.ts | 44 +++++-- .../query_editor/language_selector.test.tsx | 3 - 4 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 changelogs/fragments/8749.yml diff --git a/changelogs/fragments/8749.yml b/changelogs/fragments/8749.yml new file mode 100644 index 000000000000..9963e9cf771a --- /dev/null +++ b/changelogs/fragments/8749.yml @@ -0,0 +1,2 @@ +fix: +- Updates query and language if language is not supported by query data ([#8749](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8749)) \ No newline at end of file diff --git a/src/plugins/data/public/query/query_string/query_string_manager.test.ts b/src/plugins/data/public/query/query_string/query_string_manager.test.ts index e7507747fdd4..da054574ff85 100644 --- a/src/plugins/data/public/query/query_string/query_string_manager.test.ts +++ b/src/plugins/data/public/query/query_string/query_string_manager.test.ts @@ -141,6 +141,121 @@ describe('QueryStringManager', () => { expect(query).toHaveProperty('dataset', dataset); }); + describe('setQuery', () => { + beforeEach(() => { + // Mock dataset service and language service methods + service.getDatasetService().getType = jest.fn().mockReturnValue({ + supportedLanguages: jest.fn(), + }); + service.getLanguageService().getLanguage = jest.fn().mockReturnValue({ + title: 'SQL', + getQueryString: jest.fn().mockReturnValue('SELECT * FROM table'), + }); + }); + + test('updates query when no dataset change', () => { + const newQuery = { query: 'test query' }; + service.setQuery(newQuery); + expect(service.getQuery().query).toBe('test query'); + }); + + test('adds dataset to recent datasets when dataset changes', () => { + const mockDataset = { + id: 'test-id', + title: 'Test Dataset', + type: 'INDEX_PATTERN', + }; + const addRecentDatasetSpy = jest.spyOn(service.getDatasetService(), 'addRecentDataset'); + + service.setQuery({ dataset: mockDataset }); + + expect(addRecentDatasetSpy).toHaveBeenCalledWith(mockDataset); + }); + + test('changes language when dataset does not support current language', () => { + // Setup initial query with 'kuery' language + service.setQuery({ query: 'test', language: 'kuery' }); + + const mockDataset = { + id: 'test-id', + title: 'Test Dataset', + type: 'S3', + }; + + // Mock that S3 only supports SQL + service.getDatasetService().getType = jest.fn().mockReturnValue({ + supportedLanguages: jest.fn().mockReturnValue(['sql']), + }); + + service.setQuery({ dataset: mockDataset }); + + const resultQuery = service.getQuery(); + expect(resultQuery.language).toBe('sql'); + expect(resultQuery.dataset).toBe(mockDataset); + expect(resultQuery.query).toBe('SELECT * FROM table'); + }); + + test('maintains current language when supported by new dataset', () => { + // Setup initial query + const initialQuery = { query: 'test', language: 'sql' }; + service.setQuery(initialQuery); + + const mockDataset = { + id: 'test-id', + title: 'Test Dataset', + type: 'S3', + }; + + // Mock that dataset supports SQL + service.getDatasetService().getType = jest.fn().mockReturnValue({ + supportedLanguages: jest.fn().mockReturnValue(['sql', 'ppl']), + }); + + service.setQuery({ dataset: mockDataset }); + + const resultQuery = service.getQuery(); + expect(resultQuery.language).toBe('sql'); + expect(resultQuery.dataset).toBe(mockDataset); + }); + + test('does not trigger updates when new query equals current query', () => { + const initialQuery = { query: 'test', language: 'sql' }; + let updateCount = 0; + + // Subscribe to updates + service.getUpdates$().subscribe(() => { + updateCount++; + }); + + service.setQuery(initialQuery); + expect(updateCount).toBe(1); + + // Set same query again + service.setQuery({ ...initialQuery }); + expect(updateCount).toBe(1); // Should not increment + }); + + test('handles undefined supportedLanguages gracefully', () => { + service.setQuery({ query: 'test', language: 'sql' }); + + const mockDataset = { + id: 'test-id', + title: 'Test Dataset', + type: 'UNKNOWN', + }; + + // Mock undefined supportedLanguages + service.getDatasetService().getType = jest.fn().mockReturnValue({ + supportedLanguages: jest.fn().mockReturnValue(undefined), + }); + + service.setQuery({ dataset: mockDataset }); + + const resultQuery = service.getQuery(); + expect(resultQuery.language).toBe('sql'); // Should maintain current language + }); + }); + describe('getInitialQuery', () => { const mockDataset = { id: 'test-dataset', diff --git a/src/plugins/data/public/query/query_string/query_string_manager.ts b/src/plugins/data/public/query/query_string/query_string_manager.ts index 65591d49cdd9..33bfc7d5d10b 100644 --- a/src/plugins/data/public/query/query_string/query_string_manager.ts +++ b/src/plugins/data/public/query/query_string/query_string_manager.ts @@ -31,7 +31,7 @@ import { BehaviorSubject } from 'rxjs'; import { skip } from 'rxjs/operators'; import { CoreStart, NotificationsSetup } from 'opensearch-dashboards/public'; -import { debounce, isEqual } from 'lodash'; +import { isEqual } from 'lodash'; import { i18n } from '@osd/i18n'; import { Dataset, DataStorage, Query, TimeRange, UI_SETTINGS } from '../../../common'; import { createHistory, QueryHistory } from './query_history'; @@ -154,10 +154,40 @@ export class QueryStringManager { */ public setQuery = (query: Partial) => { const curQuery = this.query$.getValue(); - const newQuery = { ...curQuery, ...query }; + let newQuery = { ...curQuery, ...query }; if (!isEqual(curQuery, newQuery)) { - // Add to recent datasets if dataset has changed + // Check if dataset changed and if new dataset has language restrictions if (newQuery.dataset && !isEqual(curQuery.dataset, newQuery.dataset)) { + // Get supported languages for the dataset + const supportedLanguages = this.datasetService + .getType(newQuery.dataset.type) + ?.supportedLanguages(newQuery.dataset); + + // If we have supported languages and current language isn't supported + if (supportedLanguages && !supportedLanguages.includes(newQuery.language)) { + // Get initial query with first supported language and new dataset + newQuery = this.getInitialQuery({ + language: supportedLanguages[0], + dataset: newQuery.dataset, + }); + + // Show warning about language change + showWarning(this.notifications, { + title: i18n.translate('data.languageChangeTitle', { + defaultMessage: 'Language Changed', + }), + text: i18n.translate('data.languageChangeBody', { + defaultMessage: 'Query language changed to {supportedLanguage}.', + values: { + supportedLanguage: + this.languageService.getLanguage(supportedLanguages[0])?.title || + supportedLanguages[0], + }, + }), + }); + } + + // Add to recent datasets this.datasetService.addRecentDataset(newQuery.dataset); } this.query$.next(newQuery); @@ -204,7 +234,6 @@ export class QueryStringManager { * If only language is provided, uses current dataset * If only dataset is provided, uses current or dataset's preferred language */ - // TODO: claude write jest test for this public getInitialQuery = (partialQuery?: Partial) => { if (!partialQuery) { return this.getInitialQueryByLanguage(this.query$.getValue().language); @@ -243,7 +272,6 @@ export class QueryStringManager { * Gets initial query for a language, preserving current dataset * Called by getInitialQuery when only language changes */ - // TODO: claude write jest test for this public getInitialQueryByLanguage = (languageId: string) => { const curQuery = this.query$.getValue(); const language = this.languageService.getLanguage(languageId); @@ -264,7 +292,6 @@ export class QueryStringManager { * Gets initial query for a dataset, using dataset's preferred language or current language * Called by getInitialQuery when only dataset changes */ - // TODO: claude write jest test for this public getInitialQueryByDataset = (newDataset: Dataset) => { const curQuery = this.query$.getValue(); // Use dataset's preferred language or fallback to current language @@ -320,7 +347,10 @@ export class QueryStringManager { }; } -const showWarning = (notifications: NotificationsSetup, { title, text }) => { +const showWarning = ( + notifications: NotificationsSetup, + { title, text }: { title: string; text: string } +) => { notifications.toasts.addWarning({ title, text, id: 'unsupported_language_selected' }); }; diff --git a/src/plugins/data/public/ui/query_editor/language_selector.test.tsx b/src/plugins/data/public/ui/query_editor/language_selector.test.tsx index ef2dda15f72c..57633a4b3d11 100644 --- a/src/plugins/data/public/ui/query_editor/language_selector.test.tsx +++ b/src/plugins/data/public/ui/query_editor/language_selector.test.tsx @@ -9,13 +9,10 @@ import { OpenSearchDashboardsContextProvider } from 'src/plugins/opensearch_dash import { coreMock } from '../../../../../core/public/mocks'; import { mountWithIntl } from 'test_utils/enzyme_helpers'; import { Query } from '../..'; -import { of } from 'rxjs'; const startMock = coreMock.createStart(); // Mock the query updates subject -// TODO: Codeium jest is complaining about reference area and this being out of scope can you fix this? - // Create a more complete mock that matches the service structure jest.mock('../../services', () => { const getQueryMock = jest.fn().mockReturnValue({