Skip to content

Commit

Permalink
Fix Filter Manager Bugs in Saved Search
Browse files Browse the repository at this point in the history
Signed-off-by: Suchit Sahoo <[email protected]>
  • Loading branch information
LDrago27 authored and ashwin-pc committed Oct 19, 2024
1 parent 454a507 commit 27f7856
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const getLegacyTopNavLinks = (
defaultMessage: 'New Search',
}),
run() {
query.filterManager.setFilters([]); // resetting the filters while we are loading a new search

Check warning on line 57 in src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx#L57

Added line #L57 was not covered by tests
core.application.navigateToApp('discover', {
path: '#/',
});
Expand Down Expand Up @@ -306,6 +307,7 @@ export const getTopNavLinks = (
defaultMessage: 'New',
}),
run() {
query.filterManager.setFilters([]); // resetting the filters while we are loading a new search

Check warning on line 310 in src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx#L310

Added line #L310 was not covered by tests
core.application.navigateToApp('discover', {
path: '#/',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export function OpenSearchPanel({ onClose, makeUrl }: Props) {
},
]}
onChoose={(id) => {
data.query.filterManager.setFilters([]); // resetting the filters

Check warning on line 95 in src/plugins/discover/public/application/components/top_nav/open_search_panel.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/components/top_nav/open_search_panel.tsx#L95

Added line #L95 was not covered by tests
application.navigateToApp('discover', { path: `#/view/${id}` });
onClose();
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,13 @@ export const getPreloadedState = async ({
const indexPatternId = savedSearchInstance.searchSource.getField('index')?.id;

// If the query enhancement is enabled don't add the indexpattern id to the root since they will be migrated into the _q state
if (config.get(QUERY_ENHANCEMENT_ENABLED_SETTING)) {
preloadedState.root = {
metadata: {
view: PLUGIN_ID,
},
};
} else {
preloadedState.root = {
metadata: {
indexPattern: indexPatternId,
view: PLUGIN_ID,
},
};
}
preloadedState.root = {
metadata: {
...(indexPatternId &&
!config.get(QUERY_ENHANCEMENT_ENABLED_SETTING) && { indexPattern: indexPatternId }),
view: PLUGIN_ID,
},
};

savedSearchInstance.destroy(); // this instance is no longer needed, will create another one later
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { cloneDeep } from 'lodash';
import { useLocation } from 'react-router-dom';
import { RequestAdapter } from '../../../../../inspector/public';
import { DiscoverViewServices } from '../../../build_services';
import { search } from '../../../../../data/public';
import { QueryState, search } from '../../../../../data/public';
import { validateTimeRange } from '../../helpers/validate_time_range';
import { updateSearchSource } from './update_search_source';
import { useIndexPattern } from './use_index_pattern';
Expand Down Expand Up @@ -352,19 +352,23 @@ export const useSearch = (services: DiscoverViewServices) => {
useEffect(() => {
(async () => {
const savedSearchInstance = await getSavedSearchById(savedSearchId);
setSavedSearch(savedSearchInstance);

// if saved search does not exist, do not atempt to sync filters and query from savedObject
if (!savedSearchInstance) {
// if saved search does not exist, or the URL has filter tyhen don't sync the saved search state with that
if (!savedSearchInstance || !savedSearchId) {
return;
}

setSavedSearch(savedSearchInstance);

// sync initial app filters from savedObject to filterManager
const filters = cloneDeep(savedSearchInstance.searchSource.getOwnField('filter'));
// Sync Query from the saved search
const query =
savedSearchInstance.searchSource.getField('query') ||
data.query.queryString.getDefaultQuery();

data.query.queryString.setQuery(query);

Check warning on line 367 in src/plugins/discover/public/application/view_components/utils/use_search.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/view_components/utils/use_search.ts#L367

Added line #L367 was not covered by tests

// Sync Filters from the saved search
const filters = cloneDeep(savedSearchInstance.searchSource.getOwnField('filter'));

Check warning on line 370 in src/plugins/discover/public/application/view_components/utils/use_search.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/view_components/utils/use_search.ts#L370

Added line #L370 was not covered by tests

const actualFilters = [];

if (filters !== undefined) {
Expand All @@ -374,8 +378,11 @@ export const useSearch = (services: DiscoverViewServices) => {
}
}

filterManager.setAppFilters(actualFilters);
data.query.queryString.setQuery(query);
// Filters in URL are higher priority than the filters in saved search
const urlFilters = (osdUrlStateStorage.get('_q') as QueryState)?.filters ?? [];
if (!urlFilters || urlFilters.length === 0) {
filterManager.setAppFilters(actualFilters);

Check warning on line 384 in src/plugins/discover/public/application/view_components/utils/use_search.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/view_components/utils/use_search.ts#L384

Added line #L384 was not covered by tests
}

if (savedSearchInstance?.id) {
chrome.recentlyAccessed.add(
Expand Down

0 comments on commit 27f7856

Please sign in to comment.