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>
(cherry picked from commit a70a7b8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 0cd81f0 commit 433496e
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 };

Check warning on line 157 in src/plugins/data/public/query/query_string/query_string_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/query_string_manager.ts#L157

Added line #L157 was not covered by tests
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

Check warning on line 162 in src/plugins/data/public/query/query_string/query_string_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/query_string_manager.ts#L162

Added line #L162 was not covered by tests
.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({

Check warning on line 169 in src/plugins/data/public/query/query_string/query_string_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/query_string_manager.ts#L169

Added line #L169 was not covered by tests
language: supportedLanguages[0],
dataset: newQuery.dataset,
});

// Show warning about language change
showWarning(this.notifications, {

Check warning on line 175 in src/plugins/data/public/query/query_string/query_string_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/query/query_string/query_string_manager.ts#L175

Added line #L175 was not covered by tests
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 433496e

Please sign in to comment.