Skip to content

Commit

Permalink
Use currently selected data source when no source attached to saved q…
Browse files Browse the repository at this point in the history
…uery (#8883)

Opening a saved query that has no dataset stored with it, resets the currently selected dataset in the picker which breaks the query experience since the user will need to reselect the dataset which will then reset the query.

* use currently selected data source when no source attached to saved query

Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com>

* Changeset file for PR #8883 created/updated

* refactored fix

Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com>

* revert license change

Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com>

---------

Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
amsiglan and opensearch-changeset-bot[bot] authored Nov 19, 2024
1 parent 608911e commit a8d383b
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 56 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/8883.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Retain currently selected dataset when opening saved query without dataset info ([#8883](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8883))
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import React, { useCallback, useEffect, useRef, useState } from 'react';
import { i18n } from '@osd/i18n';
import { SavedQuery, SavedQueryService } from '../../query';
import { SavedQueryCard } from './saved_query_card';
import { Query } from '../../../common';
import { getQueryService } from '../../services';

export interface OpenSavedQueryFlyoutProps {
Expand Down Expand Up @@ -306,19 +305,16 @@ export function OpenSavedQueryFlyout({
fill
onClick={() => {
if (selectedQuery) {
if (
// Template queries are not associated with data sources. Apply data source from current query
selectedQuery.attributes.isTemplate
) {
const updatedQuery: Query = {
...queryStringManager?.getQuery(),
query: selectedQuery.attributes.query.query,
language: selectedQuery.attributes.query.language,
};
queryStringManager.setQuery(updatedQuery);
} else {
onQueryOpen(selectedQuery);
}
onQueryOpen({
...selectedQuery,
attributes: {
...selectedQuery.attributes,
query: {
...selectedQuery.attributes.query,
dataset: queryStringManager.getQuery().dataset,
},
},
});
onClose();
}
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export function SaveQueryFlyout({
savedQueryService={savedQueryService}
showFilterOption={showFilterOption}
showTimeFilterOption={showTimeFilterOption}
showDataSourceOption={true}
setSaveAsNew={(shouldSaveAsNew) => setSaveAsNew(shouldSaveAsNew)}
savedQuery={saveAsNew ? undefined : savedQuery}
saveAsNew={saveAsNew}
Expand Down
30 changes: 1 addition & 29 deletions src/plugins/data/public/ui/saved_query_form/helpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ interface Props {
formUiType: 'Modal' | 'Flyout';
showFilterOption?: boolean;
showTimeFilterOption?: boolean;
showDataSourceOption?: boolean;
saveAsNew?: boolean;
setSaveAsNew?: (shouldSaveAsNew: boolean) => void;
cannotBeOverwritten?: boolean;
Expand All @@ -70,7 +69,6 @@ export function useSaveQueryFormContent({
onClose,
showFilterOption = true,
showTimeFilterOption = true,
showDataSourceOption = false,
formUiType,
saveAsNew,
setSaveAsNew,
Expand All @@ -81,7 +79,6 @@ export function useSaveQueryFormContent({
const [description, setDescription] = useState('');
const [savedQueries, setSavedQueries] = useState<SavedQuery[]>([]);
const [shouldIncludeFilters, setShouldIncludeFilters] = useState(true);
const [shouldIncludeDataSource, setShouldIncludeDataSource] = useState(true);
// Defaults to false because saved queries are meant to be as portable as possible and loading
// a saved query with a time filter will override whatever the current value of the global timepicker
// is. We expect this option to be used rarely and only when the user knows they want this behavior.
Expand All @@ -96,7 +93,6 @@ export function useSaveQueryFormContent({
setDescription(savedQuery?.description || '');
setShouldIncludeFilters(savedQuery ? !!savedQuery.filters : true);
setIncludeTimefilter(!!savedQuery?.timefilter);
setShouldIncludeDataSource(savedQuery ? !!savedQuery.query.dataset : true);
setFormErrors([]);
}, [savedQuery]);

Expand Down Expand Up @@ -147,18 +143,9 @@ export function useSaveQueryFormContent({
description,
shouldIncludeFilters,
shouldIncludeTimeFilter,
shouldIncludeDataSource,
});
}
}, [
validate,
onSave,
title,
description,
shouldIncludeFilters,
shouldIncludeTimeFilter,
shouldIncludeDataSource,
]);
}, [validate, onSave, title, description, shouldIncludeFilters, shouldIncludeTimeFilter]);

const onInputChange = useCallback((event) => {
setEnabledSaveButton(Boolean(event.target.value));
Expand Down Expand Up @@ -229,21 +216,6 @@ export function useSaveQueryFormContent({
data-test-subj="saveQueryFormDescription"
/>
</EuiCompressedFormRow>
{showDataSourceOption && (
<EuiCompressedFormRow>
<EuiCompressedSwitch
name="shouldIncludeDataSource"
label={i18n.translate('data.search.searchBar.savedQueryIncludeDatasourceLabelText', {
defaultMessage: 'Include data source',
})}
checked={shouldIncludeDataSource}
onChange={() => {
setShouldIncludeDataSource(!shouldIncludeDataSource);
}}
data-test-subj="saveQueryFormIncludeDataSourceOption"
/>
</EuiCompressedFormRow>
)}
{showFilterOption && (
<EuiCompressedFormRow>
<EuiCompressedSwitch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ interface Props {
setSaveAsNew?: (shouldSaveAsNew: boolean) => void;
showFilterOption?: boolean;
showTimeFilterOption?: boolean;
showDataSourceOption?: boolean;
saveAsNew?: boolean;
cannotBeOverwritten?: boolean;
}
Expand All @@ -63,7 +62,6 @@ export interface SavedQueryMeta {
description: string;
shouldIncludeFilters: boolean;
shouldIncludeTimeFilter: boolean;
shouldIncludeDataSource: boolean;
}

export function SaveQueryForm({
Expand All @@ -74,7 +72,6 @@ export function SaveQueryForm({
onClose,
showFilterOption = true,
showTimeFilterOption = true,
showDataSourceOption = false,
saveAsNew,
setSaveAsNew,
cannotBeOverwritten,
Expand All @@ -87,7 +84,6 @@ export function SaveQueryForm({
onClose,
showFilterOption,
showTimeFilterOption,
showDataSourceOption,
saveAsNew,
setSaveAsNew,
cannotBeOverwritten,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ describe('populateStateFromSavedQuery', () => {
query: {
query: 'test',
language: 'kuery',
dataset: {
id: 'saved-query-dataset',
title: 'saved-query-dataset',
type: 'INDEX',
},
},
},
};
Expand All @@ -57,12 +62,15 @@ describe('populateStateFromSavedQuery', () => {
dataMock.query.filterManager.getGlobalFilters = jest.fn().mockReturnValue([]);
});

it('should set query', async () => {
it('should set query with current dataset', async () => {
const savedQuery: SavedQuery = {
...baseSavedQuery,
};
populateStateFromSavedQuery(dataMock.query, savedQuery);
expect(dataMock.query.queryString.setQuery).toHaveBeenCalled();
expect(dataMock.query.queryString.setQuery).toHaveBeenCalledWith({
...savedQuery.attributes.query,
dataset: dataMock.query.queryString.getQuery().dataset,
});
});

it('should set filters', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ export const populateStateFromSavedQuery = (queryService: QueryStart, savedQuery
}

// query string
queryString.setQuery(savedQuery.attributes.query);
queryString.setQuery({
...savedQuery.attributes.query,
// We should keep the currently selected dataset intact
dataset: queryString.getQuery().dataset,
});

// filters
const savedQueryFilters = savedQuery.attributes.filters || [];
Expand Down
6 changes: 1 addition & 5 deletions src/plugins/data/public/ui/search_bar/search_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import { QueryEditorTopRow } from '../query_editor';
import QueryBarTopRow from '../query_string_input/query_bar_top_row';
import { SavedQueryMeta, SaveQueryForm } from '../saved_query_form';
import { FilterOptions } from '../filter_bar/filter_options';
import { getUseNewSavedQueriesUI } from '../../services';

interface SearchBarInjectedDeps {
opensearchDashboards: OpenSearchDashboardsReactContextValue<IDataPluginServices>;
Expand Down Expand Up @@ -285,11 +284,8 @@ class SearchBarUI extends Component<SearchBarProps, State> {

public onSave = async (savedQueryMeta: SavedQueryMeta, saveAsNew = false) => {
if (!this.state.query) return;

const query = cloneDeep(this.state.query);
if (getUseNewSavedQueriesUI() && !savedQueryMeta.shouldIncludeDataSource) {
delete query.dataset;
}
delete query.dataset;

const savedQueryAttributes: SavedQueryAttributes = {
title: savedQueryMeta.title,
Expand Down

0 comments on commit a8d383b

Please sign in to comment.