Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor rule model #4639

Merged
merged 2 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/clean-meals-obey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"saleor-dashboard": minor
---

Refactor Rule model
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const DiscountCreateForm = ({
triggerChange: methods.trigger,
});

const { rules, onDeleteRule, onRuleSubmit } = useRulesHandlers();
const { rules, onDeleteRule, onRuleSubmit } = useRulesHandlers("catalog");

const handleSubmit: SubmitHandler<DiscoutFormData> = data => {
onSubmit({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const rule = {
describe("DiscountCreateForm useRulesHandlers", () => {
it("should allow to add new rule ", () => {
// Arrange
const { result } = renderHook(() => useRulesHandlers());
const { result } = renderHook(() => useRulesHandlers("catalog"));

// Act
act(() => {
Expand All @@ -29,7 +29,7 @@ describe("DiscountCreateForm useRulesHandlers", () => {

it("should allow to edit rule at index", () => {
// Arrange
const { result } = renderHook(() => useRulesHandlers());
const { result } = renderHook(() => useRulesHandlers("catalog"));

const rule = {
name: "Rule 1",
Expand All @@ -55,7 +55,7 @@ describe("DiscountCreateForm useRulesHandlers", () => {

it("should allow to delete rule at index", () => {
// Arrange
const { result } = renderHook(() => useRulesHandlers());
const { result } = renderHook(() => useRulesHandlers("catalog"));

const rule = {
name: "Rule 1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
import { Rule } from "@dashboard/discounts/models";
import { sortRules } from "@dashboard/discounts/utils";
import { useState } from "react";
import {} from "@dashboard/graphql";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: I'm surprised linters even allow that

import { useEffect, useState } from "react";

export const useRulesHandlers = () => {
export const useRulesHandlers = (
discountType: "catalog", // to be replaced by PromotionTypeEnum when API return this field
) => {
const [rules, setRules] = useState<Rule[]>([]);

useEffect(() => {
setRules([]);
}, [discountType]);

const onDeleteRule = (ruleDeleteIndex: number) => {
setRules(rules => rules.filter((_, index) => index !== ruleDeleteIndex));
};

const onRuleSubmit = async (data: Rule, ruleEditIndex: number | null) => {
const ruleObj = Rule.fromFormValues(data);

if (ruleEditIndex !== null) {
setRules(rules => {
rules[ruleEditIndex] = ruleObj;
rules[ruleEditIndex] = data;
return rules;
});
} else {
setRules(sortRules([...rules, ruleObj]));
setRules(sortRules([...rules, data]));
}
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Rule } from "@dashboard/discounts/models";
import { mapAPIRuleToForm, Rule } from "@dashboard/discounts/models";
import {
PromotionDetailsFragment,
PromotionRuleCreateErrorFragment,
Expand Down Expand Up @@ -31,7 +31,9 @@ export const useRulesHandlers = ({
const [rulesErrors, setRulesErrors] = useState<Array<CommonError<any>>>([]);
const [labelsMap, setLabelMap] = useState<Record<string, string>>({});

const rules = data?.rules?.map(rule => Rule.fromAPI(rule, labelsMap)) ?? [];
const rules =
data?.rules?.map(rule => mapAPIRuleToForm("catalog", rule, labelsMap)) ??
[];

useEffect(() => {
setLabelMap(labels => {
Expand All @@ -55,16 +57,16 @@ export const useRulesHandlers = ({
PromotionRuleUpdateErrorFragment | PromotionRuleCreateErrorFragment
>
> = [];
const ruleObj = Rule.fromFormValues(rule);

if (ruleEditIndex !== null) {
updateLabels(rule);
errors = await onRuleUpdateSubmit(ruleObj);
errors = await onRuleUpdateSubmit(rule);

if (errors.length > 0) {
setRulesErrors(errors);
}
} else {
errors = await onRuleCreateSubmit(ruleObj);
errors = await onRuleCreateSubmit(rule);
if (errors.length > 0) {
setRulesErrors(errors);
}
Expand Down
6 changes: 3 additions & 3 deletions src/discounts/components/DiscountDetailsForm/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ describe("getCurrentConditionsValuesLabels", () => {
it("should return empty object if no values", () => {
expect(
getCurrentConditionsValuesLabels([
{ conditions: [{ values: [] }] },
{ conditions: [{ value: [] }] },
] as unknown as Rule[]),
).toEqual({});
});

it("should return object with value as key and label as value", () => {
expect(
getCurrentConditionsValuesLabels([
{ conditions: [{ values: [{ value: "test", label: "test2" }] }] },
{ conditions: [{ values: [{ value: "test3", label: "test4" }] }] },
{ conditions: [{ value: [{ value: "test", label: "test2" }] }] },
{ conditions: [{ value: [{ value: "test3", label: "test4" }] }] },
] as unknown as Rule[]),
).toEqual({ test: "test2", test3: "test4" });
});
Expand Down
10 changes: 6 additions & 4 deletions src/discounts/components/DiscountDetailsForm/utils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Rule } from "@dashboard/discounts/models";
import { isArrayOfOptions, Rule } from "@dashboard/discounts/models";
import { Option } from "@saleor/macaw-ui-next";

export const getCurrentConditionsValuesLabels = (rules: Rule[]) => {
return rules
export const getCurrentConditionsValuesLabels = (rule: Rule[]) => {
return rule
.flatMap(rule => rule.conditions)
.flatMap(condition => condition.values)
.filter(condition => isArrayOfOptions(condition.value))
.flatMap(condition => condition.value as Option[])
.reduce((acc, value) => {
// Initali value and label might contain id
if (value.value !== value.label) {
Expand Down
35 changes: 21 additions & 14 deletions src/discounts/components/DiscountRules/DiscountRules.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ jest.mock("react-intl", () => ({
),
}));

jest.mock("@dashboard/hooks/useNotifier", () => ({
__esModule: true,
default: jest.fn(() => () => undefined),
}));

const Wrapper = ({ children }: { children: ReactNode }) => {
return (
<MockedProvider
Expand Down Expand Up @@ -63,14 +68,15 @@ const rules = [
channel: { label: "Test", value: "Q2hhbm5lcDoy" },
conditions: [
{
type: "product",
condition: "is",
values: [
id: "product",
type: "is",
value: [
{ label: "Product-1", value: "prod-1" },
{ label: "Product-2", value: "prod-2" },
],
},
],
rewardType: null,
rewardValue: 12,
rewardValueType: RewardValueTypeEnum.FIXED,
},
Expand All @@ -81,11 +87,12 @@ const rules = [
channel: { label: "Test", value: "Q2hhbm5lcDoy" },
conditions: [
{
type: "category",
condition: "is",
values: [{ label: "Category-1", value: "cat-1" }],
id: "category",
type: "is",
value: [{ label: "Category-1", value: "cat-1" }],
},
],
rewardType: null,
rewardValue: 34,
rewardValueType: RewardValueTypeEnum.PERCENTAGE,
},
Expand Down Expand Up @@ -281,12 +288,12 @@ describe("DiscountRules", () => {
},
conditions: [
{
condition: "is",
type: "product",
values: [
type: "is",
id: "product",
value: [
{
label: "Bean Juice",
value: "UHJvZHVjdDo3OQ==",
label: "Apple Juice",
value: "UHJvZHVjdDo3Mg==",
},
],
},
Expand Down Expand Up @@ -415,9 +422,9 @@ describe("DiscountRules", () => {
},
conditions: [
{
condition: "is",
type: "product",
values: [
type: "is",
id: "product",
value: [
{
label: "Product-1",
value: "prod-1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { Condition, Rule as RuleType } from "@dashboard/discounts/models";
import {
createEmptyCodition,
Rule as RuleType,
} from "@dashboard/discounts/models";
import { ChannelFragment, RewardValueTypeEnum } from "@dashboard/graphql";
import { commonMessages } from "@dashboard/intl";
import { getFormErrors } from "@dashboard/utils/errors";
Expand Down Expand Up @@ -78,7 +81,7 @@ export const RuleForm = <ErrorCode,>({
setValue("channel", selectedChannel, { shouldValidate: true });

if (conditions.length > 0) {
setValue("conditions", [Condition.empty()]);
setValue("conditions", [createEmptyCodition()]);
} else {
setValue("conditions", []);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Combobox, Multiselect } from "@dashboard/components/Combobox";
import { Condition, Rule } from "@dashboard/discounts/models";
import { Condition, isArrayOfOptions, Rule } from "@dashboard/discounts/models";
import { ConditionType } from "@dashboard/discounts/types";
import { getSearchFetchMoreProps } from "@dashboard/hooks/makeTopLevelSearch/utils";
import { Box, Button, Option, RemoveIcon, Select } from "@saleor/macaw-ui-next";
import React from "react";
Expand All @@ -20,22 +21,21 @@ interface DiscountConditionRowProps {
conditionIndex: number;
onRemove: () => void;
updateCondition: (index: number, value: Condition) => void;
fetchOptions: FetchOptions | undefined;
typeToFetchMap: Record<ConditionType, FetchOptions>;
isConditionTypeSelected: (conditionType: string) => boolean;
}

export const RuleConditionRow = ({
conditionIndex,
onRemove,
fetchOptions,
typeToFetchMap,
isConditionTypeSelected,
updateCondition,
disabled = false,
}: DiscountConditionRowProps) => {
const intl = useIntl();

const ruleConditionTypeFieldName =
`conditions.${conditionIndex}.type` as const;
const ruleConditionTypeFieldName = `conditions.${conditionIndex}.id` as const;
const { field: typeField } = useController<
Rule,
typeof ruleConditionTypeFieldName
Expand All @@ -44,7 +44,7 @@ export const RuleConditionRow = ({
});

const ruleConditionValuesFieldName =
`conditions.${conditionIndex}.values` as const;
`conditions.${conditionIndex}.value` as const;
const { field: valuesField } = useController<
Rule,
typeof ruleConditionValuesFieldName
Expand All @@ -54,8 +54,11 @@ export const RuleConditionRow = ({

const { watch } = useFormContext<Rule>();
const condition = watch(`conditions.${conditionIndex}`);

const { fetch = () => {}, fetchMoreProps, options } = fetchOptions || {};
const {
fetch = () => {},
fetchMoreProps,
options,
} = typeToFetchMap[(condition?.id ?? "") as ConditionType] || {};

const discountConditionType = initialDiscountConditionType.filter(
condition => !isConditionTypeSelected(condition.value),
Expand All @@ -78,7 +81,7 @@ export const RuleConditionRow = ({
fetchOptions={() => {}}
options={discountConditionType}
onChange={e => {
condition.values = [];
condition.value = [];
updateCondition(conditionIndex, condition);
typeField.onChange(e.target.value);
}}
Expand Down Expand Up @@ -111,7 +114,7 @@ export const RuleConditionRow = ({
<Multiselect
size="medium"
data-test-id="rule-values"
value={condition.values}
value={isArrayOfOptions(condition.value) ? condition.value : []}
fetchOptions={fetch}
fetchMore={fetchMoreProps}
options={options ?? []}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { ConditionType } from "@dashboard/discounts/types";
import { Option } from "@saleor/macaw-ui-next";

export const getConditionTypeValue = (
conditionType: ConditionType | undefined | null,
conditionType: string | undefined | null,
conditionTypes: Option[],
) => {
if (!conditionType) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Condition, Rule } from "@dashboard/discounts/models";
import { createEmptyCodition, Rule } from "@dashboard/discounts/models";
import { ConditionType } from "@dashboard/discounts/types";
import { Box, Button, Text } from "@saleor/macaw-ui-next";
import React from "react";
Expand Down Expand Up @@ -60,7 +60,7 @@ export const RuleConditions = ({
size="small"
alignSelf="start"
disabled={disabled}
onClick={() => append(Condition.empty())}
onClick={() => append(createEmptyCodition())}
>
<FormattedMessage defaultMessage="Add condition" id="fg8dzN" />
</Button>
Expand All @@ -75,12 +75,10 @@ export const RuleConditions = ({
<Box display="flex" flexDirection="column" gap={4}>
{fields.map((condition, conditionIndex) => (
<RuleConditionRow
key={condition.id || conditionIndex}
disabled={disabled}
fetchOptions={
condition.type ? typeToFetchMap[condition.type] : undefined
}
typeToFetchMap={typeToFetchMap}
isConditionTypeSelected={isConditionTypeSelected}
key={condition.type || conditionIndex}
conditionIndex={conditionIndex}
updateCondition={update}
onRemove={() => {
Expand All @@ -96,7 +94,7 @@ export const RuleConditions = ({
size="small"
alignSelf="start"
disabled={disabled}
onClick={() => append(Condition.empty())}
onClick={() => append(createEmptyCodition())}
>
<FormattedMessage defaultMessage="Add condition" id="fg8dzN" />
</Button>
Expand Down
Loading
Loading