Skip to content

Commit

Permalink
feat(commerce): exclude unused location facets properties (#4574)
Browse files Browse the repository at this point in the history
A few properties aren't supported by location facets. We remove:
- `preventAutoSelect`
- `freezeCurrentValues`
- `excluded` state, and related methods

We also address leftover comments from
#4562

See #4562

[COMHUB-247]

[COMHUB-247]:
https://coveord.atlassian.net/browse/COMHUB-247?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
Spuffynism authored Oct 31, 2024
1 parent 49638b9 commit 0b25980
Show file tree
Hide file tree
Showing 20 changed files with 134 additions and 265 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
Context,
ContextState,
buildContext,
LocationFacetValue,
} from '@coveo/headless/commerce';
import {Component, h, State, Element, Prop} from '@stencil/core';
import {FocusTargetController} from '../../../utils/accessibility-utils';
Expand Down Expand Up @@ -42,6 +43,7 @@ import {CommerceBindings} from '../atomic-commerce-interface/atomic-commerce-int

type AnyFacetValue =
| RegularFacetValue
| LocationFacetValue
| NumericFacetValue
| DateFacetValue
| CategoryFacetValue;
Expand Down Expand Up @@ -288,14 +290,19 @@ export class AtomicCommerceBreadbox
(pathValue: string) =>
getFieldValueCaption(field, pathValue, this.bindings.i18n)
);
default:
case 'regular':
return [
getFieldValueCaption(
field,
(value.value as RegularFacetValue).value,
this.bindings.i18n
),
];
default: {
// TODO COMHUB-291 support location breadcrumb
this.bindings.engine.logger.warn('Unexpected breadcrumb type.');
return [];
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ export class AtomicCommerceFacets implements InitializableComponent<Bindings> {
></atomic-commerce-category-facet>
);
default: {
// TODO COMHUB-291 support location facet
this.bindings.engine.logger.warn('Unexpected facet type.');
return;
}
Expand Down
1 change: 1 addition & 0 deletions packages/headless/src/commerce.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export type {
export type {DateFilterRange} from './controllers/core/facets/range-facet/date-facet/headless-core-date-filter.js';
export type {
FacetType,
BaseFacetValue,
FacetValueRequest,
RegularFacetValue,
LocationFacetValueRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ import {
NumericFacetValue,
RegularFacetValue,
} from '../../../../features/commerce/facets/facet-set/interfaces/response.js';
import {
toggleExcludeLocationFacetValue,
toggleSelectLocationFacetValue,
} from '../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {toggleSelectLocationFacetValue} from '../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {
toggleExcludeNumericFacetValue,
toggleSelectNumericFacetValue,
Expand Down Expand Up @@ -270,10 +267,23 @@ describe('core breadcrumb manager', () => {
expectBreadcrumbToBePresentInState(breadcrumb);
});

describe.each([
['selected', toggleSelectLocationFacetValue],
['excluded', toggleExcludeLocationFacetValue],
])('#deselect when facet is %s', generateDeselectionTestCases(breadcrumb));
describe('#deselect when facet is selected', () => {
beforeEach(() => {
breadcrumb.state = 'selected';
deselectBreadcrumb();
});

it('dispatches #toggleSelectActionCreator', () => {
expect(toggleSelectLocationFacetValue).toHaveBeenCalledWith({
facetId,
selection: breadcrumb,
});
});

it('dispatches #fetchProductsActionCreator', () => {
expect(fetchProductsActionCreator).toHaveBeenCalled();
});
});
});

describe('numeric facet breadcrumbs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ import {
NumericFacetResponse,
RegularFacetResponse,
} from '../../../../features/commerce/facets/facet-set/interfaces/response.js';
import {
toggleExcludeLocationFacetValue,
toggleSelectLocationFacetValue,
} from '../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {toggleSelectLocationFacetValue} from '../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {
toggleExcludeNumericFacetValue,
toggleSelectNumericFacetValue,
Expand Down Expand Up @@ -125,17 +122,11 @@ interface ActionCreators {
toggleExcludeActionCreator?: ToggleActionCreator;
}

const facetTypeWithoutExcludeAction: FacetType = 'hierarchical';

const actions: Record<FacetType, ActionCreators> = {
regular: {
toggleSelectActionCreator: toggleSelectFacetValue,
toggleExcludeActionCreator: toggleExcludeFacetValue,
},
location: {
toggleSelectActionCreator: toggleSelectLocationFacetValue,
toggleExcludeActionCreator: toggleExcludeLocationFacetValue,
},
numericalRange: {
toggleSelectActionCreator: toggleSelectNumericFacetValue,
toggleExcludeActionCreator: toggleExcludeNumericFacetValue,
Expand All @@ -144,7 +135,10 @@ const actions: Record<FacetType, ActionCreators> = {
toggleSelectActionCreator: toggleSelectDateFacetValue,
toggleExcludeActionCreator: toggleExcludeDateFacetValue,
},
[facetTypeWithoutExcludeAction]: {
location: {
toggleSelectActionCreator: toggleSelectLocationFacetValue,
},
hierarchical: {
toggleSelectActionCreator: deselectAllValuesInCoreFacet,
},
};
Expand Down Expand Up @@ -198,16 +192,19 @@ export function buildCoreBreadcrumbManager(
selection,
})
);
dispatch(
updateCoreFacetFreezeCurrentValues({
facetId: facet.facetId,
freezeCurrentValues: false,
})
);

if (facet.type !== 'location') {
dispatch(
updateCoreFacetFreezeCurrentValues({
facetId: facet.facetId,
freezeCurrentValues: false,
})
);
}
dispatch(options.fetchProductsActionCreator());
} else if (
selection.state === 'excluded' &&
facet.type !== facetTypeWithoutExcludeAction
facet.type !== 'location'
) {
dispatch(
actions[facet.type].toggleExcludeActionCreator!({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import {
AnyFacetResponse,
AnyFacetValueResponse,
BaseFacetValue,
CategoryFacetValue,
DateFacetValue,
LocationFacetValue,
Expand Down Expand Up @@ -47,6 +48,7 @@ export type {
DateFacetValue,
CategoryFacetValueRequest,
CategoryFacetValue,
BaseFacetValue,
};

export interface FacetControllerType<T extends FacetType> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import {LocationFacetRequest} from '../../../../../features/commerce/facets/facet-set/interfaces/request.js';
import {
toggleExcludeLocationFacetValue,
toggleSelectLocationFacetValue,
} from '../../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {toggleSelectLocationFacetValue} from '../../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {CommerceAppState} from '../../../../../state/commerce-app-state.js';
import {buildMockCommerceFacetRequest} from '../../../../../test/mock-commerce-facet-request.js';
import {buildMockCommerceLocationFacetResponse} from '../../../../../test/mock-commerce-facet-response.js';
Expand Down Expand Up @@ -88,16 +85,6 @@ describe('LocationFacet', () => {
});
});

it('#toggleExclude dispatches #toggleExcludeLocationFacetValue with correct payload', () => {
const facetValue = buildMockCommerceLocationFacetValue({value: 'TED'});
facet.toggleExclude(facetValue);

expect(toggleExcludeLocationFacetValue).toHaveBeenCalledWith({
facetId,
selection: facetValue,
});
});

it('#type returns "location"', () => {
expect(facet.type).toBe('location');
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import {CommerceEngine} from '../../../../../app/commerce-engine/commerce-engine.js';
import {LocationFacetValue} from '../../../../../features/commerce/facets/facet-set/interfaces/response.js';
import {
toggleExcludeLocationFacetValue,
toggleSelectLocationFacetValue,
} from '../../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {toggleSelectLocationFacetValue} from '../../../../../features/commerce/facets/location-facet/location-facet-actions.js';
import {
CoreCommerceFacet,
CoreCommerceFacetOptions,
Expand All @@ -29,9 +26,9 @@ export type LocationFacetState = Omit<
* The `LocationFacet` sub-controller offers a high-level programming interface for implementing a location commerce
* facet UI component.
*/
export type LocationFacet = CoreCommerceFacet<
FacetValueRequest,
LocationFacetValue
export type LocationFacet = Omit<
CoreCommerceFacet<FacetValueRequest, LocationFacetValue>,
'isValueExcluded' | 'toggleExclude' | 'toggleSingleExclude'
> & {
state: LocationFacetState;
} & FacetControllerType<'location'>;
Expand All @@ -52,14 +49,15 @@ export function buildCommerceLocationFacet(
engine: CommerceEngine,
options: LocationFacetOptions
): LocationFacet {
const coreController = buildCoreCommerceFacet<
FacetValueRequest,
LocationFacetValue
>(engine, {
const {
toggleSingleExclude,
toggleExclude,
isValueExcluded,
...coreController
} = buildCoreCommerceFacet<FacetValueRequest, LocationFacetValue>(engine, {
options: {
...options,
toggleSelectActionCreator: toggleSelectLocationFacetValue,
toggleExcludeActionCreator: toggleExcludeLocationFacetValue,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ import {
updateDateFacetValues,
} from '../date-facet/date-facet-actions.js';
import {getFacetIdWithCommerceFieldSuggestionNamespace} from '../facet-search-set/commerce-facet-search-actions.js';
import {
toggleExcludeLocationFacetValue,
toggleSelectLocationFacetValue,
} from '../location-facet/location-facet-actions.js';
import {toggleSelectLocationFacetValue} from '../location-facet/location-facet-actions.js';
import {
toggleExcludeNumericFacetValue,
toggleSelectNumericFacetValue,
Expand Down Expand Up @@ -1005,12 +1002,6 @@ describe('commerceFacetSetReducer', () => {
facetValueState: 'selected' as FacetValueState,
toggleAction: toggleSelectLocationFacetValue,
},
{
title:
'dispatching #toggleExcludeLocationFacetValue with a registered facet id',
facetValueState: 'excluded' as FacetValueState,
toggleAction: toggleExcludeLocationFacetValue,
},
])(
'$title',
({
Expand Down Expand Up @@ -1104,63 +1095,13 @@ describe('commerceFacetSetReducer', () => {
).find((req) => req.value === facetValue.value);
expect(targetValue?.state).toBe('idle');
});

it('sets #preventAutoSelect to true', () => {
const facetValue = buildMockCommerceLocationFacetValue({
value: 'TED',
});
const facetValueRequest =
convertLocationFacetValueToRequest(facetValue);

state[facetId] = buildMockCommerceFacetSlice({
request: buildMockCommerceFacetRequest({
values: [facetValueRequest],
type: 'location',
}),
});

const action = toggleAction({
facetId,
selection: facetValue,
});
const finalState = commerceFacetSetReducer(state, action);

expect(finalState[facetId]?.request.preventAutoSelect).toBe(true);
});

it('sets #freezeCurrentValues to true', () => {
const facetValue = buildMockCommerceLocationFacetValue({
value: 'TED',
});
const facetValueRequest =
convertLocationFacetValueToRequest(facetValue);

state[facetId] = buildMockCommerceFacetSlice({
request: buildMockCommerceFacetRequest({
values: [facetValueRequest],
type: 'location',
}),
});

const action = toggleAction({
facetId,
selection: facetValue,
});
const finalState = commerceFacetSetReducer(state, action);

expect(finalState[facetId]?.request.freezeCurrentValues).toBe(true);
});
});

describe.each([
{
facetValueState: 'selected' as FacetValueState,
toggleAction: toggleSelectLocationFacetValue,
},
{
facetValueState: 'excluded' as FacetValueState,
toggleAction: toggleExcludeLocationFacetValue,
},
])(
'when the facet value does not exist',
({
Expand Down Expand Up @@ -1214,20 +1155,6 @@ describe('commerceFacetSetReducer', () => {
).toBe(2);
expect(finalState[facetId]?.request.values.length).toBe(4);
});

it('sets #preventAutoSelect to true', () => {
state[facetId] = buildMockCommerceFacetSlice({
request: buildMockCommerceFacetRequest({type: 'location'}),
});

const action = toggleAction({
facetId,
selection: buildMockCommerceLocationFacetValue({value: 'TED'}),
});
const finalState = commerceFacetSetReducer(state, action);

expect(finalState[facetId]?.request.preventAutoSelect).toBe(true);
});
}
);
}
Expand Down Expand Up @@ -1258,33 +1185,6 @@ describe('commerceFacetSetReducer', () => {

expect(() => commerceFacetSetReducer(state, action)).not.toThrow();
});

it('dispatching #toggleExcludeLocationFacetValue with an invalid id does not throw', () => {
const facetValue = buildMockCommerceLocationFacetValue({value: 'TED'});
const action = toggleExcludeLocationFacetValue({
facetId: '1',
selection: facetValue,
});

expect(() => commerceFacetSetReducer(state, action)).not.toThrow();
});

it('dispatching #toggleExcludeLocationFacetValue with an invalid facet type does not throw', () => {
const facetValue = buildMockCommerceLocationFacetValue({value: 'TED'});
const facet = buildMockCommerceFacetRequest({
type: 'numericalRange',
values: [facetValue],
});
state[facet.facetId] = buildMockCommerceFacetSlice({
request: facet,
});
const action = toggleExcludeLocationFacetValue({
facetId: facet.facetId,
selection: facetValue,
});

expect(() => commerceFacetSetReducer(state, action)).not.toThrow();
});
});

describe('for numericalRange facets', () => {
Expand Down
Loading

0 comments on commit 0b25980

Please sign in to comment.