Skip to content

Commit

Permalink
fix(commerce): properly memoize suggestions controllers (#4184)
Browse files Browse the repository at this point in the history
Field suggestions controllers weren't being updated because we were
memoizing their generation only on the list of facets, which did not
change when field suggestions values changed. Now, we include the facet
set (which includes facet values), to the function on which to memoize.
This ensures the field suggestions controllers are update with the right
values.

[CAPI-1102]

[CAPI-1102]:
https://coveord.atlassian.net/browse/CAPI-1102?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
Spuffynism authored Jul 19, 2024
1 parent aafe3fe commit b5d226b
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import {FacetSearchType} from '../../../api/commerce/facet-search/facet-search-request';
import {FieldSuggestionsFacet} from '../../../api/commerce/search/query-suggest/query-suggest-response';
import {commerceFacetSetReducer as commerceFacetSet} from '../../../features/commerce/facets/facet-set/facet-set-slice';
import {fieldSuggestionsOrderReducer as fieldSuggestionsOrder} from '../../../features/commerce/facets/field-suggestions-order/field-suggestions-order-slice';
import {categoryFacetSearchSetReducer as categoryFacetSearchSet} from '../../../features/facets/facet-search-set/category/category-facet-search-set-slice';
import {specificFacetSearchSetReducer as facetSearchSet} from '../../../features/facets/facet-search-set/specific/specific-facet-search-set-slice';
import {CommerceAppState} from '../../../state/commerce-app-state';
import {buildMockCategoryFacetSearch} from '../../../test/mock-category-facet-search';
import {buildMockCommerceFacetRequest} from '../../../test/mock-commerce-facet-request';
Expand Down Expand Up @@ -68,6 +71,9 @@ describe('fieldSuggestionsGenerator', () => {
it('adds correct reducers to engine', () => {
expect(engine.addReducers).toHaveBeenCalledWith({
fieldSuggestionsOrder,
commerceFacetSet,
facetSearchSet,
categoryFacetSearchSet,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ import {
CommerceEngineState,
} from '../../../app/commerce-engine/commerce-engine';
import {stateKey} from '../../../app/state-key';
import {commerceFacetSetReducer as commerceFacetSet} from '../../../features/commerce/facets/facet-set/facet-set-slice';
import {fieldSuggestionsOrderReducer as fieldSuggestionsOrder} from '../../../features/commerce/facets/field-suggestions-order/field-suggestions-order-slice';
import {FieldSuggestionsFacet} from '../../../features/commerce/facets/field-suggestions-order/field-suggestions-order-state';
import {executeSearch} from '../../../features/commerce/search/search-actions';
import {categoryFacetSearchSetReducer as categoryFacetSearchSet} from '../../../features/facets/facet-search-set/category/category-facet-search-set-slice';
import {specificFacetSearchSetReducer as facetSearchSet} from '../../../features/facets/facet-search-set/specific/specific-facet-search-set-slice';
import {loadReducerError} from '../../../utils/errors';
import {
buildController,
Expand Down Expand Up @@ -65,7 +68,10 @@ export function buildFieldSuggestionsGenerator(

const createFieldSuggestionsControllers = createSelector(
(state: CommerceEngineState) => state.fieldSuggestionsOrder!,
(facetOrder) =>
(state: CommerceEngineState) => state.commerceFacetSet,
(state: CommerceEngineState) => state.facetSearchSet,
(state: CommerceEngineState) => state.categoryFacetSearchSet,
(facetOrder, _commerceFacetSet, _facetSearchSet, _categoryFacetSearchSet) =>
facetOrder.map(({type, facetId}) => {
switch (type) {
case 'hierarchical':
Expand Down Expand Up @@ -98,6 +104,9 @@ function loadFieldSuggestionsGeneratorReducers(
): engine is CommerceEngine {
engine.addReducers({
fieldSuggestionsOrder,
commerceFacetSet,
facetSearchSet,
categoryFacetSearchSet,
});
return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ import {convertToNumericRangeRequests} from '../../../facets/range-facets/numeri
import {setContext, setView} from '../../context/context-actions';
import {restoreProductListingParameters} from '../../product-listing-parameters/product-listing-parameters-actions';
import {fetchProductListing} from '../../product-listing/product-listing-actions';
import {
fetchQuerySuggestions,
FetchQuerySuggestionsThunkReturn,
} from '../../query-suggest/query-suggest-actions';
import {restoreSearchParameters} from '../../search-parameters/search-parameters-actions';
import {executeSearch} from '../../search/search-actions';
import {
Expand Down Expand Up @@ -639,6 +643,65 @@ describe('commerceFacetSetReducer', () => {
}
);

describe('#fetchQuerySuggestions.fulfilled', () => {
const payloadWithoutFieldSuggestionsFacets = {
query: '',
id: '',
completions: [],
responseId: '',
};
it('does not change the state when there are no field suggestion facets in the payload', () => {
const finalState = commerceFacetSetReducer(
state,
fetchQuerySuggestions.fulfilled(
payloadWithoutFieldSuggestionsFacets as unknown as FetchQuerySuggestionsThunkReturn,
'',
{id: ''}
)
);
expect(finalState).toEqual(state);
});

it('initializes field suggestion facets when they are in the payload', () => {
const finalState = commerceFacetSetReducer(
state,
fetchQuerySuggestions.fulfilled(
{
...payloadWithoutFieldSuggestionsFacets,
fieldSuggestionsFacets: [
{
facetId: 'regular_field',
field: 'regular_field',
displayName: 'Regular Field',
type: 'regular',
},
{
facetId: 'hierarchical_field',
field: 'hierarchical_field',
displayName: 'Hierarchical Field',
type: 'hierarchical',
},
],
},
'',
{id: ''}
)
);
expect(finalState).toEqual({
regular_field: {
request: {
initialNumberOfValues: 10,
},
},
hierarchical_field: {
request: {
initialNumberOfValues: 10,
},
},
});
});
});

describe('for regular facets', () => {
describe.each([
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {convertToNumericRangeRequests} from '../../../facets/range-facets/numeri
import {setContext, setView} from '../../context/context-actions';
import {restoreProductListingParameters} from '../../product-listing-parameters/product-listing-parameters-actions';
import {fetchProductListing} from '../../product-listing/product-listing-actions';
import {fetchQuerySuggestions} from '../../query-suggest/query-suggest-actions';
import {restoreSearchParameters} from '../../search-parameters/search-parameters-actions';
import {executeSearch} from '../../search/search-actions';
import '../category-facet/category-facet-actions';
Expand Down Expand Up @@ -79,10 +80,18 @@ export const commerceFacetSetReducer = createReducer(
builder
.addCase(fetchProductListing.fulfilled, handleQueryFulfilled)
.addCase(executeSearch.fulfilled, handleQueryFulfilled)
.addCase(
executeCommerceFieldSuggest.fulfilled,
handleFieldSuggestionsFulfilled
.addCase(executeCommerceFieldSuggest.fulfilled, (state, action) =>
handleFieldSuggestionsFulfilled(state, action.payload.facetId)
)
.addCase(fetchQuerySuggestions.fulfilled, (state, action) => {
if (!action.payload.fieldSuggestionsFacets) {
return;
}

for (const {facetId} of action.payload.fieldSuggestionsFacets) {
handleFieldSuggestionsFulfilled(state, facetId);
}
})
.addCase(toggleSelectFacetValue, (state, action) => {
const {facetId, selection} = action.payload;
const facetRequest = state[facetId]?.request;
Expand Down Expand Up @@ -469,10 +478,8 @@ function handleQueryFulfilled(

function handleFieldSuggestionsFulfilled(
state: WritableDraft<CommerceFacetSetState>,
action: ReturnType<typeof executeCommerceFieldSuggest.fulfilled>
facetId: string
) {
const facetId = action.payload.facetId;

let facetRequest = state[facetId]?.request;
if (!facetRequest) {
state[facetId] = {request: {} as AnyFacetRequest};
Expand Down
75 changes: 62 additions & 13 deletions packages/headless/src/integration-tests/commerce.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@ import {
CommerceEngineConfiguration,
} from '../app/commerce-engine/commerce-engine';
import {getSampleCommerceEngineConfiguration} from '../app/commerce-engine/commerce-engine-configuration';
import {CategoryFieldSuggestions} from '../controllers/commerce/field-suggestions/headless-category-field-suggestions';
import {buildFieldSuggestionsGenerator} from '../controllers/commerce/field-suggestions/headless-field-suggestions-generator';
import {ProductListing} from '../controllers/commerce/product-listing/headless-product-listing';
import {buildProductListing} from '../controllers/commerce/product-listing/headless-product-listing';
import {buildRecommendations} from '../controllers/commerce/recommendations/headless-recommendations';
import {buildSearchBox} from '../controllers/commerce/search-box/headless-search-box';
import {
buildSearchBox,
SearchBox,
} from '../controllers/commerce/search-box/headless-search-box';
import {buildSearch} from '../controllers/commerce/search/headless-search';
import {waitForNextStateChange} from '../test/functional-test-utils';

Expand All @@ -16,15 +21,15 @@ describe.skip('commerce', () => {
let engine: CommerceEngine;

beforeAll(async () => {
Object.defineProperty(global, 'document', {
value: {referrer: 'referrer'},
configurable: true,
});

configuration = getSampleCommerceEngineConfiguration();
engine = buildCommerceEngine({
configuration,
loggerOptions: {level: 'silent'},
configuration: {
...configuration,
analytics: {
...configuration.analytics,
enabled: false,
},
},
});
});

Expand Down Expand Up @@ -79,7 +84,7 @@ describe.skip('commerce', () => {
const controllers = facetGenerator.facets;
const facetController = controllers[0];

await waitForNextStateChange(productListing, {
await waitForNextStateChange(facetController, {
action: () => {
switch (facetController.type) {
case 'numericalRange':
Expand Down Expand Up @@ -121,10 +126,7 @@ describe.skip('commerce', () => {

it('provides suggestions', async () => {
const box = buildSearchBox(engine);
await waitForNextStateChange(box, {
action: () => box.updateText('l'),
expectedSubscriberCalls: 3,
});
await search(box, 'l');

expect(box.state.suggestions).not.toEqual([]);
});
Expand All @@ -142,4 +144,51 @@ describe.skip('commerce', () => {

expect(recommendations.state.products).not.toEqual([]);
});

it('provides field suggestions', async () => {
const box = buildSearchBox(engine);
const generator = buildFieldSuggestionsGenerator(engine);

await search(box, 'can');

expect(generator.fieldSuggestions).toHaveLength(3);

for (const controller of generator.fieldSuggestions) {
await waitForNextStateChange(engine, {
action: () => controller.updateText('can'),
expectedSubscriberCalls: 3,
});
}

let controller = generator.fieldSuggestions.find(
(controller) => controller.state.facetId === 'ec_category'
)!;

expect(controller.state.values).not.toEqual([]);

await search(box, 'acc');

for (const controller of generator.fieldSuggestions) {
await waitForNextStateChange(engine, {
action: () => controller.updateText('acc'),
expectedSubscriberCalls: 3,
});
}

controller = generator.fieldSuggestions.find(
(controller) => controller.state.facetId === 'ec_category'
)! as CategoryFieldSuggestions;

expect(controller.state.values).not.toEqual([]);
for (const value of controller.state.values) {
expect(value.displayValue).toContain('Acc');
}
});

async function search(box: SearchBox, query: string) {
await waitForNextStateChange(box, {
action: () => box.updateText(query),
expectedSubscriberCalls: 3,
});
}
});

0 comments on commit b5d226b

Please sign in to comment.