Skip to content

Commit

Permalink
feat: simplify the arguments for the setRanges() method in numeric …
Browse files Browse the repository at this point in the history
…and date facets (#4042)

Refactor date and numeric facet controllers to use `DateRangeRequest`
and `NumericRangeRequest` instead of `*FacetValue` types.

The current `setRanges` method requires unnecessary parameters from the
implementers, such as `isAutoSelected`, `isSuggested`,
`moreValuesAvailable`, and `numberOfResults`.

This update changes the method's type so that users only need to provide
the necessary parameters: `start`, `end`, `state`, and `endInclusive`.

⚠️ BREAKINGish CHANGE ⚠️ 

[KIT-3226](https://coveord.atlassian.net/browse/KIT-3226).

[KIT-3226]:
https://coveord.atlassian.net/browse/KIT-3226?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Olivier Lamothe <[email protected]>
Co-authored-by: GitHub Actions Bot <>
Co-authored-by: Frederic Beaudoin <[email protected]>
Co-authored-by: Louis Bompart <[email protected]>
Co-authored-by: Felix Perron-Brault <[email protected]>
  • Loading branch information
5 people authored Jun 3, 2024
1 parent ea5c9b1 commit 20c0978
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,6 @@ export class AtomicCommerceTimeframeFacet
start,
end,
endInclusive,
isAutoSelected: false,
isSuggested: false,
numberOfResults: 0,
moreValuesAvailable: false,
state: 'selected',
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ export class FacetNumberInput {
start: this.start!,
end: this.end!,
endInclusive: true,
isAutoSelected: false,
state: 'selected',
numberOfResults: 0,
isSuggested: false,
moreValuesAvailable: false,
},
]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
buildMockCommerceEngine,
MockedCommerceEngine,
} from '../../../../../test/mock-engine-v2';
import {DateFacetValue} from '../headless-core-commerce-facet';
import {DateRangeRequest} from '../headless-core-commerce-facet';
import {
DateFacet,
DateFacetOptions,
Expand Down Expand Up @@ -98,15 +98,22 @@ describe('DateFacet', () => {
});

describe('#setRanges', () => {
let values: DateFacetValue[];
let values: DateRangeRequest[];
beforeEach(() => {
values = [buildMockCommerceDateFacetValue()];
values = [buildMockCommerceDateFacetValue()].map(
({start, end, endInclusive, state}) => ({
start,
end,
endInclusive,
state,
})
);
facet.setRanges(values);
});
it('dispatches #updateDateFacetValues with the correct payload', () => {
expect(updateDateFacetValues).toHaveBeenCalledWith({
facetId,
values,
values: values.map((value) => ({...value, numberOfResults: 0})),
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export type DateFacet = CoreCommerceFacet<DateRangeRequest, DateFacetValue> & {
*
* @param ranges - The new ranges to set.
*/
setRanges: (ranges: DateFacetValue[]) => void;
setRanges: (ranges: DateRangeRequest[]) => void;
/**
* The state of the `DateFacet` controller.
*/
Expand Down Expand Up @@ -75,12 +75,11 @@ export function buildCommerceDateFacet(
return {
...coreController,

// TODO: KIT-3226 do not accept DateFacetValue as the argument as it expose unnecessary properties
setRanges(ranges: DateFacetValue[]) {
setRanges(ranges: DateRangeRequest[]) {
dispatch(
updateDateFacetValues({
facetId,
values: ranges,
values: ranges.map((range) => ({...range, numberOfResults: 0})),
})
);
dispatch(fetchProductsActionCreator());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {FacetType} from '../../../../../features/commerce/facets/facet-set/interfaces/common';
import {NumericFacetRequest} from '../../../../../features/commerce/facets/facet-set/interfaces/request';
import {NumericFacetValue} from '../../../../../features/commerce/facets/facet-set/interfaces/response';
import {
toggleExcludeNumericFacetValue,
toggleSelectNumericFacetValue,
Expand All @@ -16,6 +15,7 @@ import {
buildMockCommerceEngine,
MockedCommerceEngine,
} from '../../../../../test/mock-engine-v2';
import {NumericRangeRequest} from '../headless-core-commerce-facet';
import {
NumericFacet,
NumericFacetOptions,
Expand Down Expand Up @@ -103,15 +103,22 @@ describe('NumericFacet', () => {
});

describe('#setRanges', () => {
let values: NumericFacetValue[];
let values: NumericRangeRequest[];
beforeEach(() => {
values = [buildMockCommerceNumericFacetValue()];
values = [buildMockCommerceNumericFacetValue()].map(
({start, end, endInclusive, state}) => ({
start,
end,
endInclusive,
state,
})
);
facet.setRanges(values);
});
it('dispatches #updateNumericFacetValues with the correct payload', () => {
expect(updateNumericFacetValues).toHaveBeenCalledWith({
facetId,
values,
values: values.map((value) => ({...value, numberOfResults: 0})),
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export type NumericFacet = CoreCommerceFacet<
*
* @param ranges - The new ranges to set.
*/
setRanges: (ranges: NumericFacetValue[]) => void;
setRanges: (ranges: NumericRangeRequest[]) => void;
/**
* The state of the `NumericFacet` controller.
*/
Expand Down Expand Up @@ -96,13 +96,11 @@ export function buildCommerceNumericFacet(
return {
...coreController,

setRanges(ranges: NumericFacetValue[]) {
// TODO: KIT-3226 do not accept NumericFacetValue as the argument as it exposes unnecessary properties
// The properties isAutoSelected, isSuggested, numberOfResults, and moreValuesAvailable should not be relevant to the user when setting a range
setRanges(ranges: NumericRangeRequest[]) {
dispatch(
updateNumericFacetValues({
facetId,
values: ranges,
values: ranges.map((range) => ({...range, numberOfResults: 0})),
})
);
dispatch(fetchProductsActionCreator());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,6 @@ export const commerceFacetSetReducer = createReducer(
return;
}

// TODO: KIT-3226 No need for this function if the values in the payload already contains appropriate parameters
request.values = convertToDateRangeRequests(values);
request.numberOfValues = values.length;
})
Expand Down

0 comments on commit 20c0978

Please sign in to comment.