Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/10913.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Skip loading filter if navigating to a saved search without params ([#10913](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10913))
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,108 @@ describe('connect_storage_to_query_state', () => {
const updatedStorage = osdUrlStateStorage.get('_q');
expect(previousStorage).not.toStrictEqual(updatedStorage);
});

test('when skipAppFiltersFromMemory is true, state initializes with empty filters even if filterManager has app filters', () => {
// Set some app filters in filterManager
filterManager.setFilters([aF1, aF2]);
expect(filterManager.getAppFilters().length).toBe(2);

expect(osdUrlStateStorage.get('_q')).toBeNull();

// Connect with skipAppFiltersFromMemory enabled
connectStorageToQueryState(queryServiceStart, osdUrlStateStorage, {
filters: FilterStateStore.APP_STATE,
query: true,
skipAppFiltersFromMemory: true,
});

// State should have empty filters, not filters from filterManager
expect(osdUrlStateStorage.get('_q')).toEqual({
query: queryString.getDefaultQuery(),
filters: [],
});
});

test('when skipAppFiltersFromMemory is true, app filters are cleared from filterManager', () => {
// Set some app filters in filterManager
filterManager.setFilters([aF1, aF2]);
expect(filterManager.getAppFilters().length).toBe(2);

// Connect with skipAppFiltersFromMemory enabled
connectStorageToQueryState(queryServiceStart, osdUrlStateStorage, {
filters: FilterStateStore.APP_STATE,
query: true,
skipAppFiltersFromMemory: true,
});

// App filters should be cleared
expect(filterManager.getAppFilters()).toEqual([]);
});

test('when skipAppFiltersFromMemory is false, state initializes with filters from filterManager', () => {
// Set some app filters in filterManager
filterManager.setFilters([aF1, aF2]);

expect(osdUrlStateStorage.get('_q')).toBeNull();

// Connect with skipAppFiltersFromMemory disabled (default behavior)
connectStorageToQueryState(queryServiceStart, osdUrlStateStorage, {
filters: FilterStateStore.APP_STATE,
query: true,
skipAppFiltersFromMemory: false,
});

// State should have filters from filterManager
expect(osdUrlStateStorage.get('_q')).toEqual({
query: queryString.getDefaultQuery(),
filters: [aF1, aF2],
});
});

test('when skipAppFiltersFromMemory is true but URL has filters, URL filters take precedence', () => {
// Set some app filters in filterManager
filterManager.setFilters([aF1, aF2]);

// Set different filters in URL
const urlFilters = [aF1];
osdUrlStateStorage.set(
'_q',
{
filters: urlFilters,
query: q1,
},
{
replace: true,
}
);

// Connect with skipAppFiltersFromMemory enabled
connectStorageToQueryState(queryServiceStart, osdUrlStateStorage, {
filters: FilterStateStore.APP_STATE,
query: true,
skipAppFiltersFromMemory: true,
});

// URL filters should be used, not empty array or filterManager filters
expect(filterManager.getFilters().length).toBe(1);
expect(queryString.getQuery()).toStrictEqual(q1);
});

test('when skipAppFiltersFromMemory is true with GLOBAL_STATE filters, app filters should not be cleared', () => {
// Set both global and app filters
filterManager.setFilters([gF1, aF1, aF2]);
expect(filterManager.getAppFilters().length).toBe(2);

// Connect with skipAppFiltersFromMemory enabled but for GLOBAL_STATE
connectStorageToQueryState(queryServiceStart, osdUrlStateStorage, {
filters: FilterStateStore.GLOBAL_STATE,
query: true,
skipAppFiltersFromMemory: true,
});

// App filters should NOT be cleared because we're syncing GLOBAL_STATE
expect(filterManager.getAppFilters().length).toBe(2);
});
});

describe('connect_to_global_state', () => {
Expand Down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe unrelated to this PR, it seems in connectStorageToQueryState, we're doing two-way state syncing memory state <-> url sate, do you know the intention?

Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ export interface ISyncConfig {
filters: FilterStateStore;
query: boolean;
dataset?: boolean;
/**
* When true, skips using existing filters from filterManager when initializing state from URL.
* This is useful when navigating to a saved search/explore to prevent filter persistence.
*/
skipAppFiltersFromMemory?: boolean;
}

/**
Expand Down Expand Up @@ -74,7 +79,8 @@ export const connectStorageToQueryState = (

const initialStateFromURL: QueryState = osdUrlStateStorage.get('_q') ?? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe unrelated to this PR: the name initialStateFromURL is a bit misleading, because it's not just url state, it also reads in-memory state

query: queryString.getDefaultQuery(),
filters: filterManager.getAppFilters(),
// If caller specifies to skip filters from memory, use empty array
filters: syncConfig.skipAppFiltersFromMemory ? [] : filterManager.getAppFilters(),
};

if (!osdUrlStateStorage.get('_q')) {
Expand All @@ -86,6 +92,11 @@ export const connectStorageToQueryState = (
queryString.clearQuery();
}

// Clear app filters if caller requested to skip filters from memory
if (syncConfig.skipAppFiltersFromMemory && syncConfig.filters === FilterStateStore.APP_STATE) {
filterManager.setAppFilters([]);
}

if (syncConfig.query && !_.isEqual(initialStateFromURL.query, queryString.getQuery())) {
if (initialStateFromURL.query) {
queryString.setQuery(_.cloneDeep(initialStateFromURL.query));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,16 @@ export const TopNav = ({ opts, showSaveQuery, isEnhancementsEnabled }: TopNavPro
: [];

const syncConfig = useMemo(() => {
// Check if navigating to a saved search without params - skip filter persistence
const currentHash = window.location.hash.split('?')[0];
const isSavedSearchUrl = currentHash.includes('#/view/');
const hasUrlParams = window.location.hash.includes('?');
const shouldSkipFilters = isSavedSearchUrl && !hasUrlParams;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't get why it should check hasUrlParams here, could you elaborate please?


return {
filters: opensearchFilters.FilterStateStore.APP_STATE,
query: true,
skipAppFiltersFromMemory: shouldSkipFilters,
};
}, []);

Expand Down
Loading