diff --git a/CHANGELOG.md b/CHANGELOG.md index 056dfcb0b..3ccceb2a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable, unreleased changes to this project will be documented in this file. ## [Unreleased] - Added links instead of imperative navigation with onClick - #1969 by @taniotanio7 +- Fixed clearing attribute values - #2047 by @witoszekdev ## 3.1 ### PREVIEW FEATURES diff --git a/src/attributes/utils/handlers.test.ts b/src/attributes/utils/handlers.test.ts index f03e0e28a..a1ef08e8b 100644 --- a/src/attributes/utils/handlers.test.ts +++ b/src/attributes/utils/handlers.test.ts @@ -1,9 +1,18 @@ -import { createAttributeMultiChangeHandler } from "@saleor/attributes/utils/handlers"; -import { AttributeInputData } from "@saleor/components/Attributes"; -import { AttributeInputTypeEnum } from "@saleor/graphql"; +import { + createAttributeMultiChangeHandler, + prepareAttributesInput +} from "@saleor/attributes/utils/handlers"; +import { + AttributeInput, + AttributeInputData +} from "@saleor/components/Attributes"; +import { + AttributeInputTypeEnum, + AttributeValueDetailsFragment +} from "@saleor/graphql"; import { FormsetData } from "@saleor/hooks/useFormset"; -const attributes: FormsetData = [ +const multipleValueAttributes: FormsetData = [ { data: { inputType: AttributeInputTypeEnum.DROPDOWN, @@ -108,13 +117,76 @@ const attributes: FormsetData = [ } ]; -describe("Multiple select", () => { +const ATTR_ID = "123" as const; + +interface CreateAttribute { + inputType: AttributeInputTypeEnum; + initialValue?: AttributeValueDetailsFragment[]; + availableValues?: AttributeValueDetailsFragment[]; + value?: string; +} + +const createAttribute = ({ + inputType, + value +}: CreateAttribute): AttributeInput => ({ + data: { + entityType: null, + inputType, + isRequired: false, + // those values don't matter + selectedValues: [], + values: [], + unit: null + }, + id: ATTR_ID, + label: "MyAttribute", + value: value !== null ? [value] : [] +}); + +const createSelectAttribute = (value: string) => + createAttribute({ + inputType: AttributeInputTypeEnum.DROPDOWN, + value + }); + +const createReferenceAttribute = (value: string) => + createAttribute({ + inputType: AttributeInputTypeEnum.REFERENCE, + value + }); + +const createBooleanAttribute = (value: string) => + createAttribute({ + inputType: AttributeInputTypeEnum.BOOLEAN, + value + }); + +const createRichTextAttribute = (value: string) => + createAttribute({ inputType: AttributeInputTypeEnum.RICH_TEXT, value }); + +const createDateAttribute = (value: string) => + createAttribute({ inputType: AttributeInputTypeEnum.DATE, value }); + +const createDateTimeAttribute = (value: string) => + createAttribute({ inputType: AttributeInputTypeEnum.DATE_TIME, value }); + +const createSwatchAttribute = (value: string) => + createAttribute({ inputType: AttributeInputTypeEnum.SWATCH, value }); + +const createNumericAttribute = (value: string) => + createAttribute({ inputType: AttributeInputTypeEnum.NUMERIC, value }); + +const createFileAttribute = (value: string) => + createAttribute({ inputType: AttributeInputTypeEnum.FILE, value }); + +describe("Multiple select change handler", () => { it("is able to select value", () => { const change = jest.fn(); const trigger = jest.fn(); const handler = createAttributeMultiChangeHandler( change, - attributes, + multipleValueAttributes, trigger ); @@ -133,7 +205,7 @@ describe("Multiple select", () => { const trigger = jest.fn(); const handler = createAttributeMultiChangeHandler( change, - attributes, + multipleValueAttributes, trigger ); @@ -150,7 +222,7 @@ describe("Multiple select", () => { const trigger = jest.fn(); const handler = createAttributeMultiChangeHandler( change, - attributes, + multipleValueAttributes, trigger ); @@ -164,3 +236,280 @@ describe("Multiple select", () => { expect(trigger).toHaveBeenCalledTimes(1); }); }); + +describe("Sending only changed attributes", () => { + // null in expected = attribute not present in output + describe("works with reference attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"my value"} | ${"my value"} | ${null} + ${"my value"} | ${null} | ${["my value"]} + ${null} | ${"my value"} | ${[]} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createReferenceAttribute(newAttr); + const prevAttribute = createReferenceAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, references: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with select attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"my value"} | ${"my value"} | ${null} + ${"my value"} | ${null} | ${["my value"]} + ${null} | ${"my value"} | ${[]} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createSelectAttribute(newAttr); + const prevAttribute = createSelectAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, values: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with boolean attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"true"} | ${true} | ${null} + ${"true"} | ${false} | ${true} + ${"true"} | ${null} | ${true} + ${"false"} | ${false} | ${null} + ${"false"} | ${true} | ${false} + ${"false"} | ${null} | ${false} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createBooleanAttribute(newAttr); + const prevAttribute = createBooleanAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, boolean: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with rich text attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"my value"} | ${"my value"} | ${null} + ${"my value"} | ${null} | ${"my value"} + ${null} | ${"my value"} | ${undefined} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createRichTextAttribute(newAttr); + const prevAttribute = createRichTextAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, richText: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with date attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"2021-01-01"} | ${"2021-01-01"} | ${null} + ${"2021-01-01"} | ${null} | ${"2021-01-01"} + ${null} | ${"2021-01-01"} | ${undefined} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createDateAttribute(newAttr); + const prevAttribute = createDateAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, date: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with date time attributes", () => { + const dateTime = "2021-01-01T11:00:00+01:00"; + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${dateTime} | ${dateTime} | ${null} + ${dateTime} | ${null} | ${dateTime} + ${null} | ${dateTime} | ${undefined} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createDateTimeAttribute(newAttr); + const prevAttribute = createDateTimeAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, dateTime: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with swatch attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"my value"} | ${"my value"} | ${null} + ${"my value"} | ${null} | ${["my value"]} + ${null} | ${"my value"} | ${[]} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createSwatchAttribute(newAttr); + const prevAttribute = createSwatchAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, values: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with numeric attributes", () => { + test.each` + newAttr | oldAttr | expected + ${null} | ${null} | ${null} + ${"1"} | ${"1"} | ${null} + ${"1"} | ${null} | ${["1"]} + ${null} | ${"1"} | ${[]} + `( + "$oldAttr -> $newAttr returns $expected", + ({ newAttr, oldAttr, expected }) => { + const attribute = createNumericAttribute(newAttr); + const prevAttribute = createNumericAttribute(oldAttr); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [] + }); + + const expectedResult = + expected !== null ? [{ id: ATTR_ID, values: expected }] : []; + expect(result).toEqual(expectedResult); + } + ); + }); + describe("works with file attributes", () => { + it("removes existing image (img -> null)", () => { + const attribute = createFileAttribute(null); + const prevAttribute = createNumericAttribute("bob.jpg"); + + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [ + { file: undefined, id: ATTR_ID, contentType: undefined, values: [] } + ] + }); + + // Files are deleted by using AttributeValueDetele mutation + expect(result).toEqual([]); + }); + it("adds new image (null -> img)", () => { + const attribute = createFileAttribute("bob.jpg"); + const prevAttribute = createNumericAttribute(null); + + const uploadUrl = "http://some-url.com/media/file_upload/bob.jpg"; + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [ + { + file: uploadUrl, + id: ATTR_ID, + contentType: "image/jpeg", + values: [] + } + ] + }); + + expect(result).toEqual([ + { + id: ATTR_ID, + file: uploadUrl, + contentType: "image/jpeg" + } + ]); + }); + it("replaces existing image (bob.jpg -> juice.png)", () => { + const attribute = createFileAttribute("bob.jpg"); + const prevAttribute = createNumericAttribute("juice.png"); + + const uploadUrl = "http://some-url.com/media/file_upload/juice.jpg"; + const result = prepareAttributesInput({ + attributes: [attribute], + prevAttributes: [prevAttribute], + updatedFileAttributes: [ + { + file: uploadUrl, + id: ATTR_ID, + contentType: "image/png", + values: [] + } + ] + }); + + expect(result).toEqual([ + { + id: ATTR_ID, + file: uploadUrl, + contentType: "image/png" + } + ]); + }); + }); +}); diff --git a/src/attributes/utils/handlers.ts b/src/attributes/utils/handlers.ts index 8304e4f50..2987a9a2d 100644 --- a/src/attributes/utils/handlers.ts +++ b/src/attributes/utils/handlers.ts @@ -22,6 +22,7 @@ import { } from "@saleor/hooks/useFormset"; import { FetchMoreProps, ReorderEvent } from "@saleor/types"; import { move, toggle } from "@saleor/utils/lists"; +import isEqual from "lodash/isEqual"; import { getFileValuesToUploadFromAttributes, isFileValueUnused } from "./data"; @@ -168,11 +169,6 @@ export function createAttributeValueReorderHandler( }; } -interface AttributesArgs { - attributes: AttributeInput[]; - updatedFileAttributes: AttributeValueInput[]; -} - function getFileInput( attribute: AttributeInput, updatedFileAttributes: AttributeValueInput[] @@ -202,11 +198,34 @@ function getBooleanInput(attribute: AttributeInput) { }; } +function getAttributesMap(attributes: AttributeInput[] | null) { + if (attributes && attributes?.length !== 0) { + return new Map( + attributes.map(attribute => [attribute.id, attribute.value]) + ); + } + return new Map(); +} + +interface AttributesArgs { + attributes: AttributeInput[]; + prevAttributes: AttributeInput[] | null; + updatedFileAttributes: AttributeValueInput[]; +} + export const prepareAttributesInput = ({ attributes, + prevAttributes, updatedFileAttributes -}: AttributesArgs): AttributeValueInput[] => - attributes.reduce((attrInput, attr) => { +}: AttributesArgs): AttributeValueInput[] => { + const prevAttributesMap = getAttributesMap(prevAttributes); + + return attributes.reduce((attrInput, attr) => { + const prevAttrValue = prevAttributesMap.get(attr.id); + if (isEqual(attr.value, prevAttrValue)) { + return attrInput; + } + const inputType = attr.data.inputType; if (inputType === AttributeInputTypeEnum.FILE) { const fileInput = getFileInput(attr, updatedFileAttributes); @@ -217,6 +236,11 @@ export const prepareAttributesInput = ({ } if (inputType === AttributeInputTypeEnum.BOOLEAN) { const booleanInput = getBooleanInput(attr); + // previous comparison doesn't work because value was string + if (isEqual([booleanInput.boolean], prevAttrValue)) { + return attrInput; + } + attrInput.push(booleanInput); return attrInput; } @@ -228,11 +252,6 @@ export const prepareAttributesInput = ({ return attrInput; } - // for cases other than rich text, boolean and file - // we can skip attribute - if (!attr.value[0]) { - return attrInput; - } if (inputType === AttributeInputTypeEnum.REFERENCE) { attrInput.push({ id: attr.id, @@ -262,6 +281,7 @@ export const prepareAttributesInput = ({ return attrInput; }, []); +}; export const handleUploadMultipleFiles = async ( attributesWithNewFileValue: FormsetData, diff --git a/src/pages/views/PageCreate.tsx b/src/pages/views/PageCreate.tsx index 59a5cb4b3..215197dde 100644 --- a/src/pages/views/PageCreate.tsx +++ b/src/pages/views/PageCreate.tsx @@ -119,6 +119,7 @@ export const PageCreate: React.FC = ({ params }) => { input: { attributes: prepareAttributesInput({ attributes: formData.attributes, + prevAttributes: null, updatedFileAttributes }), content: getParsedDataForJsonStringField(formData.content), diff --git a/src/pages/views/PageDetails.tsx b/src/pages/views/PageDetails.tsx index e91777d14..fefa8ef3a 100644 --- a/src/pages/views/PageDetails.tsx +++ b/src/pages/views/PageDetails.tsx @@ -19,6 +19,7 @@ import { import { AttributeErrorFragment, AttributeValueInput, + PageDetailsFragment, PageErrorFragment, PageInput, UploadErrorFragment, @@ -46,6 +47,7 @@ import { getStringOrPlaceholder, maybe } from "../../misc"; import PageDetailsPage from "../components/PageDetailsPage"; import { PageData, PageSubmitData } from "../components/PageDetailsPage/form"; import { pageListUrl, pageUrl, PageUrlQueryParams } from "../urls"; +import { getAttributeInputFromPage } from "../utils/data"; export interface PageDetailsProps { id: string; @@ -54,10 +56,12 @@ export interface PageDetailsProps { const createPageInput = ( data: PageData, + page: PageDetailsFragment, updatedFileAttributes: AttributeValueInput[] ): PageInput => ({ attributes: prepareAttributesInput({ attributes: data.attributes, + prevAttributes: getAttributeInputFromPage(page), updatedFileAttributes }), content: getParsedDataForJsonStringField(data.content), @@ -138,7 +142,11 @@ export const PageDetails: React.FC = ({ id, params }) => { const updateResult = await pageUpdate({ variables: { id, - input: createPageInput(data, updatedFileAttributes), + input: createPageInput( + data, + pageDetails?.data?.page, + updatedFileAttributes + ), firstValues: VALUES_PAGINATE_BY } }); diff --git a/src/products/views/ProductCreate/handlers.ts b/src/products/views/ProductCreate/handlers.ts index 28fb4e053..e57548378 100644 --- a/src/products/views/ProductCreate/handlers.ts +++ b/src/products/views/ProductCreate/handlers.ts @@ -102,6 +102,7 @@ export function createHandler( input: { attributes: prepareAttributesInput({ attributes: formData.attributes, + prevAttributes: null, updatedFileAttributes }), category: formData.category, diff --git a/src/products/views/ProductUpdate/handlers/index.ts b/src/products/views/ProductUpdate/handlers/index.ts index 3e5be7b47..d85479633 100644 --- a/src/products/views/ProductUpdate/handlers/index.ts +++ b/src/products/views/ProductUpdate/handlers/index.ts @@ -39,7 +39,10 @@ import { VariantCreateMutationVariables } from "@saleor/graphql"; import { ProductUpdatePageSubmitData } from "@saleor/products/components/ProductUpdatePage"; -import { mapFormsetStockToStockInput } from "@saleor/products/utils/data"; +import { + getAttributeInputFromProduct, + mapFormsetStockToStockInput +} from "@saleor/products/utils/data"; import { ReorderEvent } from "@saleor/types"; import { move } from "@saleor/utils/lists"; import { getParsedDataForJsonStringField } from "@saleor/utils/richText/misc"; @@ -116,6 +119,7 @@ export function createUpdateHandler( input: { attributes: prepareAttributesInput({ attributes: data.attributes, + prevAttributes: getAttributeInputFromProduct(product), updatedFileAttributes }), category: data.category, diff --git a/src/products/views/ProductVariant.tsx b/src/products/views/ProductVariant.tsx index f9ff9d412..29d70a32f 100644 --- a/src/products/views/ProductVariant.tsx +++ b/src/products/views/ProductVariant.tsx @@ -36,6 +36,7 @@ import useNotifier from "@saleor/hooks/useNotifier"; import useOnSetDefaultVariant from "@saleor/hooks/useOnSetDefaultVariant"; import useShop from "@saleor/hooks/useShop"; import { commonMessages } from "@saleor/intl"; +import { getAttributeInputFromVariant } from "@saleor/products/utils/data"; import usePageSearch from "@saleor/searches/usePageSearch"; import useProductSearch from "@saleor/searches/useProductSearch"; import useAttributeValueSearchHandler from "@saleor/utils/handlers/attributeValueSearchHandler"; @@ -267,6 +268,7 @@ export const ProductVariant: React.FC = ({ addStocks: data.addStocks.map(mapFormsetStockToStockInput), attributes: prepareAttributesInput({ attributes: data.attributes, + prevAttributes: getAttributeInputFromVariant(variant), updatedFileAttributes }), id: variantId, diff --git a/src/products/views/ProductVariantCreate.tsx b/src/products/views/ProductVariantCreate.tsx index fe29a8de7..9101dfd36 100644 --- a/src/products/views/ProductVariantCreate.tsx +++ b/src/products/views/ProductVariantCreate.tsx @@ -121,6 +121,7 @@ export const ProductVariant: React.FC = ({ attributes: formData.attributes.filter( attribute => attribute.value?.length && attribute.value[0] !== "" ), + prevAttributes: null, updatedFileAttributes }), product: productId,