Skip to content

Commit

Permalink
fix(commerce): expose child product directly on promoteChildToParent (
Browse files Browse the repository at this point in the history
#4128)

Instead of mutating the received products from the API, I propose we
dynamically find the corresponding parent and children in the reducer.
This _seems_ less error-prone to me, because we don't need to mutate
products in every place we fulfill a request containing `products`, at
the cost of being less performant, as we iterate on every product every
time a children is promoted. Thoughts?

An other option to consider would be to mutate the products directly in
the commerce api client.

https://coveord.atlassian.net/browse/KIT-3221
  • Loading branch information
Spuffynism authored Jun 27, 2024
1 parent 64cda10 commit d333812
Show file tree
Hide file tree
Showing 27 changed files with 172 additions and 198 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
NumericFacetValue,
DateFacetValue,
SortCriterion,
ChildProduct,
} from '@coveo/headless/commerce';
import {DEFAULT_MOBILE_BREAKPOINT} from '../../../utils/replace-breakpoint';
import {
Expand Down Expand Up @@ -32,7 +33,7 @@ export interface AtomicStoreData extends AtomicCommonStoreData {
sortOptions: SortDropdownOption[];
mobileBreakpoint: string;
currentQuickviewPosition: number;
activeProductChild: {parentPermanentId: string; childPermanentId: string};
activeProductChild: ChildProduct | undefined;
}

export interface AtomicCommerceStore
Expand Down Expand Up @@ -63,7 +64,7 @@ export function createAtomicCommerceStore(
mobileBreakpoint: DEFAULT_MOBILE_BREAKPOINT,
fieldsToInclude: [],
currentQuickviewPosition: -1,
activeProductChild: {parentPermanentId: '', childPermanentId: ''},
activeProductChild: undefined,
});

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,12 @@ export class AtomicCommerceProductList
@Listen('atomic/selectChildProduct')
public onSelectChildProduct(event: CustomEvent<SelectChildProductEventArgs>) {
event.stopPropagation();
const {parentPermanentId, childPermanentId} = event.detail;
const child = event.detail.child;

if (this.bindings.interfaceElement.type === 'product-listing') {
this.productListing.promoteChildToParent(
childPermanentId,
parentPermanentId
);
this.productListing.promoteChildToParent(child);
} else if (this.bindings.interfaceElement.type === 'search') {
this.search.promoteChildToParent(childPermanentId, parentPermanentId);
this.search.promoteChildToParent(child);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {ChildProduct} from '@coveo/headless/commerce';
import {DEFAULT_MOBILE_BREAKPOINT} from '../../../utils/replace-breakpoint';
import {
AtomicCommonStore,
Expand All @@ -9,7 +10,7 @@ import {makeDesktopQuery} from '../atomic-commerce-layout/commerce-layout';
export interface AtomicStoreData extends AtomicCommonStoreData {
mobileBreakpoint: string;
currentQuickviewPosition: number;
activeProductChild: {parentPermanentId: string; childPermanentId: string};
activeProductChild: ChildProduct | undefined;
}

export interface AtomicCommerceRecommendationStore
Expand All @@ -29,7 +30,7 @@ export function createAtomicCommerceRecommendationStore(): AtomicCommerceRecomme
mobileBreakpoint: DEFAULT_MOBILE_BREAKPOINT,
fieldsToInclude: [],
currentQuickviewPosition: -1,
activeProductChild: {parentPermanentId: '', childPermanentId: ''},
activeProductChild: undefined,
});

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,8 @@ export class AtomicCommerceRecommendationList
@Listen('atomic/selectChildProduct')
public onSelectChildProduct(event: CustomEvent<SelectChildProductEventArgs>) {
event.stopPropagation();
const {parentPermanentId, childPermanentId} = event.detail;

this.recommendations.promoteChildToParent(
childPermanentId,
parentPermanentId
);
this.recommendations.promoteChildToParent(event.detail.child);
}

public get focusTarget() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,7 @@ export class AtomicCommerceSearchBox
@Listen('atomic/selectChildProduct')
public onSelectChildProduct(event: CustomEvent<SelectChildProductEventArgs>) {
event.stopPropagation();
const {parentPermanentId, childPermanentId} = event.detail;
this.bindings.store.state.activeProductChild = {
parentPermanentId,
childPermanentId,
};
this.bindings.store.state.activeProductChild = event.detail.child;
this.suggestionManager.forceUpdate();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import {CommerceBindings} from '../../atomic-commerce-interface/atomic-commerce-
import {ProductContext} from '../product-template-decorators';

export interface SelectChildProductEventArgs {
childPermanentId: string;
parentPermanentId: string;
child: ChildProduct;
}

/**
Expand Down Expand Up @@ -98,11 +97,10 @@ export class AtomicProductChildren
});
}

private onSelectChild(childPermanentId: string, parentPermanentId: string) {
this.activeChildId = childPermanentId;
private onSelectChild(child: ChildProduct) {
this.activeChildId = child.permanentid;
this.selectChildProduct.emit({
childPermanentId,
parentPermanentId,
child,
});
}

Expand Down Expand Up @@ -131,15 +129,10 @@ export class AtomicProductChildren
class={`product-child${child.permanentid === this.activeChildId ? this.activeChildClasses : ' '}`}
title={child.ec_name || undefined}
onKeyPress={(event) =>
event.key === 'Enter' &&
this.onSelectChild(child.permanentid, this.product.permanentid)
}
onMouseEnter={() =>
this.onSelectChild(child.permanentid, this.product.permanentid)
}
onTouchStart={() =>
this.onSelectChild(child.permanentid, this.product.permanentid)
event.key === 'Enter' && this.onSelectChild(child)
}
onMouseEnter={() => this.onSelectChild(child)}
onTouchStart={() => this.onSelectChild(child)}
>
<img
class="aspect-square p-1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,11 @@ export class AtomicCommerceSearchBoxInstantProducts
});

this.bindings.store.onChange('activeProductChild', () => {
this.instantProducts.promoteChildToParent(
this.bindings.store.state.activeProductChild.childPermanentId,
this.bindings.store.state.activeProductChild.parentPermanentId
);
if (this.bindings.store.state.activeProductChild) {
this.instantProducts.promoteChildToParent(
this.bindings.store.state.activeProductChild
);
}
});

this.itemTemplateProvider = new ProductTemplateProvider({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {ChildProduct} from '../../../api/commerce/common/product';
import {stateKey} from '../../../app/state-key';
import {
registerInstantProducts,
Expand Down Expand Up @@ -46,8 +47,9 @@ describe('instant products', () => {
// TODO KIT-3210 test #updateQuery, #clearExpired, and #state

it('#promoteChildToParent dispatches #promoteChildToParent with the correct arguments', () => {
const childPermanentId = 'childPermanentId';
const parentPermanentId = 'parentPermanentId';
const child = {
permanentid: 'childPermanentId',
} as ChildProduct;

const query = 'query';
engine[stateKey].instantProducts![searchBoxId] = {
Expand All @@ -56,11 +58,10 @@ describe('instant products', () => {
};
instantProducts = buildInstantProducts(engine, {options: {searchBoxId}});

instantProducts.promoteChildToParent(childPermanentId, parentPermanentId);
instantProducts.promoteChildToParent(child);

expect(promoteChildToParent).toHaveBeenCalledWith({
childPermanentId,
parentPermanentId,
child,
id: searchBoxId,
query,
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {NumberValue, Schema} from '@coveo/bueno';
import {SerializedError} from '@reduxjs/toolkit';
import {CommerceAPIErrorResponse} from '../../../api/commerce/commerce-api-error-response';
import {Product} from '../../../api/commerce/common/product';
import {ChildProduct, Product} from '../../../api/commerce/common/product';
import {CommerceEngine} from '../../../app/commerce-engine/commerce-engine';
import {stateKey} from '../../../app/state-key';
import {
Expand Down Expand Up @@ -78,13 +78,9 @@ export interface InstantProducts extends Controller {
* **Note:** In the controller state, a product that has children will always include itself as its own child so that
* it can be rendered as a nested product, and restored as the parent product through this method as needed.
*
* @param childPermanentId The permanentid of the child product that will become the new parent.
* @param parentPermanentId The permanentid of the current parent product of the child product to promote.
* @param child The child product that will become the new parent.
*/
promoteChildToParent(
childPermanentId: string,
parentPermanentId: string
): void;
promoteChildToParent(child: ChildProduct): void;

/**
* Creates an `InteractiveProduct` sub-controller.
Expand Down Expand Up @@ -198,11 +194,10 @@ export function buildInstantProducts(
);
},

promoteChildToParent(childPermanentId, parentPermanentId) {
promoteChildToParent(child: ChildProduct) {
dispatch(
promoteChildToParent({
childPermanentId,
parentPermanentId,
child,
id: searchBoxId,
query: getQuery(),
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {ChildProduct} from '../../../api/commerce/common/product';
import {configuration} from '../../../app/common-reducers';
import {contextReducer} from '../../../features/commerce/context/context-slice';
import {
Expand Down Expand Up @@ -88,14 +89,12 @@ describe('headless product-listing', () => {
ProductListingActions,
'promoteChildToParent'
);
const childPermanentId = 'childPermanentId';
const parentPermanentId = 'parentPermanentId';
const child = {permanentid: 'childPermanentId'} as ChildProduct;

productListing.promoteChildToParent(childPermanentId, parentPermanentId);
productListing.promoteChildToParent(child);

expect(promoteChildToParent).toHaveBeenCalledWith({
childPermanentId,
parentPermanentId,
child,
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {CommerceAPIErrorStatusResponse} from '../../../api/commerce/commerce-api-error-response';
import {Product} from '../../../api/commerce/common/product';
import {ChildProduct, Product} from '../../../api/commerce/common/product';
import {CommerceEngine} from '../../../app/commerce-engine/commerce-engine';
import {configuration} from '../../../app/common-reducers';
import {stateKey} from '../../../app/state-key';
Expand Down Expand Up @@ -74,13 +74,9 @@ export interface ProductListing
* **Note:** In the controller state, a product that has children will always include itself as its own child so that
* it can be rendered as a nested product, and restored as the parent product through this method as needed.
*
* @param childPermanentId The permanentid of the child product that will become the new parent.
* @param parentPermanentId The permanentid of the current parent product of the child product to promote.
* @param child The child product that will become the new parent.
*/
promoteChildToParent(
childPermanentId: string,
parentPermanentId: string
): void;
promoteChildToParent(child: ChildProduct): void;

/**
* A scoped and simplified part of the headless state that is relevant to the `ProductListing` controller.
Expand Down Expand Up @@ -144,8 +140,8 @@ export function buildProductListing(engine: CommerceEngine): ProductListing {
};
},

promoteChildToParent(childPermanentId, parentPermanentId) {
dispatch(promoteChildToParent({childPermanentId, parentPermanentId}));
promoteChildToParent(child: ChildProduct) {
dispatch(promoteChildToParent({child}));
},

refresh: () => dispatch(fetchProductListing()),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {ChildProduct} from '../../../api/commerce/common/product';
import {
fetchRecommendations,
promoteChildToParent,
Expand Down Expand Up @@ -33,14 +34,12 @@ describe('headless recommendations', () => {
});

it('#promoteChildToParent dispatches #promoteChildToParent with the correct arguments', () => {
const childPermanentId = 'childPermanentId';
const parentPermanentId = 'parentPermanentId';
const child = {permanentid: 'childPermanentId'} as ChildProduct;

recommendations.promoteChildToParent(childPermanentId, parentPermanentId);
recommendations.promoteChildToParent(child);

expect(promoteChildToParent).toHaveBeenCalledWith({
childPermanentId,
parentPermanentId,
child,
slotId: 'slot-id',
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {createSelector} from '@reduxjs/toolkit';
import {CommerceAPIErrorStatusResponse} from '../../../api/commerce/commerce-api-error-response';
import {Product} from '../../../api/commerce/common/product';
import {ChildProduct, Product} from '../../../api/commerce/common/product';
import {
CommerceEngine,
CommerceEngineState,
Expand Down Expand Up @@ -60,13 +60,9 @@ export interface Recommendations
* **Note:** In the controller state, a product that has children will always include itself as its own child so that
* it can be rendered as a nested product, and restored as the parent product through this method as needed.
*
* @param childPermanentId The permanentid of the child product that will become the new parent.
* @param parentPermanentId The permanentid of the current parent product of the child product to promote.
* @param child The child product that will become the new parent.
*/
promoteChildToParent(
childPermanentId: string,
parentPermanentId: string
): void;
promoteChildToParent(child: ChildProduct): void;

/**
* A scoped and simplified part of the headless state that is relevant to the `Recommendations` controller.
Expand Down Expand Up @@ -154,10 +150,8 @@ export function buildRecommendations(
...controller,
...subControllers,

promoteChildToParent(childPermanentId, parentPermanentId) {
dispatch(
promoteChildToParent({childPermanentId, parentPermanentId, slotId})
);
promoteChildToParent(child: ChildProduct) {
dispatch(promoteChildToParent({child, slotId}));
},

get state() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {ChildProduct} from '../../../api/commerce/common/product';
import {configuration} from '../../../app/common-reducers';
import {contextReducer as commerceContext} from '../../../features/commerce/context/context-slice';
import {
Expand Down Expand Up @@ -90,15 +91,11 @@ describe('headless search', () => {
SearchActions,
'promoteChildToParent'
);
const childPermanentId = 'childPermanentId';
const parentPermanentId = 'parentPermanentId';
const child = {permanentid: 'childPermanentId'} as ChildProduct;

search.promoteChildToParent(childPermanentId, parentPermanentId);
search.promoteChildToParent(child);

expect(promoteChildToParent).toHaveBeenCalledWith({
childPermanentId,
parentPermanentId,
});
expect(promoteChildToParent).toHaveBeenCalledWith({child});
});

it('executeFirstSearch dispatches #executeSearch', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {CommerceAPIErrorStatusResponse} from '../../../api/commerce/commerce-api-error-response';
import {Product} from '../../../api/commerce/common/product';
import {ChildProduct, Product} from '../../../api/commerce/common/product';
import {CommerceEngine} from '../../../app/commerce-engine/commerce-engine';
import {configuration} from '../../../app/common-reducers';
import {stateKey} from '../../../app/state-key';
Expand Down Expand Up @@ -62,13 +62,9 @@ export interface Search extends Controller, SearchSubControllers {
* **Note:** In the controller state, a product that has children will always include itself as its own child so that
* it can be rendered as a nested product, and restored as the parent product through this method as needed.
*
* @param childPermanentId The permanentid of the child product that will become the new parent.
* @param parentPermanentId The permanentid of the current parent product of the child product to promote.
* @param child The child product that will become the new parent.
*/
promoteChildToParent(
childPermanentId: string,
parentPermanentId: string
): void;
promoteChildToParent(child: ChildProduct): void;

/**
* A scoped and simplified part of the headless state that is relevant to the `Search` controller.
Expand Down Expand Up @@ -125,8 +121,8 @@ export function buildSearch(engine: CommerceEngine): Search {
return getState().commerceSearch;
},

promoteChildToParent(childPermanentId: string, parentPermanentId: string) {
dispatch(promoteChildToParent({childPermanentId, parentPermanentId}));
promoteChildToParent(child: ChildProduct) {
dispatch(promoteChildToParent({child}));
},

executeFirstSearch() {
Expand Down
Loading

0 comments on commit d333812

Please sign in to comment.