From 0741d3ca2cb2a52a298f54e4e9125dd95b474b64 Mon Sep 17 00:00:00 2001 From: Dawid Date: Tue, 6 Dec 2022 12:03:41 +0100 Subject: [PATCH] Improve saving variant images (#2640) * Improve saving variant images * Update storybook types * Add getSelectedMedia function for products * Add test for handleAssignMedia in products * Follow AAA test convention * Add confirm button to media dialog selector * Move dialog styles to separate file --- .../ProductVariantMediaSelectDialog.tsx | 92 +++++----- .../ProductVariantImageSelectDialog/styles.ts | 42 +++++ .../ProductVariantPage/ProductVariantPage.tsx | 40 ++--- .../components/ProductVariantPage/form.tsx | 7 + src/products/utils/data.test.ts | 78 +++++++++ src/products/utils/data.ts | 11 ++ src/products/utils/handlers.test.ts | 163 ++++++++++++++++++ src/products/utils/handlers.ts | 77 ++++++++- .../views/ProductVariant/ProductVariant.tsx | 30 +--- .../ProductVariantImageSelectDialog.tsx | 2 +- .../stories/products/ProductVariantPage.tsx | 4 - 11 files changed, 451 insertions(+), 95 deletions(-) create mode 100644 src/products/components/ProductVariantImageSelectDialog/styles.ts create mode 100644 src/products/utils/data.test.ts create mode 100644 src/products/utils/handlers.test.ts diff --git a/src/products/components/ProductVariantImageSelectDialog/ProductVariantMediaSelectDialog.tsx b/src/products/components/ProductVariantImageSelectDialog/ProductVariantMediaSelectDialog.tsx index 85112516b..e2263d871 100644 --- a/src/products/components/ProductVariantImageSelectDialog/ProductVariantMediaSelectDialog.tsx +++ b/src/products/components/ProductVariantImageSelectDialog/ProductVariantMediaSelectDialog.tsx @@ -5,65 +5,58 @@ import { DialogTitle, } from "@material-ui/core"; import BackButton from "@saleor/components/BackButton"; +import ConfirmButton from "@saleor/components/ConfirmButton"; import { ProductMediaFragment } from "@saleor/graphql"; -import { makeStyles } from "@saleor/macaw-ui"; +import useModalDialogOpen from "@saleor/hooks/useModalDialogOpen"; +import { buttonMessages } from "@saleor/intl"; import clsx from "clsx"; -import React from "react"; +import React, { useState } from "react"; import { FormattedMessage } from "react-intl"; -const useStyles = makeStyles( - theme => ({ - image: { - height: "100%", - objectFit: "contain", - userSelect: "none", - width: "100%", - }, - imageContainer: { - background: "transparent", - border: "1px solid #eaeaea", - borderRadius: theme.spacing(), - cursor: "pointer", - height: theme.spacing(21.5), - overflow: "hidden", - padding: theme.spacing(2), - position: "relative", - transitionDuration: theme.transitions.duration.standard + "ms", - }, - content: { - overflowY: "scroll", - }, - root: { - display: "grid", - gridColumnGap: theme.spacing(2), - gridRowGap: theme.spacing(2), - gridTemplateColumns: "repeat(3, 1fr)", - maxWidth: "100%", - width: theme.breakpoints.values.lg, - [theme.breakpoints.down("sm")]: { - gridTemplateColumns: "repeat(2, 1fr)", - }, - }, - selectedImageContainer: { - borderColor: theme.palette.primary.main, - borderWidth: "2px", - }, - }), - { name: "ProductVariantImageSelectDialog" }, -); +import { useStyles } from "./styles"; interface ProductVariantImageSelectDialogProps { media?: ProductMediaFragment[]; selectedMedia?: string[]; open: boolean; - onClose(); - onMediaSelect(id: string); + onClose: () => void; + onConfirm: (selectedIds: string[]) => void; } const ProductVariantMediaSelectDialog: React.FC = props => { - const { media, open, selectedMedia, onClose, onMediaSelect } = props; + const { + media, + open, + selectedMedia: initialMedia, + onClose, + onConfirm, + } = props; const classes = useStyles(props); + const [selectedMedia, setSelectedMedia] = useState(initialMedia); + + useModalDialogOpen(open, { + onOpen: () => setSelectedMedia(initialMedia), + onClose: () => setSelectedMedia(initialMedia), + }); + + const handleMediaSelect = (id: string) => { + const isMediaAssigned = selectedMedia.includes(id); + + if (isMediaAssigned) { + setSelectedMedia(selectedMedia => + selectedMedia.filter(mediaId => mediaId !== id), + ); + } else { + setSelectedMedia(selectedMedia => [...selectedMedia, id]); + } + }; + + const handleConfirm = () => { + onConfirm(selectedMedia); + onClose(); + }; + return ( @@ -90,7 +83,7 @@ const ProductVariantMediaSelectDialog: React.FC handleMediaSelect(mediaObj.id)} key={mediaObj.id} > + + + ); diff --git a/src/products/components/ProductVariantImageSelectDialog/styles.ts b/src/products/components/ProductVariantImageSelectDialog/styles.ts new file mode 100644 index 000000000..8aedf65ca --- /dev/null +++ b/src/products/components/ProductVariantImageSelectDialog/styles.ts @@ -0,0 +1,42 @@ +import { makeStyles } from "@saleor/macaw-ui"; + +export const useStyles = makeStyles( + theme => ({ + image: { + height: "100%", + objectFit: "contain", + userSelect: "none", + width: "100%", + }, + imageContainer: { + background: "transparent", + border: "1px solid #eaeaea", + borderRadius: theme.spacing(), + cursor: "pointer", + height: theme.spacing(21.5), + overflow: "hidden", + padding: theme.spacing(2), + position: "relative", + transitionDuration: theme.transitions.duration.standard + "ms", + }, + content: { + overflowY: "scroll", + }, + root: { + display: "grid", + gridColumnGap: theme.spacing(2), + gridRowGap: theme.spacing(2), + gridTemplateColumns: "repeat(3, 1fr)", + maxWidth: "100%", + width: theme.breakpoints.values.lg, + [theme.breakpoints.down("sm")]: { + gridTemplateColumns: "repeat(2, 1fr)", + }, + }, + selectedImageContainer: { + borderColor: theme.palette.primary.main, + borderWidth: "2px", + }, + }), + { name: "ProductVariantImageSelectDialog" }, +); diff --git a/src/products/components/ProductVariantPage/ProductVariantPage.tsx b/src/products/components/ProductVariantPage/ProductVariantPage.tsx index 7f8fffb36..feb3b9ed9 100644 --- a/src/products/components/ProductVariantPage/ProductVariantPage.tsx +++ b/src/products/components/ProductVariantPage/ProductVariantPage.tsx @@ -29,6 +29,7 @@ import useNavigator from "@saleor/hooks/useNavigator"; import { ConfirmButtonTransitionState } from "@saleor/macaw-ui"; import { VariantDetailsChannelsAvailabilityCard } from "@saleor/products/components/ProductVariantChannels/ChannelsAvailabilityCard"; import { productUrl } from "@saleor/products/urls"; +import { getSelectedMedia } from "@saleor/products/utils/data"; import { FetchMoreProps, RelayToFlat, ReorderAction } from "@saleor/types"; import React from "react"; import { defineMessages, useIntl } from "react-intl"; @@ -118,7 +119,6 @@ interface ProductVariantPageProps { onAttributeSelectBlur: () => void; onDelete(); onSubmit(data: ProductVariantUpdateSubmitData); - onMediaSelect(id: string); onSetDefaultVariant(); onWarehouseConfigure(); } @@ -140,7 +140,6 @@ const ProductVariantPage: React.FC = ({ referenceProducts = [], attributeValues, onDelete, - onMediaSelect, onSubmit, onVariantPreorderDeactivate, variantDeactivatePreoderButtonState, @@ -172,13 +171,9 @@ const ProductVariantPage: React.FC = ({ setIsEndPreorderModalOpened, ] = React.useState(false); - const variantMedia = variant?.media?.map(image => image.id); const productMedia = [ ...(variant?.product?.media ?? []), ]?.sort((prev, next) => (prev.sortOrder > next.sortOrder ? 1 : -1)); - const media = productMedia - ?.filter(image => variantMedia.indexOf(image.id) !== -1) - .sort((prev, next) => (prev.sortOrder > next.sortOrder ? 1 : -1)); const canOpenAssignReferencesAttributeDialog = !!assignReferencesAttributeId; @@ -246,6 +241,7 @@ const ProductVariantPage: React.FC = ({ const selectionAttributes = data.attributes.filter( byAttributeScope(VariantAttributeScope.VARIANT_SELECTION), ); + const media = getSelectedMedia(productMedia, data.media); const errors = [...apiErrors, ...validationErrors]; @@ -423,28 +419,28 @@ const ProductVariantPage: React.FC = ({ /> )} {variant && ( - + <> + + + )} ); }} - {variant && ( - image.id)} - /> - )} {!!variant?.preorder && ( void> { changePreorderEndDate: FormChange; changeMetadata: FormChange; + changeMedia: (ids: string[]) => void; updateChannels: (selectedChannelsIds: string[]) => void; fetchReferences: (value: string) => void; fetchMoreReferences: FetchMoreProps; @@ -191,6 +194,7 @@ function useProductVariantUpdateForm( weight: variant?.weight?.value.toString() || "", quantityLimitPerCustomer: variant?.quantityLimitPerCustomer || null, name: variant?.name ?? "", + media: variant?.media?.map(({ id }) => id) || [], }; const form = useForm(initial, undefined, { @@ -300,6 +304,8 @@ function useProductVariantUpdateForm( intl.formatMessage(errorMessages.preorderEndDateInFutureErrorText), ); + const handleMediaChange = createMediaChangeHandler(form, triggerChange); + const handleUpdateChannels = (selectedIds: string[]) => { const allChannels = variant.product.channelListings.map(listing => { const variantChannel = variant?.channelListings?.find( @@ -428,6 +434,7 @@ function useProductVariantUpdateForm( changeMetadata, changeStock: handleStockChange, changePreorderEndDate: handlePreorderEndDateChange, + changeMedia: handleMediaChange, deleteStock: handleStockDelete, fetchMoreReferences: handleFetchMoreReferences, fetchReferences: handleFetchReferences, diff --git a/src/products/utils/data.test.ts b/src/products/utils/data.test.ts new file mode 100644 index 000000000..24893394c --- /dev/null +++ b/src/products/utils/data.test.ts @@ -0,0 +1,78 @@ +import { getSelectedMedia } from "./data"; + +type GetSelectedMediaParams = Parameters; + +describe("Product media utils", () => { + it("should return selected media in proper order when media ids passed", () => { + // Arrange + const media: GetSelectedMediaParams[0] = [ + { + id: "1", + sortOrder: 2, + }, + { + id: "2", + sortOrder: 3, + }, + { + id: "3", + sortOrder: 4, + }, + { + id: "4", + sortOrder: 1, + }, + ]; + const selectedIds: GetSelectedMediaParams[1] = ["1", "3", "4"]; + + // Act + const result = getSelectedMedia(media, selectedIds); + + // Assert + const expectedResult = [ + { + id: "4", + sortOrder: 1, + }, + { + id: "1", + sortOrder: 2, + }, + { + id: "3", + sortOrder: 4, + }, + ]; + + expect(result).toEqual(expectedResult); + }); + + it("should return empty array of media when no media ids passed", () => { + // Arrange + const media: GetSelectedMediaParams[0] = [ + { + id: "1", + sortOrder: 2, + }, + { + id: "2", + sortOrder: 3, + }, + { + id: "3", + sortOrder: 4, + }, + { + id: "4", + sortOrder: 1, + }, + ]; + const selectedIds: GetSelectedMediaParams[1] = []; + + // Act + const result = getSelectedMedia(media, selectedIds); + + // Assert + expect(result).toEqual([]); + }); +}); diff --git a/src/products/utils/data.ts b/src/products/utils/data.ts index 232211098..4125b21b2 100644 --- a/src/products/utils/data.ts +++ b/src/products/utils/data.ts @@ -11,6 +11,7 @@ import { SingleAutocompleteChoiceType } from "@saleor/components/SingleAutocompl import { ProductDetailsVariantFragment, ProductFragment, + ProductMediaFragment, ProductTypeQuery, ProductVariantCreateDataQuery, ProductVariantFragment, @@ -247,3 +248,13 @@ export const getPreorderEndDateFormData = (endDate?: string) => export const getPreorderEndHourFormData = (endDate?: string) => endDate ? moment(endDate).format("HH:mm") : ""; + +export const getSelectedMedia = < + T extends Pick +>( + media: T[] = [], + selectedMediaIds: string[], +) => + media + .filter(image => selectedMediaIds.indexOf(image.id) !== -1) + .sort((prev, next) => (prev.sortOrder > next.sortOrder ? 1 : -1)); diff --git a/src/products/utils/handlers.test.ts b/src/products/utils/handlers.test.ts new file mode 100644 index 000000000..db0144617 --- /dev/null +++ b/src/products/utils/handlers.test.ts @@ -0,0 +1,163 @@ +import { ProductMediaType } from "@saleor/graphql"; + +import { handleAssignMedia } from "./handlers"; + +type HandleAssignMediaParams = Parameters; + +describe("Product handlers", () => { + it("should not alter product variant media when the same selected media ids as previously passed", async () => { + // Arrange + const media: HandleAssignMediaParams[0] = ["1", "2"]; + const variant: HandleAssignMediaParams[1] = { + id: "1", + media: [ + { + id: "1", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + { + id: "2", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + ], + }; + const assignMedia = jest.fn(() => Promise.resolve({})); + const unassignMedia = jest.fn(() => Promise.resolve({})); + + // Act + await handleAssignMedia(media, variant, assignMedia, unassignMedia); + + // Assert + expect(assignMedia).not.toHaveBeenCalled(); + expect(unassignMedia).not.toHaveBeenCalled(); + }); + + it("should assign media to product variant when more then all previous selected media ids passed", async () => { + // Arrange + const media: HandleAssignMediaParams[0] = ["1", "2", "3"]; + const variant: HandleAssignMediaParams[1] = { + id: "1", + media: [ + { + id: "3", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + ], + }; + const assignMedia = jest.fn(() => Promise.resolve({})); + const unassignMedia = jest.fn(() => Promise.resolve({})); + + // Act + await handleAssignMedia(media, variant, assignMedia, unassignMedia); + + // Assert + expect(assignMedia).toHaveBeenCalledTimes(2); + expect(assignMedia).toHaveBeenCalledWith({ + variantId: "1", + mediaId: "1", + }); + expect(assignMedia).toHaveBeenCalledWith({ + variantId: "1", + mediaId: "2", + }); + expect(unassignMedia).not.toHaveBeenCalled(); + }); + + it("should unassign media from product variant when not all previous selected media ids passed", async () => { + // Arrange + const media: HandleAssignMediaParams[0] = ["3"]; + const variant: HandleAssignMediaParams[1] = { + id: "1", + media: [ + { + id: "1", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + { + id: "2", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + { + id: "3", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + ], + }; + const assignMedia = jest.fn(() => Promise.resolve({})); + const unassignMedia = jest.fn(() => Promise.resolve({})); + + // Act + await handleAssignMedia(media, variant, assignMedia, unassignMedia); + + // Assert + expect(assignMedia).not.toHaveBeenCalled(); + expect(unassignMedia).toHaveBeenCalledTimes(2); + expect(unassignMedia).toHaveBeenCalledWith({ + variantId: "1", + mediaId: "1", + }); + expect(unassignMedia).toHaveBeenCalledWith({ + variantId: "1", + mediaId: "2", + }); + }); + + it("should assign and unassign media from product variant when not all but more selected media ids from previously selected passed", async () => { + // Arrange + const media: HandleAssignMediaParams[0] = ["1", "3"]; + const variant: HandleAssignMediaParams[1] = { + id: "1", + media: [ + { + id: "1", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + { + id: "2", + url: "", + type: ProductMediaType.IMAGE, + oembedData: null, + __typename: "ProductMedia", + }, + ], + }; + const assignMedia = jest.fn(() => Promise.resolve({})); + const unassignMedia = jest.fn(() => Promise.resolve({})); + + // Act + await handleAssignMedia(media, variant, assignMedia, unassignMedia); + + // Assert + expect(assignMedia).toHaveBeenCalledTimes(1); + expect(assignMedia).toHaveBeenCalledWith({ + variantId: "1", + mediaId: "3", + }); + expect(unassignMedia).toHaveBeenCalledTimes(1); + expect(unassignMedia).toHaveBeenCalledWith({ + variantId: "1", + mediaId: "2", + }); + }); +}); diff --git a/src/products/utils/handlers.ts b/src/products/utils/handlers.ts index 0312410e7..be0b86ffc 100644 --- a/src/products/utils/handlers.ts +++ b/src/products/utils/handlers.ts @@ -1,11 +1,20 @@ +import { FetchResult } from "@apollo/client"; import { ChannelData, ChannelPriceAndPreorderData, ChannelPriceArgs, ChannelPriceData, } from "@saleor/channels/utils"; -import { ProductChannelListingAddInput } from "@saleor/graphql"; +import { + ProductChannelListingAddInput, + ProductVariantFragment, + VariantMediaAssignMutation, + VariantMediaAssignMutationVariables, + VariantMediaUnassignMutation, + VariantMediaUnassignMutationVariables, +} from "@saleor/graphql"; import { FormChange, UseFormResult } from "@saleor/hooks/useForm"; +import { diff } from "fast-array-diff"; import moment from "moment"; export function createChannelsPriceChangeHandler( @@ -144,3 +153,69 @@ export const createPreorderEndDateChangeHandler = ( } triggerChange(); }; + +export const createMediaChangeHandler = ( + form: UseFormResult<{ media: string[] }>, + triggerChange: () => void, +) => (ids: string[]) => { + form.change({ + target: { + name: "media", + value: ids, + }, + }); + + triggerChange(); +}; + +export const handleAssignMedia = async < + T extends Pick +>( + media: string[], + variant: T, + assignMedia: ( + variables: VariantMediaAssignMutationVariables, + ) => Promise>, + unassignMedia: ( + variables: VariantMediaUnassignMutationVariables, + ) => Promise>, +) => { + const { added, removed } = diff( + variant.media.map(mediaObj => mediaObj.id), + media, + ); + + const assignResults = await Promise.all( + added.map(mediaId => + assignMedia({ + mediaId, + variantId: variant.id, + }), + ), + ); + const unassignResults = await Promise.all( + removed.map(mediaId => + unassignMedia({ + mediaId, + variantId: variant.id, + }), + ), + ); + + const assignErrors = assignResults.reduce( + (errors, result) => [ + ...errors, + ...(result.data?.variantMediaAssign.errors || []), + ], + [], + ); + const unassignErrors = unassignResults.reduce( + (errors, result) => [ + ...errors, + ...(result.data?.variantMediaUnassign.errors || []), + ], + [], + ); + + return [...assignErrors, ...unassignErrors]; +}; diff --git a/src/products/views/ProductVariant/ProductVariant.tsx b/src/products/views/ProductVariant/ProductVariant.tsx index 3a759a54a..cd1b54af0 100644 --- a/src/products/views/ProductVariant/ProductVariant.tsx +++ b/src/products/views/ProductVariant/ProductVariant.tsx @@ -36,6 +36,7 @@ import useShop from "@saleor/hooks/useShop"; import { commonMessages } from "@saleor/intl"; import { weight } from "@saleor/misc"; import { getAttributeInputFromVariant } from "@saleor/products/utils/data"; +import { handleAssignMedia } from "@saleor/products/utils/handlers"; import usePageSearch from "@saleor/searches/usePageSearch"; import useProductSearch from "@saleor/searches/useProductSearch"; import useAttributeValueSearchHandler from "@saleor/utils/handlers/attributeValueSearchHandler"; @@ -181,26 +182,6 @@ export const ProductVariant: React.FC = ({ reorderProductVariantsOpts.loading || deleteAttributeValueOpts.loading; - const handleMediaSelect = (id: string) => () => { - if (variant) { - if (variant?.media?.map(media_obj => media_obj.id).indexOf(id) !== -1) { - unassignMedia({ - variables: { - mediaId: id, - variantId: variant.id, - }, - }); - } else { - assignMedia({ - variables: { - mediaId: id, - variantId: variant.id, - }, - }); - } - } - }; - const handleUpdate = async (data: ProductVariantUpdateSubmitData) => { const uploadFilesResult = await handleUploadMultipleFiles( data.attributesWithNewFileValue, @@ -218,6 +199,13 @@ export const ProductVariant: React.FC = ({ uploadFilesResult, ); + const assignMediaErrors = await handleAssignMedia( + data.media, + variant, + variables => assignMedia({ variables }), + variables => unassignMedia({ variables }), + ); + const result = await updateVariant({ variables: { addStocks: data.addStocks.map(mapFormsetStockToStockInput), @@ -255,6 +243,7 @@ export const ProductVariant: React.FC = ({ ...result.data?.productVariantStocksDelete.errors, ...result.data?.productVariantStocksUpdate.errors, ...result.data?.productVariantUpdate.errors, + ...assignMediaErrors, ...channelErrors, ]; }; @@ -340,7 +329,6 @@ export const ProductVariant: React.FC = ({ header={variant?.name || variant?.sku} warehouses={mapEdgesToItems(warehouses?.data?.warehouses) || []} onDelete={() => openModal("remove")} - onMediaSelect={handleMediaSelect} onSubmit={handleSubmit} onWarehouseConfigure={() => navigate(warehouseAddPath)} onVariantPreorderDeactivate={handleDeactivateVariantPreorder} diff --git a/src/storybook/stories/products/ProductVariantImageSelectDialog.tsx b/src/storybook/stories/products/ProductVariantImageSelectDialog.tsx index 9a075b987..a038952ca 100644 --- a/src/storybook/stories/products/ProductVariantImageSelectDialog.tsx +++ b/src/storybook/stories/products/ProductVariantImageSelectDialog.tsx @@ -19,7 +19,7 @@ storiesOf("Products / ProductVariantImageSelectDialog", module) media={variantProductImages} selectedMedia={variantImages.map(image => image.id)} onClose={() => undefined} - onMediaSelect={() => undefined} + onConfirm={() => undefined} open={true} /> )); diff --git a/src/storybook/stories/products/ProductVariantPage.tsx b/src/storybook/stories/products/ProductVariantPage.tsx index 4807190f8..9ae50e71c 100644 --- a/src/storybook/stories/products/ProductVariantPage.tsx +++ b/src/storybook/stories/products/ProductVariantPage.tsx @@ -25,7 +25,6 @@ storiesOf("Views / Products / Product variant details", module) variant={variant} onDelete={undefined} onSetDefaultVariant={() => undefined} - onMediaSelect={() => undefined} onSubmit={() => undefined} onVariantReorder={() => undefined} saveButtonBarState="default" @@ -54,7 +53,6 @@ storiesOf("Views / Products / Product variant details", module) placeholderImage={placeholderImage} onDelete={undefined} onSetDefaultVariant={() => undefined} - onMediaSelect={() => undefined} onSubmit={() => undefined} onVariantReorder={() => undefined} saveButtonBarState="default" @@ -82,7 +80,6 @@ storiesOf("Views / Products / Product variant details", module) variant={variant} onDelete={undefined} onSetDefaultVariant={() => undefined} - onMediaSelect={() => undefined} onSubmit={() => undefined} onVariantReorder={() => undefined} saveButtonBarState="default" @@ -108,7 +105,6 @@ storiesOf("Views / Products / Product variant details", module) variant={variant} onDelete={undefined} onSetDefaultVariant={() => undefined} - onMediaSelect={() => undefined} onSubmit={() => undefined} onVariantReorder={() => undefined} saveButtonBarState="default"