From f5852140c6b3621f912acc979424c5ac545ba8ed Mon Sep 17 00:00:00 2001 From: Vlad0n20 Date: Fri, 12 Jun 2026 16:05:09 +0200 Subject: [PATCH 1/2] fix(ENG-11334): preserve cedar_roperty_iri on api filter key collision --- .../global-search/global-search.state.spec.ts | 65 +++++++++++++++++++ .../global-search/global-search.state.ts | 14 +++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/app/shared/stores/global-search/global-search.state.spec.ts b/src/app/shared/stores/global-search/global-search.state.spec.ts index dd78a40d0..1ebb5115b 100644 --- a/src/app/shared/stores/global-search/global-search.state.spec.ts +++ b/src/app/shared/stores/global-search/global-search.state.spec.ts @@ -94,6 +94,16 @@ describe('GlobalSearchState', () => { expect(mockGetFilterOptions).not.toHaveBeenCalled(); }); + it('should skip the API call for a CEDAR filter found only in extraFilters (before first fetch)', () => { + const { store, mockGetFilterOptions } = setup(); + store.dispatch(new SetExtraFilters([CEDAR_FILTER])); + // Intentionally no FetchResources — state.filters is still empty + + store.dispatch(new LoadFilterOptions(CEDAR_FILTER.key)); + + expect(mockGetFilterOptions).not.toHaveBeenCalled(); + }); + it('should set isLoaded to true for a CEDAR filter when short-circuiting', () => { const { store } = setup(); @@ -264,4 +274,59 @@ describe('GlobalSearchState', () => { expect(params['cardSearchFilter[defaultKey][]']).toBe('default-value'); }); }); + + describe('updateResourcesState (CEDAR filter key collision)', () => { + const COLLIDING_API_FILTER: DiscoverableFilter = { + key: CEDAR_FILTER.key, + label: 'School Type (from API)', + operator: FilterOperatorOption.AnyOf, + resultCount: 5, + }; + + const MOCK_RESOURCES_WITH_COLLIDING_FILTER: ResourcesData = { + resources: [], + filters: [COLLIDING_API_FILTER], + count: 5, + self: '', + first: null, + next: null, + previous: null, + }; + + it('should preserve cedarPropertyIri when API returns a filter with the same key as an extra filter', () => { + const { store, mockGetResources } = setup(); + mockGetResources.mockReturnValue(of(MOCK_RESOURCES_WITH_COLLIDING_FILTER)); + + store.dispatch(new SetExtraFilters([CEDAR_FILTER])); + store.dispatch(new FetchResources()); + + const filters = store.selectSnapshot(GlobalSearchSelectors.getFilters); + const filter = filters.find((f) => f.key === CEDAR_FILTER.key); + expect(filter?.cedarPropertyIri).toBe(CEDAR_FILTER.cedarPropertyIri); + expect(filter?.options).toEqual(CEDAR_FILTER.options); + }); + + it('should use cardSearchText even when the API returns a filter with the same key', () => { + const { store, mockGetResources } = setup(); + mockGetResources.mockReturnValue(of(MOCK_RESOURCES_WITH_COLLIDING_FILTER)); + + store.dispatch(new SetExtraFilters([CEDAR_FILTER])); + store.dispatch(new FetchResources()); + mockGetResources.mockClear(); + mockGetResources.mockReturnValue(of(MOCK_RESOURCES_DATA)); + + store.dispatch( + new LoadFilterOptionsAndSetValues({ + [CEDAR_FILTER.key]: [{ label: 'High School', value: 'High School', cardSearchResultCount: null }], + }) + ); + store.dispatch(new FetchResources()); + + const params = mockGetResources.mock.calls[0][0]; + expect(params[`cardSearchText[osf:hasCedarRecord.cedar:${CEDAR_FILTER.cedarPropertyIri}][]`]).toEqual([ + '"High School"', + ]); + expect(params[`cardSearchFilter[${CEDAR_FILTER.key}][]`]).toBeUndefined(); + }); + }); }); diff --git a/src/app/shared/stores/global-search/global-search.state.ts b/src/app/shared/stores/global-search/global-search.state.ts index bc7bb2710..733a0f67e 100644 --- a/src/app/shared/stores/global-search/global-search.state.ts +++ b/src/app/shared/stores/global-search/global-search.state.ts @@ -63,7 +63,8 @@ export class GlobalSearchState { const state = ctx.getState(); const filterKey = action.filterKey; - const filter = state.filters.find((f) => f.key === filterKey); + const filter = + state.filters.find((f) => f.key === filterKey) ?? state.extraFilters.find((f) => f.key === filterKey); if (filter?.cedarPropertyIri) { ctx.patchState({ filters: state.filters.map((f) => (f.key === filterKey ? { ...f, isLoaded: true } : f)), @@ -291,14 +292,21 @@ export class GlobalSearchState { private updateResourcesState(ctx: StateContext, response: ResourcesData) { const { extraFilters } = ctx.getState(); const apiFilterKeys = new Set(response.filters.map((f) => f.key)); - const merged = [...response.filters, ...extraFilters.filter((f) => !apiFilterKeys.has(f.key))]; + + const merged = response.filters.map((apiFilter) => { + const cedarExtra = extraFilters.find((ef) => ef.key === apiFilter.key); + return cedarExtra?.cedarPropertyIri + ? { ...apiFilter, cedarPropertyIri: cedarExtra.cedarPropertyIri, options: cedarExtra.options } + : apiFilter; + }); + const extraFiltersNotInApi = extraFilters.filter((ef) => !apiFilterKeys.has(ef.key)); ctx.patchState({ resources: { data: response.resources, isLoading: false, error: null }, filterOptionsCache: {}, filterSearchCache: {}, filterPaginationCache: {}, - filters: merged, + filters: [...merged, ...extraFiltersNotInApi], resourcesCount: response.count, first: response.first, next: response.next, From 92b7e6065604b9209faea02a5533c76be19f9e43 Mon Sep 17 00:00:00 2001 From: Vlad0n20 Date: Mon, 15 Jun 2026 17:32:37 +0200 Subject: [PATCH 2/2] fix(ENG-11334): fix comments --- .../global-search/global-search.state.spec.ts | 10 ---------- .../stores/global-search/global-search.state.ts | 14 ++++++++++---- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/app/shared/stores/global-search/global-search.state.spec.ts b/src/app/shared/stores/global-search/global-search.state.spec.ts index 1ebb5115b..4efcf706d 100644 --- a/src/app/shared/stores/global-search/global-search.state.spec.ts +++ b/src/app/shared/stores/global-search/global-search.state.spec.ts @@ -94,16 +94,6 @@ describe('GlobalSearchState', () => { expect(mockGetFilterOptions).not.toHaveBeenCalled(); }); - it('should skip the API call for a CEDAR filter found only in extraFilters (before first fetch)', () => { - const { store, mockGetFilterOptions } = setup(); - store.dispatch(new SetExtraFilters([CEDAR_FILTER])); - // Intentionally no FetchResources — state.filters is still empty - - store.dispatch(new LoadFilterOptions(CEDAR_FILTER.key)); - - expect(mockGetFilterOptions).not.toHaveBeenCalled(); - }); - it('should set isLoaded to true for a CEDAR filter when short-circuiting', () => { const { store } = setup(); diff --git a/src/app/shared/stores/global-search/global-search.state.ts b/src/app/shared/stores/global-search/global-search.state.ts index 733a0f67e..f998b620b 100644 --- a/src/app/shared/stores/global-search/global-search.state.ts +++ b/src/app/shared/stores/global-search/global-search.state.ts @@ -293,11 +293,17 @@ export class GlobalSearchState { const { extraFilters } = ctx.getState(); const apiFilterKeys = new Set(response.filters.map((f) => f.key)); + const extraByKey = new Map(extraFilters.map((ef) => [ef.key, ef])); + const merged = response.filters.map((apiFilter) => { - const cedarExtra = extraFilters.find((ef) => ef.key === apiFilter.key); - return cedarExtra?.cedarPropertyIri - ? { ...apiFilter, cedarPropertyIri: cedarExtra.cedarPropertyIri, options: cedarExtra.options } - : apiFilter; + const cedarExtra = extraByKey.get(apiFilter.key); + if (!cedarExtra) return apiFilter; + + return { + ...apiFilter, + cedarPropertyIri: cedarExtra.cedarPropertyIri, + ...(cedarExtra.options !== undefined && { options: cedarExtra.options }), + }; }); const extraFiltersNotInApi = extraFilters.filter((ef) => !apiFilterKeys.has(ef.key));