diff --git a/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts b/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts index 3b5f77e94f..72acb05824 100644 --- a/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts +++ b/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts @@ -12,10 +12,10 @@ import { ProductStatus, } from "@medusajs/framework/utils" import { - ProductImage, Product, ProductCategory, ProductCollection, + ProductImage, ProductType, } from "@models" @@ -180,6 +180,22 @@ moduleIntegrationTestRunner({ productTwo = res[1] }) + it("should update multiple products", async () => { + await service.upsertProducts([ + { id: productOne.id, title: "updated title 1" }, + { id: productTwo.id, title: "updated title 2" }, + ]) + + const products = await service.listProducts( + { id: [productOne.id, productTwo.id] }, + { relations: ["*"] } + ) + + expect(products).toHaveLength(2) + expect(products[0].title).toEqual("updated title 1") + expect(products[1].title).toEqual("updated title 2") + }) + it("should update a product and upsert relations that are not created yet", async () => { const tags = await service.createProductTags([{ value: "tag-1" }]) const data = buildProductAndRelationsData({ @@ -400,9 +416,7 @@ moduleIntegrationTestRunner({ options: { size: "x", color: "red" }, // update options }, { - id: existingVariant2.id, - title: "new variant 2", - options: { size: "l", color: "green" }, // just preserve old one + id: existingVariant2.id, // just preserve old one }, { product_id: product.id, @@ -722,30 +736,6 @@ moduleIntegrationTestRunner({ expect(error).toEqual(`Product with id: does-not-exist was not found`) }) - it("should throw because variant doesn't have all options set", async () => { - const error = await service - .createProducts([ - { - title: "Product with variants and options", - options: [ - { title: "opt1", values: ["1", "2"] }, - { title: "opt2", values: ["3", "4"] }, - ], - variants: [ - { - title: "missing option", - options: { opt1: "1" }, - }, - ], - }, - ]) - .catch((e) => e) - - expect(error.message).toEqual( - `Product "Product with variants and options" has variants with missing options: [missing option]` - ) - }) - it("should update, create and delete variants", async () => { const updateData = { id: productTwo.id, @@ -849,6 +839,148 @@ moduleIntegrationTestRunner({ ]) ) }) + + it("should simultaneously update options and variants", async () => { + const updateData = { + id: productTwo.id, + options: [{ title: "material", values: ["cotton", "silk"] }], + variants: [{ title: "variant 1", options: { material: "cotton" } }], + } + + await service.upsertProducts([updateData]) + + const product = await service.retrieveProduct(productTwo.id, { + relations: ["*"], + }) + + expect(product.options).toHaveLength(1) + expect(product.options[0].title).toEqual("material") + expect(product.options[0].values).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + value: "cotton", + }), + expect.objectContaining({ + value: "silk", + }), + ]) + ) + + expect(product.variants).toHaveLength(1) + expect(product.variants[0].options).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + value: "cotton", + }), + ]) + ) + }) + + it("should throw an error when some tag id does not exist", async () => { + const error = await service + .updateProducts(productOne.id, { + tag_ids: ["does-not-exist"], + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `You tried to set relationship product_tag_id: does-not-exist, but such entity does not exist` + ) + }) + + it("should throw an error when some category id does not exist", async () => { + const error = await service + .updateProducts(productOne.id, { + category_ids: ["does-not-exist"], + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `You tried to set relationship product_category_id: does-not-exist, but such entity does not exist` + ) + }) + + it("should throw an error when collection id does not exist", async () => { + const error = await service + .updateProducts(productOne.id, { + collection_id: "does-not-exist", + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `You tried to set relationship collection_id: does-not-exist, but such entity does not exist` + ) + }) + + it("should throw an error when type id does not exist", async () => { + const error = await service + .updateProducts(productOne.id, { + type_id: "does-not-exist", + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `You tried to set relationship type_id: does-not-exist, but such entity does not exist` + ) + }) + + it("should throw if two variants have the same options combination", async () => { + const error = await service + .updateProducts(productTwo.id, { + variants: [ + { + title: "variant 1", + options: { size: "small", color: "blue" }, + }, + { + title: "variant 2", + options: { size: "small", color: "blue" }, + }, + ], + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `Variant "variant 1" has same combination of option values as "variant 2".` + ) + }) + + it("should throw if a variant doesn't have all options set", async () => { + const error = await service + .updateProducts(productTwo.id, { + variants: [ + { + title: "variant 1", + options: { size: "small" }, + }, + ], + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `Product has 2 option values but there were 1 provided option values for the variant: variant 1.` + ) + }) + + it("should throw if a variant uses a non-existing option", async () => { + const error = await service + .updateProducts(productTwo.id, { + variants: [ + { + title: "variant 1", + options: { + size: "small", + non_existing_option: "non_existing_value", + }, + }, + ], + }) + .catch((e) => e) + + expect(error?.message).toEqual( + `Option value non_existing_value does not exist for option non_existing_option` + ) + }) }) describe("create", function () { @@ -963,6 +1095,30 @@ moduleIntegrationTestRunner({ } ) }) + + it("should throw because variant doesn't have all options set", async () => { + const error = await service + .createProducts([ + { + title: "Product with variants and options", + options: [ + { title: "opt1", values: ["1", "2"] }, + { title: "opt2", values: ["3", "4"] }, + ], + variants: [ + { + title: "missing option", + options: { opt1: "1" }, + }, + ], + }, + ]) + .catch((e) => e) + + expect(error.message).toEqual( + `Product "Product with variants and options" has variants with missing options: [missing option]` + ) + }) }) describe("softDelete", function () { @@ -1408,6 +1564,28 @@ moduleIntegrationTestRunner({ ]) }) + it("should delete images if empty array is passed on update", async () => { + const images = [ + { url: "image-1" }, + { url: "image-2" }, + { url: "image-3" }, + ] + + const [product] = await service.createProducts([ + buildProductAndRelationsData({ images }), + ]) + + await service.updateProducts(product.id, { + images: [], + }) + + const productAfterUpdate = await service.retrieveProduct(product.id, { + relations: ["*"], + }) + + expect(productAfterUpdate.images).toHaveLength(0) + }) + it("should retrieve images in the correct order consistently", async () => { const images = Array.from({ length: 1000 }, (_, i) => ({ url: `image-${i + 1}`, diff --git a/packages/modules/product/src/repositories/product.ts b/packages/modules/product/src/repositories/product.ts index 3b44d82a74..dc245f46d4 100644 --- a/packages/modules/product/src/repositories/product.ts +++ b/packages/modules/product/src/repositories/product.ts @@ -1,8 +1,8 @@ -import { Product } from "@models" +import { Product, ProductOption } from "@models" -import { Context, DAL } from "@medusajs/framework/types" -import { DALUtils } from "@medusajs/framework/utils" -import { SqlEntityManager } from "@mikro-orm/postgresql" +import { Context, DAL, InferEntityType } from "@medusajs/framework/types" +import { buildQuery, DALUtils } from "@medusajs/framework/utils" +import { SqlEntityManager, wrap } from "@mikro-orm/postgresql" // eslint-disable-next-line max-len export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory( @@ -13,6 +13,68 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory( super(...arguments) } + async deepUpdate( + updates: any[], + validateVariantOptions: ( + variants: any[], + options: InferEntityType[] + ) => void, + context: Context = {} + ): Promise[]> { + const products = await this.find( + buildQuery({ id: updates.map((p) => p.id) }, { relations: ["*"] }), + context + ) + const productsMap = new Map(products.map((p) => [p.id, p])) + + for (const update of updates) { + const product = productsMap.get(update.id)! + + // Assign the options first, so they'll be available for the variants loop below + if (update.options) { + wrap(product).assign({ options: update.options }) + delete update.options // already assigned above, so no longer necessary + } + + if (update.variants) { + validateVariantOptions(update.variants, product.options) + + update.variants.forEach((variant: any) => { + if (variant.options) { + variant.options = Object.entries(variant.options).map( + ([key, value]) => { + const productOption = product.options.find( + (option) => option.title === key + )! + const productOptionValue = productOption.values?.find( + (optionValue) => optionValue.value === value + )! + return productOptionValue.id + } + ) + } + }) + } + + if (update.tags) { + update.tags = update.tags.map((t: { id: string }) => t.id) + } + if (update.categories) { + update.categories = update.categories.map((c: { id: string }) => c.id) + } + if (update.images) { + update.images = update.images.map((image: any, index: number) => ({ + ...image, + rank: index, + })) + } + + wrap(product!).assign(update) + } + + return products + } + /** * In order to be able to have a strict not in categories, and prevent a product * to be return in the case it also belongs to other categories, we need to diff --git a/packages/modules/product/src/services/product-module-service.ts b/packages/modules/product/src/services/product-module-service.ts index cbf36f4706..0ad9b57395 100644 --- a/packages/modules/product/src/services/product-module-service.ts +++ b/packages/modules/product/src/services/product-module-service.ts @@ -39,10 +39,10 @@ import { MedusaService, Modules, ProductStatus, - promiseAll, removeUndefined, toHandle, } from "@medusajs/framework/utils" +import { ProductRepository } from "../repositories" import { UpdateCategoryInput, UpdateCollectionInput, @@ -57,6 +57,7 @@ import { joinerConfig } from "./../joiner-config" type InjectedDependencies = { baseRepository: DAL.RepositoryService + productRepository: ProductRepository productService: ModulesSdkTypes.IMedusaInternalService productVariantService: ModulesSdkTypes.IMedusaInternalService productTagService: ModulesSdkTypes.IMedusaInternalService @@ -113,6 +114,7 @@ export default class ProductModuleService implements ProductTypes.IProductModuleService { protected baseRepository_: DAL.RepositoryService + protected readonly productRepository_: ProductRepository protected readonly productService_: ModulesSdkTypes.IMedusaInternalService< InferEntityType > @@ -143,6 +145,7 @@ export default class ProductModuleService constructor( { baseRepository, + productRepository, productService, productVariantService, productTagService, @@ -161,6 +164,7 @@ export default class ProductModuleService super(...arguments) this.baseRepository_ = baseRepository + this.productRepository_ = productRepository this.productService_ = productService this.productVariantService_ = productVariantService this.productTagService_ = productTagService @@ -1661,124 +1665,11 @@ export default class ProductModuleService this.validateProductUpdatePayload(product) } - const { entities: productData } = - await this.productService_.upsertWithReplace( - normalizedProducts, - { - relations: ["tags", "categories"], - }, - sharedContext - ) - - // There is more than 1-level depth of relations here, so we need to handle the options and variants manually - await promiseAll( - // Note: It's safe to rely on the order here as `upsertWithReplace` preserves the order of the input - normalizedProducts.map(async (product, i) => { - const upsertedProduct: any = productData[i] - let allOptions: any[] = [] - - if (product.options?.length) { - const { entities: productOptions } = - await this.productOptionService_.upsertWithReplace( - product.options?.map((option) => ({ - ...option, - product_id: upsertedProduct.id, - })) ?? [], - { relations: ["values"] }, - sharedContext - ) - upsertedProduct.options = productOptions - - // Since we handle the options and variants outside of the product upsert, we need to clean up manually - await this.productOptionService_.delete( - { - product_id: upsertedProduct.id, - id: { - $nin: upsertedProduct.options.map(({ id }) => id), - }, - }, - sharedContext - ) - allOptions = upsertedProduct.options - } else { - // If the options weren't affected, but the user is changing the variants, make sure we have all options available locally - if (product.variants?.length) { - allOptions = await this.productOptionService_.list( - { product_id: upsertedProduct.id }, - { relations: ["values"] }, - sharedContext - ) - } - } - - if (product.variants?.length) { - const productVariantsWithOptions = - ProductModuleService.assignOptionsToVariants( - product.variants.map((v) => ({ - ...v, - product_id: upsertedProduct.id, - })) ?? [], - allOptions - ) - - ProductModuleService.checkIfVariantsHaveUniqueOptionsCombinations( - productVariantsWithOptions as any - ) - - const { entities: productVariants } = - await this.productVariantService_.upsertWithReplace( - productVariantsWithOptions, - { relations: ["options"] }, - sharedContext - ) - - upsertedProduct.variants = productVariants - - await this.productVariantService_.delete( - { - product_id: upsertedProduct.id, - id: { - $nin: upsertedProduct.variants.map(({ id }) => id), - }, - }, - sharedContext - ) - } - - if (Array.isArray(product.images)) { - if (product.images.length) { - const { entities: productImages } = - await this.productImageService_.upsertWithReplace( - product.images.map((image, rank) => ({ - ...image, - product_id: upsertedProduct.id, - rank, - })), - {}, - sharedContext - ) - upsertedProduct.images = productImages - - await this.productImageService_.delete( - { - product_id: upsertedProduct.id, - id: { - $nin: productImages.map(({ id }) => id), - }, - }, - sharedContext - ) - } else { - await this.productImageService_.delete( - { product_id: upsertedProduct.id }, - sharedContext - ) - } - } - }) + return this.productRepository_.deepUpdate( + normalizedProducts, + ProductModuleService.validateVariantOptions, + sharedContext ) - - return productData } // @ts-expect-error @@ -2052,6 +1943,26 @@ export default class ProductModuleService return collectionData } + protected static validateVariantOptions( + variants: + | ProductTypes.CreateProductVariantDTO[] + | ProductTypes.UpdateProductVariantDTO[], + options: InferEntityType[] + ) { + const variantsWithOptions = ProductModuleService.assignOptionsToVariants( + variants.map((v) => ({ + ...v, + // adding product_id to the variant to make it valid for the assignOptionsToVariants function + ...(options.length ? { product_id: options[0].product_id } : {}), + })), + options + ) + + ProductModuleService.checkIfVariantsHaveUniqueOptionsCombinations( + variantsWithOptions as any + ) + } + protected static assignOptionsToVariants( variants: | ProductTypes.CreateProductVariantDTO[] @@ -2063,7 +1974,6 @@ export default class ProductModuleService if (!variants.length) { return variants } - const variantsWithOptions = variants.map((variant: any) => { const numOfProvidedVariantOptionValues = Object.keys( variant.options || {} @@ -2079,7 +1989,7 @@ export default class ProductModuleService ) { throw new MedusaError( MedusaError.Types.INVALID_DATA, - `Product has ${productsOptions.length} but there were ${numOfProvidedVariantOptionValues} provided option values for the variant: ${variant.title}.` + `Product has ${productsOptions.length} option values but there were ${numOfProvidedVariantOptionValues} provided option values for the variant: ${variant.title}.` ) }