Skip to content

Commit

Permalink
[data] updates query and lang if language is not supported by query d…
Browse files Browse the repository at this point in the history
…ata (#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 <[email protected]>

* Changeset file for PR #8749 created/updated

* update string in notification

Signed-off-by: Kawika Avilla <[email protected]>

---------

Signed-off-by: Kawika Avilla <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
kavilla and opensearch-changeset-bot[bot] authored Oct 30, 2024
1 parent e1a1b8f commit a70a7b8
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 10 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/8749.yml
Original file line number Diff line number Diff line change
@@ -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))
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
44 changes: 37 additions & 7 deletions src/plugins/data/public/query/query_string/query_string_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -154,10 +154,40 @@ export class QueryStringManager {
*/
public setQuery = (query: Partial<Query>) => {
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);
Expand Down Expand Up @@ -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<Query>) => {
if (!partialQuery) {
return this.getInitialQueryByLanguage(this.query$.getValue().language);
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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' });
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down

0 comments on commit a70a7b8

Please sign in to comment.