diff --git a/.changeset/new-tips-mate.md b/.changeset/new-tips-mate.md new file mode 100644 index 0000000000..987c413425 --- /dev/null +++ b/.changeset/new-tips-mate.md @@ -0,0 +1,7 @@ +--- +"medusa-test-utils": patch +"@medusajs/medusa": patch +"integration-tests-api": patch +--- + +chores(medusa): Refactoring product update flow to improve handling and performances diff --git a/integration-tests/api/__tests__/batch-jobs/product/import.js b/integration-tests/api/__tests__/batch-jobs/product/import.js index 2acb346675..9ad2e15a34 100644 --- a/integration-tests/api/__tests__/batch-jobs/product/import.js +++ b/integration-tests/api/__tests__/batch-jobs/product/import.js @@ -81,7 +81,7 @@ describe("Product import batch job", () => { await batchJobSeeder(dbConnection) await adminSeeder(dbConnection) await userSeeder(dbConnection) - await simpleProductCollectionFactory(dbConnection, [ + await simpleProductCollectionFactory(dbConnection, [ { handle: collectionHandle1, }, @@ -184,7 +184,7 @@ describe("Product import batch job", () => { ean: null, upc: null, inventory_quantity: 10, - prices: [ + prices: expect.arrayContaining([ expect.objectContaining({ currency_code: "eur", amount: 100, @@ -199,7 +199,7 @@ describe("Product import batch job", () => { amount: 130, region_id: "region-product-import-1", }), - ], + ]), options: expect.arrayContaining([ expect.objectContaining({ value: "option 1 value red", @@ -211,24 +211,24 @@ describe("Product import batch job", () => { }), ], type: null, - images: [ + images: expect.arrayContaining([ expect.objectContaining({ url: "test-image.png", }), - ], - options: [ + ]), + options: expect.arrayContaining([ expect.objectContaining({ title: "test-option-1", }), expect.objectContaining({ title: "test-option-2", }), - ], - tags: [ + ]), + tags: expect.arrayContaining([ expect.objectContaining({ value: "123_1", }), - ], + ]), collection: expect.objectContaining({ handle: collectionHandle1, }), @@ -250,7 +250,7 @@ describe("Product import batch job", () => { ean: null, upc: null, inventory_quantity: 10, - prices: [ + prices: expect.arrayContaining([ expect.objectContaining({ currency_code: "eur", amount: 100, @@ -265,7 +265,7 @@ describe("Product import batch job", () => { amount: 130, region_id: "region-product-import-1", }), - ], + ]), options: expect.arrayContaining([ expect.objectContaining({ value: "option 1 value red", @@ -277,19 +277,19 @@ describe("Product import batch job", () => { }), ], type: null, - images: [ + images: expect.arrayContaining([ expect.objectContaining({ url: "test-image.png", }), - ], - options: [ + ]), + options: expect.arrayContaining([ expect.objectContaining({ title: "test-option-1", }), expect.objectContaining({ title: "test-option-2", }), - ], + ]), tags: [], collection: expect.objectContaining({ handle: collectionHandle1, diff --git a/packages/medusa-test-utils/src/mock-manager.js b/packages/medusa-test-utils/src/mock-manager.js index 59599f4043..ba117cb338 100644 --- a/packages/medusa-test-utils/src/mock-manager.js +++ b/packages/medusa-test-utils/src/mock-manager.js @@ -4,6 +4,11 @@ export default { }, getCustomRepository: function (repo) { + if (repo) { + repo["metadata"] = repo["metadata"] ?? { + columns: [] + } + } return repo; }, diff --git a/packages/medusa-test-utils/src/mock-repository.js b/packages/medusa-test-utils/src/mock-repository.js index 2871f4497a..da81488b9d 100644 --- a/packages/medusa-test-utils/src/mock-repository.js +++ b/packages/medusa-test-utils/src/mock-repository.js @@ -13,25 +13,30 @@ class MockRepo { findAndCount, del, count, - insertBulk + insertBulk, + metadata }) { - this.create_ = create; - this.update_ = update; - this.remove_ = remove; - this.delete_ = del; - this.softRemove_ = softRemove; - this.find_ = find; - this.findDescendantsTree_ = findDescendantsTree; - this.findOne_ = findOne; - this.findOneOrFail_ = findOneOrFail; - this.save_ = save; - this.findAndCount_ = findAndCount; - this.findOneWithRelations_ = findOneWithRelations; - this.insertBulk_ = insertBulk; + this.create_ = create + this.update_ = update + this.remove_ = remove + this.delete_ = del + this.softRemove_ = softRemove + this.find_ = find + this.findDescendantsTree_ = findDescendantsTree + this.findOne_ = findOne + this.findOneOrFail_ = findOneOrFail + this.save_ = save + this.findAndCount_ = findAndCount + this.findOneWithRelations_ = findOneWithRelations + this.insertBulk_ = insertBulk + + this.metadata = metadata ?? { + columns: [] + } } setFindOne(fn) { - this.findOne_ = fn; + this.findOne_ = fn } insertBulk = jest.fn().mockImplementation((...args) => { @@ -42,83 +47,83 @@ class MockRepo { }) create = jest.fn().mockImplementation((...args) => { if (this.create_) { - return this.create_(...args); + return this.create_(...args) } - return {}; - }); + return {} + }) softRemove = jest.fn().mockImplementation((...args) => { if (this.softRemove_) { - return this.softRemove_(...args); + return this.softRemove_(...args) } - return {}; - }); + return {} + }) remove = jest.fn().mockImplementation((...args) => { if (this.remove_) { - return this.remove_(...args); + return this.remove_(...args) } - return {}; - }); + return {} + }) update = jest.fn().mockImplementation((...args) => { if (this.update_) { - return this.update_(...args); + return this.update_(...args) } - }); + }) findOneOrFail = jest.fn().mockImplementation((...args) => { if (this.findOneOrFail_) { - return this.findOneOrFail_(...args); + return this.findOneOrFail_(...args) } - }); + }) findOneWithRelations = jest.fn().mockImplementation((...args) => { if (this.findOneWithRelations_) { - return this.findOneWithRelations_(...args); + return this.findOneWithRelations_(...args) } - }); + }) findOne = jest.fn().mockImplementation((...args) => { if (this.findOne_) { - return this.findOne_(...args); + return this.findOne_(...args) } - }); + }) findDescendantsTree = jest.fn().mockImplementation((...args) => { if (this.findDescendantsTree_) { - return this.findDescendantsTree_(...args); + return this.findDescendantsTree_(...args) } - }); + }) findOneOrFail = jest.fn().mockImplementation((...args) => { if (this.findOneOrFail_) { - return this.findOneOrFail_(...args); + return this.findOneOrFail_(...args) } - }); + }) find = jest.fn().mockImplementation((...args) => { if (this.find_) { - return this.find_(...args); + return this.find_(...args) } - }); + }) softRemove = jest.fn().mockImplementation((...args) => { if (this.softRemove_) { - return this.softRemove_(...args); + return this.softRemove_(...args) } - }); + }) save = jest.fn().mockImplementation((...args) => { if (this.save_) { - return this.save_(...args); + return this.save_(...args) } - return Promise.resolve(...args); - }); + return Promise.resolve(...args) + }) findAndCount = jest.fn().mockImplementation((...args) => { if (this.findAndCount_) { - return this.findAndCount_(...args); + return this.findAndCount_(...args) } - return {}; - }); + return {} + }) delete = jest.fn().mockImplementation((...args) => { if (this.delete_) { - return this.delete_(...args); + return this.delete_(...args) } - return {}; - }); + return {} + }) } export default (methods = {}) => { - return new MockRepo(methods); -}; + return new MockRepo(methods) +} diff --git a/packages/medusa/src/api/routes/admin/products/__tests__/update-product.js b/packages/medusa/src/api/routes/admin/products/__tests__/update-product.js index 16ff180587..4ea300ca05 100644 --- a/packages/medusa/src/api/routes/admin/products/__tests__/update-product.js +++ b/packages/medusa/src/api/routes/admin/products/__tests__/update-product.js @@ -2,7 +2,6 @@ import { IdMap } from "medusa-test-utils" import { request } from "../../../../../helpers/test-request" import { ProductServiceMock } from "../../../../../services/__mocks__/product" import { ProductVariantServiceMock } from "../../../../../services/__mocks__/product-variant" -import { EventBusServiceMock } from "../../../../../services/__mocks__/event-bus" describe("POST /admin/products/:id", () => { describe("successfully updates a product", () => { @@ -18,7 +17,7 @@ describe("POST /admin/products/:id", () => { description: "Updated test description", handle: "handle", variants: [ - { id: IdMap.getId("variant_1"), title: "Green" }, + { id: IdMap.getId("testVariant"), title: "Green" }, { title: "Blue" }, { title: "Yellow" }, ], @@ -49,7 +48,6 @@ describe("POST /admin/products/:id", () => { }) it("successfully updates variants and create new ones", async () => { - expect(ProductVariantServiceMock.delete).toHaveBeenCalledTimes(2) expect(ProductVariantServiceMock.update).toHaveBeenCalledTimes(1) expect(ProductVariantServiceMock.create).toHaveBeenCalledTimes(2) }) @@ -74,7 +72,7 @@ describe("POST /admin/products/:id", () => { ) expect(subject.status).toEqual(404) expect(subject.error.text).toEqual( - `{"type":"not_found","message":"Variant with id: test_321 is not associated with this product"}` + `{"type":"not_found","message":"Variants with id: test_321 are not associated with this product"}` ) }) }) diff --git a/packages/medusa/src/api/routes/admin/products/update-product.ts b/packages/medusa/src/api/routes/admin/products/update-product.ts index 25a0feda20..59c71f66ed 100644 --- a/packages/medusa/src/api/routes/admin/products/update-product.ts +++ b/packages/medusa/src/api/routes/admin/products/update-product.ts @@ -11,7 +11,6 @@ import { ValidateIf, ValidateNested, } from "class-validator" -import { defaultAdminProductFields, defaultAdminProductRelations } from "." import { PricingService, ProductService, @@ -19,10 +18,10 @@ import { ProductVariantService, } from "../../../../services" import { + ProductProductCategoryReq, ProductSalesChannelReq, ProductTagReq, ProductTypeReq, - ProductProductCategoryReq, } from "../../../../types/product" import { Type } from "class-transformer" @@ -32,17 +31,23 @@ import { ProductStatus, ProductVariant } from "../../../../models" import { CreateProductVariantInput, ProductVariantPricesUpdateReq, + UpdateProductVariantInput, } from "../../../../types/product-variant" import { FeatureFlagDecorators } from "../../../../utils/feature-flag-decorators" import { validator } from "../../../../utils/validator" import { MedusaError } from "medusa-core-utils" import { DistributedTransaction } from "../../../../utils/transaction" +import { IInventoryService } from "../../../../interfaces" +import { Logger } from "../../../../types/global" +import { + defaultAdminProductFields, + defaultAdminProductRelations, +} from "./index" +import { ProductVariantRepository } from "../../../../repositories/product-variant" import { createVariantTransaction, revertVariantTransaction, } from "./transaction/create-product-variant" -import { IInventoryService } from "../../../../interfaces" -import { Logger } from "../../../../types/global" /** * @oas [post] /products/{id} @@ -113,6 +118,9 @@ export default async (req, res) => { const validated = await validator(AdminPostProductsProductReq, req.body) const logger: Logger = req.scope.resolve("logger") + const productVariantRepo: typeof ProductVariantRepository = req.scope.resolve( + "productVariantRepository" + ) const productService: ProductService = req.scope.resolve("productService") const pricingService: PricingService = req.scope.resolve("pricingService") const productVariantService: ProductVariantService = req.scope.resolve( @@ -125,31 +133,74 @@ export default async (req, res) => { const manager: EntityManager = req.scope.resolve("manager") await manager.transaction(async (transactionManager) => { + const productServiceTx = productService.withTransaction(transactionManager) + const { variants } = validated delete validated.variants - await productService - .withTransaction(transactionManager) - .update(id, validated) + const product = await productServiceTx.update(id, validated) if (!variants) { return } - const product = await productService + const variantRepo = manager.getCustomRepository(productVariantRepo) + const productVariants = await productVariantService .withTransaction(transactionManager) - .retrieve(id, { - relations: ["variants"], - }) + .list( + { product_id: id }, + { + select: variantRepo.metadata.columns.map( + (c) => c.propertyName + ) as (keyof ProductVariant)[], + } + ) - // Iterate product variants and update their properties accordingly - for (const variant of product.variants) { - const exists = variants.find((v) => v.id && variant.id === v.id) - if (!exists) { - await productVariantService - .withTransaction(transactionManager) - .delete(variant.id) + const productVariantMap = new Map(productVariants.map((v) => [v.id, v])) + const variantWithIdSet = new Set() + + const variantIdsNotBelongingToProduct: string[] = [] + const variantsToUpdate: { + variant: ProductVariant + updateData: UpdateProductVariantInput + }[] = [] + const variantsToCreate: ProductVariantReq[] = [] + + // Preparing the data step + for (const [variantRank, variant] of variants.entries()) { + if (!variant.id) { + Object.assign(variant, { + variant_rank: variantRank, + options: variant.options || [], + prices: variant.prices || [], + }) + variantsToCreate.push(variant) + continue } + + // Will be used to find the variants that should be removed during the next steps + variantWithIdSet.add(variant.id) + + if (!productVariantMap.has(variant.id)) { + variantIdsNotBelongingToProduct.push(variant.id) + continue + } + + const productVariant = productVariantMap.get(variant.id)! + Object.assign(variant, { + variant_rank: variantRank, + product_id: productVariant.product_id, + }) + variantsToUpdate.push({ variant: productVariant, updateData: variant }) + } + + if (variantIdsNotBelongingToProduct.length) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `Variants with id: ${variantIdsNotBelongingToProduct.join( + ", " + )} are not associated with this product` + ) } const allVariantTransactions: DistributedTransaction[] = [] @@ -160,57 +211,46 @@ export default async (req, res) => { productVariantService, } - for (const [index, newVariant] of variants.entries()) { - const variantRank = index + const productVariantServiceTx = + productVariantService.withTransaction(transactionManager) - if (newVariant.id) { - const variant = product.variants.find((v) => v.id === newVariant.id) + // Delete the variant that does not exist anymore from the provided variants + const variantIdsToDelete = [...productVariantMap.keys()].filter( + (variantId) => !variantWithIdSet.has(variantId) + ) - if (!variant) { - throw new MedusaError( - MedusaError.Types.NOT_FOUND, - `Variant with id: ${newVariant.id} is not associated with this product` - ) - } + if (variantIdsToDelete) { + await productVariantServiceTx.delete(variantIdsToDelete) + } - await productVariantService - .withTransaction(transactionManager) - .update(variant, { - ...newVariant, - variant_rank: variantRank, - product_id: variant.product_id, - }) - } else { - // If the provided variant does not have an id, we assume that it - // should be created + if (variantsToUpdate.length) { + await productVariantServiceTx.update(variantsToUpdate) + } - try { - const input = { - ...newVariant, - variant_rank: variantRank, - options: newVariant.options || [], - prices: newVariant.prices || [], + if (variantsToCreate.length) { + await Promise.all( + variantsToCreate.map(async (data) => { + try { + const varTransaction = await createVariantTransaction( + transactionDependencies, + product.id, + data as CreateProductVariantInput + ) + allVariantTransactions.push(varTransaction) + } catch (e) { + await Promise.all( + allVariantTransactions.map(async (transaction) => { + await revertVariantTransaction( + transactionDependencies, + transaction + ).catch(() => logger.warn("Transaction couldn't be reverted.")) + }) + ) + + throw e } - - const varTransation = await createVariantTransaction( - transactionDependencies, - product.id, - input as CreateProductVariantInput - ) - allVariantTransactions.push(varTransation) - } catch (e) { - await Promise.all( - allVariantTransactions.map(async (transaction) => { - await revertVariantTransaction( - transactionDependencies, - transaction - ).catch(() => logger.warn("Transaction couldn't be reverted.")) - }) - ) - - throw e - } - } + }) + ) } }) diff --git a/packages/medusa/src/helpers/test-request.js b/packages/medusa/src/helpers/test-request.js index 9201a9848c..68d20b54d2 100644 --- a/packages/medusa/src/helpers/test-request.js +++ b/packages/medusa/src/helpers/test-request.js @@ -7,12 +7,13 @@ import "reflect-metadata" import supertest from "supertest" import apiLoader from "../loaders/api" import featureFlagLoader, { featureFlagRouter } from "../loaders/feature-flags" -import { moduleHelper } from "../loaders/module" +import moduleLoader, { moduleHelper } from "../loaders/module" import passportLoader from "../loaders/passport" import servicesLoader from "../loaders/services" import strategiesLoader from "../loaders/strategies" import registerModuleDefinitions from "../loaders/module-definitions" -import moduleLoader from "../loaders/module" +import repositories from "../loaders/repositories" +import models from "../loaders/models" const adminSessionOpts = { cookieName: "session", @@ -39,8 +40,32 @@ const config = { const testApp = express() +function asArray(resolvers) { + return { + resolve: (container) => + resolvers.map((resolver) => container.build(resolver)), + } +} + const container = createContainer() +// TODO: remove once the util is merged in master +container.registerAdd = function (name, registration) { + const storeKey = name + "_STORE" + + if (this.registrations[storeKey] === undefined) { + this.register(storeKey, asValue([])) + } + const store = this.resolve(storeKey) + + if (this.registrations[name] === undefined) { + this.register(name, asArray(store)) + } + store.unshift(registration) + + return this +}.bind(container) + container.register("featureFlagRouter", asValue(featureFlagRouter)) container.register("modulesHelper", asValue(moduleHelper)) container.register("configModule", asValue(config)) @@ -65,6 +90,8 @@ testApp.use((req, res, next) => { }) featureFlagLoader(config) +models({ container, configModule: config, isTest: true }) +repositories({ container, isTest: true }) servicesLoader({ container, configModule: config }) strategiesLoader({ container, configModule: config }) passportLoader({ app: testApp, container, configModule: config }) diff --git a/packages/medusa/src/loaders/models.ts b/packages/medusa/src/loaders/models.ts index 91d741f95d..c237e734dc 100644 --- a/packages/medusa/src/loaders/models.ts +++ b/packages/medusa/src/loaders/models.ts @@ -3,16 +3,16 @@ import glob from "glob" import path from "path" import { ClassConstructor, MedusaContainer } from "../types/global" import { EntitySchema } from "typeorm" -import { asClass, asValue, AwilixContainer } from "awilix" +import { asClass, asValue } from "awilix" /** * Registers all models in the model directory */ export default ( - { container }: { container: MedusaContainer }, + { container, isTest }: { container: MedusaContainer; isTest?: boolean }, config = { register: true } ) => { - const corePath = "../models/*.js" + const corePath = isTest ? "../models/*.ts" : "../models/*.js" const coreFull = path.join(__dirname, corePath) const models: (ClassConstructor | EntitySchema)[] = [] diff --git a/packages/medusa/src/loaders/repositories.ts b/packages/medusa/src/loaders/repositories.ts index 9735ba3b87..810d9e4fd7 100644 --- a/packages/medusa/src/loaders/repositories.ts +++ b/packages/medusa/src/loaders/repositories.ts @@ -8,8 +8,14 @@ import { ClassConstructor, MedusaContainer } from "../types/global" /** * Registers all models in the model directory */ -export default ({ container }: { container: MedusaContainer }): void => { - const corePath = "../repositories/*.js" +export default ({ + container, + isTest, +}: { + container: MedusaContainer + isTest?: boolean +}): void => { + const corePath = isTest ? "../repositories/*.ts" : "../repositories/*.js" const coreFull = path.join(__dirname, corePath) const core = glob.sync(coreFull, { cwd: __dirname }) diff --git a/packages/medusa/src/repositories/image.ts b/packages/medusa/src/repositories/image.ts index fac2aad568..b8cd006a72 100644 --- a/packages/medusa/src/repositories/image.ts +++ b/packages/medusa/src/repositories/image.ts @@ -1,8 +1,25 @@ import { EntityRepository, In, Repository } from "typeorm" -import { Image } from "../models/image" +import { Image } from "../models" +import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity" @EntityRepository(Image) export class ImageRepository extends Repository { + async insertBulk(data: QueryDeepPartialEntity[]): Promise { + const queryBuilder = this.createQueryBuilder() + .insert() + .into(Image) + .values(data) + + // TODO: remove if statement once this issue is resolved https://github.com/typeorm/typeorm/issues/9850 + if (!queryBuilder.connection.driver.isReturningSqlSupported("insert")) { + const rawImages = await queryBuilder.execute() + return rawImages.generatedMaps.map((d) => this.create(d)) as Image[] + } + + const rawImages = await queryBuilder.returning("*").execute() + return rawImages.generatedMaps.map((d) => this.create(d)) + } + public async upsertImages(imageUrls: string[]) { const existingImages = await this.find({ where: { @@ -14,16 +31,21 @@ export class ImageRepository extends Repository { ) const upsertedImgs: Image[] = [] + const imageToCreate: QueryDeepPartialEntity[] = [] - for (const url of imageUrls) { + imageUrls.forEach((url) => { const aImg = existingImagesMap.get(url) if (aImg) { upsertedImgs.push(aImg) } else { const newImg = this.create({ url }) - const savedImg = await this.save(newImg) - upsertedImgs.push(savedImg) + imageToCreate.push(newImg as QueryDeepPartialEntity) } + }) + + if (imageToCreate.length) { + const newImgs = await this.insertBulk(imageToCreate) + upsertedImgs.push(...newImgs) } return upsertedImgs diff --git a/packages/medusa/src/repositories/money-amount.ts b/packages/medusa/src/repositories/money-amount.ts index 2906b8cc48..cdcc1d901e 100644 --- a/packages/medusa/src/repositories/money-amount.ts +++ b/packages/medusa/src/repositories/money-amount.ts @@ -10,11 +10,13 @@ import { WhereExpressionBuilder, } from "typeorm" import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity" -import { MoneyAmount } from "../models/money-amount" +import { MoneyAmount } from "../models" import { PriceListPriceCreateInput, PriceListPriceUpdateInput, } from "../types/price-list" +import { isString } from "../utils" +import { ProductVariantPrice } from "../types/product-variant" type Price = Partial< Omit @@ -24,6 +26,26 @@ type Price = Partial< @EntityRepository(MoneyAmount) export class MoneyAmountRepository extends Repository { + async insertBulk( + data: QueryDeepPartialEntity[] + ): Promise { + const queryBuilder = this.createQueryBuilder() + .insert() + .into(MoneyAmount) + .values(data) + + // TODO: remove if statement once this issue is resolved https://github.com/typeorm/typeorm/issues/9850 + if (!queryBuilder.connection.driver.isReturningSqlSupported("insert")) { + const rawMoneyAmounts = await queryBuilder.execute() + return rawMoneyAmounts.generatedMaps.map((d) => + this.create(d) + ) as MoneyAmount[] + } + + const rawMoneyAmounts = await queryBuilder.returning("*").execute() + return rawMoneyAmounts.generatedMaps.map((d) => this.create(d)) + } + /** * Will be removed in a future release. * Use `deleteVariantPricesNotIn` instead. @@ -50,41 +72,58 @@ export class MoneyAmountRepository extends Repository { } public async deleteVariantPricesNotIn( - variantId: string, - prices: Price[] + variantIdOrData: + | string + | { variantId: string; prices: ProductVariantPrice[] }[], + prices?: Price[] ): Promise { - const where = { - variant_id: variantId, - price_list_id: IsNull(), - } - - const orWhere: ObjectLiteral[] = [] - - for (const price of prices) { - if (price.currency_code) { - orWhere.push( + const data = isString(variantIdOrData) + ? [ { - currency_code: Not(price.currency_code), + variantId: variantIdOrData, + prices: prices!, }, - { - region_id: price.region_id ? Not(price.region_id) : Not(IsNull()), - currency_code: price.currency_code, - } - ) + ] + : variantIdOrData + + const queryBuilder = this.createQueryBuilder().delete() + + for (const data_ of data) { + const where = { + variant_id: data_.variantId, + price_list_id: IsNull(), } - if (price.region_id) { - orWhere.push({ - region_id: Not(price.region_id), - }) + const orWhere: ObjectLiteral[] = [] + + for (const price of data_.prices) { + if (price.currency_code) { + orWhere.push( + { + currency_code: Not(price.currency_code), + }, + { + region_id: price.region_id ? Not(price.region_id) : Not(IsNull()), + currency_code: price.currency_code, + } + ) + } + + if (price.region_id) { + orWhere.push({ + region_id: Not(price.region_id), + }) + } } + + queryBuilder.orWhere( + new Brackets((localQueryBuild) => { + localQueryBuild.where(where).andWhere(orWhere) + }) + ) } - await this.createQueryBuilder() - .delete() - .where(where) - .andWhere(orWhere) - .execute() + await queryBuilder.execute() } public async upsertVariantCurrencyPrice( diff --git a/packages/medusa/src/repositories/staged-job.ts b/packages/medusa/src/repositories/staged-job.ts index 4db1ee7118..97327f22e3 100644 --- a/packages/medusa/src/repositories/staged-job.ts +++ b/packages/medusa/src/repositories/staged-job.ts @@ -1,7 +1,6 @@ import { EntityRepository, Repository } from "typeorm" import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity" import { StagedJob } from "../models" -import { rowSqlResultsToEntityTransformer } from "../utils" @EntityRepository(StagedJob) export class StagedJobRepository extends Repository { @@ -14,15 +13,10 @@ export class StagedJobRepository extends Repository { // TODO: remove if statement once this issue is resolved https://github.com/typeorm/typeorm/issues/9850 if (!queryBuilder.connection.driver.isReturningSqlSupported("insert")) { const rawStagedJobs = await queryBuilder.execute() - return rawStagedJobs.generatedMaps + return rawStagedJobs.generatedMaps.map((d) => this.create(d)) } const rawStagedJobs = await queryBuilder.returning("*").execute() - - return rowSqlResultsToEntityTransformer( - rawStagedJobs.raw, - queryBuilder, - this.queryRunner! - ) + return rawStagedJobs.generatedMaps.map((d) => this.create(d)) } } diff --git a/packages/medusa/src/services/__tests__/product-variant.js b/packages/medusa/src/services/__tests__/product-variant.js index 389f19bbe8..07ef34d62b 100644 --- a/packages/medusa/src/services/__tests__/product-variant.js +++ b/packages/medusa/src/services/__tests__/product-variant.js @@ -276,6 +276,10 @@ describe("ProductVariantService", () => { const productVariantRepository = MockRepository({ findOne: (query) => Promise.resolve({ id: IdMap.getId("ironman") }), + update: (data) => ({ + generatedMaps: [data], + }), + create: (data) => data, }) const moneyAmountRepository = MockRepository({ @@ -315,22 +319,54 @@ describe("ProductVariantService", () => { }) expect(eventBusService.emit).toHaveBeenCalledTimes(1) - expect(eventBusService.emit).toHaveBeenCalledWith( - "product-variant.updated", + expect(eventBusService.emit).toHaveBeenCalledWith([ { - id: IdMap.getId("ironman"), - fields: ["title"], + eventName: "product-variant.updated", + data: { + id: IdMap.getId("ironman"), + fields: ["title"], + }, + }, + ]) + + expect(productVariantRepository.update).toHaveBeenCalledTimes(1) + expect(productVariantRepository.update).toHaveBeenCalledWith( + { id: IdMap.getId("ironman") }, + { + title: "new title", } ) - - expect(productVariantRepository.save).toHaveBeenCalledTimes(1) - expect(productVariantRepository.save).toHaveBeenCalledWith({ - id: IdMap.getId("ironman"), - title: "new title", - }) }) it("successfully updates variant", async () => { + await productVariantService.update( + { id: IdMap.getId("ironman"), title: "new title" }, + { + title: "new title 2", + } + ) + + expect(eventBusService.emit).toHaveBeenCalledTimes(1) + expect(eventBusService.emit).toHaveBeenCalledWith([ + { + eventName: "product-variant.updated", + data: { + id: IdMap.getId("ironman"), + fields: ["title"], + }, + }, + ]) + + expect(productVariantRepository.update).toHaveBeenCalledTimes(1) + expect(productVariantRepository.update).toHaveBeenCalledWith( + { id: IdMap.getId("ironman") }, + { + title: "new title 2", + } + ) + }) + + it("successfully avoid to update variant if the data have not changed", async () => { await productVariantService.update( { id: IdMap.getId("ironman"), title: "new title" }, { @@ -338,20 +374,9 @@ describe("ProductVariantService", () => { } ) - expect(eventBusService.emit).toHaveBeenCalledTimes(1) - expect(eventBusService.emit).toHaveBeenCalledWith( - "product-variant.updated", - { - id: IdMap.getId("ironman"), - fields: ["title"], - } - ) + expect(eventBusService.emit).toHaveBeenCalledTimes(0) - expect(productVariantRepository.save).toHaveBeenCalledTimes(1) - expect(productVariantRepository.save).toHaveBeenCalledWith({ - id: IdMap.getId("ironman"), - title: "new title", - }) + expect(productVariantRepository.save).toHaveBeenCalledTimes(0) }) it("throws if provided variant is missing an id", async () => { @@ -376,22 +401,26 @@ describe("ProductVariantService", () => { }) expect(eventBusService.emit).toHaveBeenCalledTimes(1) - expect(eventBusService.emit).toHaveBeenCalledWith( - "product-variant.updated", + expect(eventBusService.emit).toHaveBeenCalledWith([ { - id: IdMap.getId("ironman"), - fields: ["title", "metadata"], + eventName: "product-variant.updated", + data: { + id: IdMap.getId("ironman"), + fields: ["title", "metadata"], + }, + }, + ]) + + expect(productVariantRepository.update).toHaveBeenCalledTimes(1) + expect(productVariantRepository.update).toHaveBeenCalledWith( + { id: IdMap.getId("ironman") }, + { + title: "new title", + metadata: { + testing: "this", + }, } ) - - expect(productVariantRepository.save).toHaveBeenCalledTimes(1) - expect(productVariantRepository.save).toHaveBeenCalledWith({ - id: IdMap.getId("ironman"), - title: "new title", - metadata: { - testing: "this", - }, - }) }) it("successfully updates variant inventory_quantity", async () => { @@ -401,20 +430,24 @@ describe("ProductVariantService", () => { }) expect(eventBusService.emit).toHaveBeenCalledTimes(1) - expect(eventBusService.emit).toHaveBeenCalledWith( - "product-variant.updated", + expect(eventBusService.emit).toHaveBeenCalledWith([ { - id: IdMap.getId("ironman"), - fields: ["title", "inventory_quantity"], + eventName: "product-variant.updated", + data: { + id: IdMap.getId("ironman"), + fields: ["title", "inventory_quantity"], + }, + }, + ]) + + expect(productVariantRepository.update).toHaveBeenCalledTimes(1) + expect(productVariantRepository.update).toHaveBeenCalledWith( + { id: IdMap.getId("ironman") }, + { + inventory_quantity: 98, + title: "new title", } ) - - expect(productVariantRepository.save).toHaveBeenCalledTimes(1) - expect(productVariantRepository.save).toHaveBeenCalledWith({ - id: IdMap.getId("ironman"), - inventory_quantity: 98, - title: "new title", - }) }) it("successfully updates variant prices", async () => { @@ -429,17 +462,19 @@ describe("ProductVariantService", () => { }) expect(productVariantService.updateVariantPrices).toHaveBeenCalledTimes(1) - expect(productVariantService.updateVariantPrices).toHaveBeenCalledWith( - IdMap.getId("ironman"), - [ - { - currency_code: "dkk", - amount: 1000, - }, - ] - ) + expect(productVariantService.updateVariantPrices).toHaveBeenCalledWith([ + { + variantId: IdMap.getId("ironman"), + prices: [ + { + currency_code: "dkk", + amount: 1000, + }, + ], + }, + ]) - expect(productVariantRepository.save).toHaveBeenCalledTimes(1) + expect(productVariantRepository.update).toHaveBeenCalledTimes(1) }) it("successfully updates variant options", async () => { @@ -460,7 +495,7 @@ describe("ProductVariantService", () => { "red" ) - expect(productVariantRepository.save).toHaveBeenCalledTimes(1) + expect(productVariantRepository.update).toHaveBeenCalledTimes(1) }) }) @@ -628,8 +663,22 @@ describe("ProductVariantService", () => { amount: 750, }) }, + find: (query) => { + if (query.where.region_id === IdMap.getId("cali")) { + return Promise.resolve([]) + } + return Promise.resolve([ + { + id: IdMap.getId("dkk"), + variant_id: IdMap.getId("ironman"), + currency_code: "dkk", + amount: 750, + }, + ]) + }, create: (p) => p, remove: () => Promise.resolve(), + insertBulk: (data) => data, }) const oldPrices = [ @@ -683,7 +732,7 @@ describe("ProductVariantService", () => { jest.clearAllMocks() }) - it("successfully removes obsolete prices and calls save on new/existing prices", async () => { + it("successfully removes obsolete prices and create new prices", async () => { await productVariantService.updateVariantPrices("ironman", [ { currency_code: "usd", @@ -695,15 +744,14 @@ describe("ProductVariantService", () => { moneyAmountRepository.deleteVariantPricesNotIn ).toHaveBeenCalledTimes(1) - expect( - moneyAmountRepository.upsertVariantCurrencyPrice - ).toHaveBeenCalledTimes(1) - expect( - moneyAmountRepository.upsertVariantCurrencyPrice - ).toHaveBeenCalledWith("ironman", { - currency_code: "usd", - amount: 4000, - }) + expect(moneyAmountRepository.insertBulk).toHaveBeenCalledTimes(1) + expect(moneyAmountRepository.insertBulk).toHaveBeenCalledWith([ + { + variant_id: "ironman", + currency_code: "usd", + amount: 4000, + }, + ]) }) it("successfully creates new a region price", async () => { @@ -718,33 +766,31 @@ describe("ProductVariantService", () => { expect(moneyAmountRepository.create).toHaveBeenCalledWith({ variant_id: IdMap.getId("ironman"), region_id: IdMap.getId("cali"), + currency_code: "usd", amount: 100, }) - expect(moneyAmountRepository.save).toHaveBeenCalledTimes(1) + expect(moneyAmountRepository.insertBulk).toHaveBeenCalledTimes(1) }) - it("successfully creates a currency price", async () => { + it("successfully updates a currency price", async () => { await productVariantService.updateVariantPrices(IdMap.getId("ironman"), [ { id: IdMap.getId("dkk"), currency_code: "dkk", - amount: 750, + amount: 850, }, ]) expect(moneyAmountRepository.create).toHaveBeenCalledTimes(0) - expect( - moneyAmountRepository.upsertVariantCurrencyPrice - ).toHaveBeenCalledTimes(1) - expect( - moneyAmountRepository.upsertVariantCurrencyPrice - ).toHaveBeenCalledWith(IdMap.getId("ironman"), { - id: IdMap.getId("dkk"), - currency_code: "dkk", - amount: 750, - }) + expect(moneyAmountRepository.update).toHaveBeenCalledTimes(1) + expect(moneyAmountRepository.update).toHaveBeenCalledWith( + { id: IdMap.getId("dkk") }, + { + amount: 850, + } + ) }) }) @@ -881,14 +927,16 @@ describe("ProductVariantService", () => { describe("delete", () => { const productVariantRepository = MockRepository({ - findOne: (query) => { + find: (query) => { if (query.where.id === IdMap.getId("ironmanv2")) { - return Promise.resolve(undefined) + return Promise.resolve([]) } - return Promise.resolve({ - id: IdMap.getId("ironman"), - product_id: IdMap.getId("product-test"), - }) + return Promise.resolve([ + { + id: IdMap.getId("ironman"), + product_id: IdMap.getId("product-test"), + }, + ]) }, }) @@ -906,20 +954,22 @@ describe("ProductVariantService", () => { await productVariantService.delete(IdMap.getId("ironman")) expect(productVariantRepository.softRemove).toBeCalledTimes(1) - expect(productVariantRepository.softRemove).toBeCalledWith( + expect(productVariantRepository.softRemove).toBeCalledWith([ expect.objectContaining({ id: IdMap.getId("ironman"), - }) - ) + }), + ]) expect(eventBusService.emit).toHaveBeenCalledTimes(1) - expect(eventBusService.emit).toHaveBeenCalledWith( - "product-variant.deleted", + expect(eventBusService.emit).toHaveBeenCalledWith([ { - id: IdMap.getId("ironman"), - product_id: IdMap.getId("product-test"), - } - ) + eventName: "product-variant.deleted", + data: { + id: IdMap.getId("ironman"), + product_id: IdMap.getId("product-test"), + }, + }, + ]) }) it("successfully resolves if variant does not exist", async () => { diff --git a/packages/medusa/src/services/product-variant.ts b/packages/medusa/src/services/product-variant.ts index 0c2e0140c2..6056e7a522 100644 --- a/packages/medusa/src/services/product-variant.ts +++ b/packages/medusa/src/services/product-variant.ts @@ -1,5 +1,12 @@ import { isDefined, MedusaError } from "medusa-core-utils" -import { Brackets, EntityManager, ILike, SelectQueryBuilder } from "typeorm" +import { + Brackets, + EntityManager, + ILike, + In, + IsNull, + SelectQueryBuilder, +} from "typeorm" import { IPriceSelectionStrategy, PriceSelectionContext, @@ -19,17 +26,28 @@ import { FindWithRelationsOptions, ProductVariantRepository, } from "../repositories/product-variant" -import { FindConfig } from "../types/common" +import { FindConfig, WithRequiredProperty } from "../types/common" import { CreateProductVariantInput, FilterableProductVariantProps, GetRegionPriceContext, ProductVariantPrice, + UpdateProductVariantData, UpdateProductVariantInput, + UpdateVariantCurrencyPriceData, + UpdateVariantPricesData, + UpdateVariantRegionPriceData, } from "../types/product-variant" -import { buildQuery, setMetadata } from "../utils" +import { + buildQuery, + hasChanges, + isObject, + isString, + setMetadata, +} from "../utils" import EventBusService from "./event-bus" import RegionService from "./region" +import { QueryDeepPartialEntity } from "typeorm/query-builder/QueryPartialEntity" class ProductVariantService extends TransactionBaseService { static Events = { @@ -160,7 +178,7 @@ class ProductVariantService extends TransactionBaseService { let product = productOrProductId - if (typeof product === `string`) { + if (isString(product)) { product = (await productRepo.findOne({ where: { id: productOrProductId }, relations: ["variants", "variants.options", "options"], @@ -219,19 +237,12 @@ class ProductVariantService extends TransactionBaseService { const result = await variantRepo.save(productVariant) if (prices) { - for (const price of prices) { - if (price.region_id) { - const region = await this.regionService_.retrieve(price.region_id) - - await this.setRegionPrice(result.id, { - amount: price.amount, - region_id: price.region_id, - currency_code: region.currency_code, - }) - } else { - await this.setCurrencyPrice(result.id, price) - } - } + await this.updateVariantPrices([ + { + variantId: result.id, + prices, + }, + ]) } await this.eventBus_ @@ -245,86 +256,186 @@ class ProductVariantService extends TransactionBaseService { }) } + /** + * Updates a collection of variant. + * @param variantData - a collection of variant and the data to update. + * @return resolves to the update result. + */ + async update( + variantData: { + variant: ProductVariant + updateData: UpdateProductVariantInput + }[] + ): Promise + /** * Updates a variant. * Price updates should use dedicated methods. * The function will throw, if price updates are attempted. * @param variantOrVariantId - variant or id of a variant. * @param update - an object with the update values. - * @param config - an object with the config values for returning the variant. * @return resolves to the update result. */ async update( variantOrVariantId: string | Partial, update: UpdateProductVariantInput - ): Promise { + ): Promise + + async update( + variantOrVariantId: string | Partial, + update: UpdateProductVariantInput + ): Promise + + async update< + TInput extends + | string + | Partial + | UpdateProductVariantData[], + TResult = TInput extends UpdateProductVariantData[] + ? ProductVariant[] + : ProductVariant + >( + variantOrVariantIdOrData: TInput, + updateData?: UpdateProductVariantInput + ): Promise { + let data = Array.isArray(variantOrVariantIdOrData) + ? variantOrVariantIdOrData + : ([] as UpdateProductVariantData[]) + return await this.atomicPhase_(async (manager: EntityManager) => { const variantRepo = manager.getCustomRepository( this.productVariantRepository_ ) - let variant = variantOrVariantId - if (typeof variant === `string`) { - const variantRes = await variantRepo.findOne({ - where: { id: variantOrVariantId as string }, - }) - if (!isDefined(variantRes)) { + if (updateData) { + let variant: Partial | null = + variantOrVariantIdOrData as Partial + + if (isString(variantOrVariantIdOrData)) { + variant = await this.retrieve(variantOrVariantIdOrData, { + select: variantRepo.metadata.columns.map( + (c) => c.propertyName + ) as (keyof ProductVariant)[], + }) + } + + if (!variant?.id) { throw new MedusaError( - MedusaError.Types.NOT_FOUND, - `Variant with id ${variantOrVariantId} was not found` - ) - } else { - variant = variantRes as ProductVariant - } - } else if (!variant.id) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Variant id missing` - ) - } - - const { prices, options, metadata, inventory_quantity, ...rest } = update - - if (prices) { - await this.updateVariantPrices(variant.id!, prices) - } - - if (options) { - for (const option of options) { - await this.updateOptionValue( - variant.id!, - option.option_id, - option.value + MedusaError.Types.INVALID_DATA, + `Variant id missing` ) } + + data = [{ variant: variant as ProductVariant, updateData: updateData }] } - if (typeof metadata === "object") { - variant.metadata = setMetadata(variant as ProductVariant, metadata) - } + const result = await this.updateBatch(data) - if (typeof inventory_quantity === "number") { - variant.inventory_quantity = inventory_quantity as number - } - - for (const [key, value] of Object.entries(rest)) { - variant[key] = value - } - - const result = await variantRepo.save(variant) - - await this.eventBus_ - .withTransaction(manager) - .emit(ProductVariantService.Events.UPDATED, { - id: result.id, - product_id: result.product_id, - fields: Object.keys(update), - }) - - return result + return (Array.isArray(variantOrVariantIdOrData) + ? result + : result[0]) as unknown as TResult }) } + protected async updateBatch( + variantData: UpdateProductVariantData[] + ): Promise { + return await this.atomicPhase_(async (manager: EntityManager) => { + const variantRepo = manager.getCustomRepository( + this.productVariantRepository_ + ) + + const variantPriceUpdateData = variantData + .filter((data) => isDefined(data.updateData.prices)) + .map((data) => ({ + variantId: data.variant.id, + prices: data.updateData.prices!, + })) + + if (variantPriceUpdateData.length) { + await this.updateVariantPrices(variantPriceUpdateData) + } + + const results: [ProductVariant, UpdateProductVariantInput, boolean][] = + await Promise.all( + variantData.map(async ({ variant, updateData }) => { + const { prices, options, ...rest } = updateData + + const shouldUpdate = hasChanges(variant, rest) + const shouldEmitUpdateEvent = + shouldUpdate || !!options?.length || !!prices?.length + + for (const option of options ?? []) { + await this.updateOptionValue( + variant.id!, + option.option_id, + option.value + ) + } + + const toUpdate: QueryDeepPartialEntity = {} + + if (isObject(rest.metadata)) { + toUpdate["metadata"] = setMetadata( + variant as ProductVariant, + rest.metadata + ) as QueryDeepPartialEntity> + delete rest.metadata + } + + if (Object.keys(rest).length) { + for (const [key, value] of Object.entries(rest)) { + if (variant[key] !== value) { + toUpdate[key] = value + } + } + } + + let result = variant + + // No need to update if nothing on the variant has changed + if (shouldUpdate) { + const { id } = variant + const rawResult = await variantRepo.update({ id }, toUpdate) + result = variantRepo.create({ + ...variant, + ...rawResult.generatedMaps[0], + }) + } + + return [result, updateData, shouldEmitUpdateEvent] + }) + ) + + const events = results + .filter(([, , shouldEmitUpdateEvent]) => shouldEmitUpdateEvent) + .map(([result, updatedData]) => { + return { + eventName: ProductVariantService.Events.UPDATED, + data: { + id: result.id, + product_id: result.product_id, + fields: Object.keys(updatedData), + }, + } + }) + + if (events.length) { + await this.eventBus_.withTransaction(manager).emit(events) + } + + return results.map(([variant]) => variant) + }) + } + + /** + * Updates variant/prices collection. + * Deletes any prices that are not in the update object, and is not associated with a price list. + * @param data + * @returns empty promise + */ + async updateVariantPrices(data: UpdateVariantPricesData[]): Promise + /** * Updates a variant's prices. * Deletes any prices that are not in the update object, and is not associated with a price list. @@ -335,6 +446,25 @@ class ProductVariantService extends TransactionBaseService { async updateVariantPrices( variantId: string, prices: ProductVariantPrice[] + ): Promise + + async updateVariantPrices( + variantIdOrData: string | UpdateVariantPricesData[], + prices?: ProductVariantPrice[] + ): Promise { + let data = !isString(variantIdOrData) + ? variantIdOrData + : ([] as UpdateVariantPricesData[]) + + if (prices && isString(variantIdOrData)) { + data = [{ variantId: variantIdOrData, prices }] + } + + return await this.updateVariantPricesBatch(data) + } + + protected async updateVariantPricesBatch( + data: UpdateVariantPricesData[] ): Promise { return await this.atomicPhase_(async (manager: EntityManager) => { const moneyAmountRepo = manager.getCustomRepository( @@ -342,23 +472,214 @@ class ProductVariantService extends TransactionBaseService { ) // Delete obsolete prices - await moneyAmountRepo.deleteVariantPricesNotIn(variantId, prices) + await moneyAmountRepo.deleteVariantPricesNotIn(data) - const regionsServiceTx = this.regionService_.withTransaction(manager) + const regionIdsSet: Set = new Set( + data + .map((data_) => + data_.prices + .filter((price) => price.region_id) + .map((price) => price.region_id!) + ) + .flat() + ) - for (const price of prices) { - if (price.region_id) { - const region = await regionsServiceTx.retrieve(price.region_id) - - await this.setRegionPrice(variantId, { - currency_code: region.currency_code, - region_id: price.region_id, - amount: price.amount, - }) - } else { - await this.setCurrencyPrice(variantId, price) + const regions = await this.regionService_.withTransaction(manager).list( + { + id: [...regionIdsSet], + }, + { + select: ["id", "currency_code"], } + ) + + const regionsMap = new Map(regions.map((r) => [r.id, r])) + + const dataRegionPrices: UpdateVariantRegionPriceData[] = [] + const dataCurrencyPrices: UpdateVariantCurrencyPriceData[] = [] + + data.forEach(({ prices, variantId }) => { + prices.forEach((price) => { + if (price.region_id) { + const region = regionsMap.get(price.region_id)! + dataRegionPrices.push({ + variantId, + price: { + currency_code: region.currency_code, + region_id: price.region_id, + amount: price.amount, + }, + }) + } else { + dataCurrencyPrices.push({ + variantId, + price: { + ...price, + currency_code: price.currency_code!, + }, + }) + } + }) + }) + + const promises: Promise[] = [] + + if (dataRegionPrices.length) { + promises.push(this.addOrUpdateRegionPrices(dataRegionPrices)) } + + if (dataCurrencyPrices.length) { + promises.push(this.addOrUpdateCurrencyPrices(dataCurrencyPrices)) + } + + await Promise.all(promises) + }) + } + + async addOrUpdateRegionPrices( + data: UpdateVariantRegionPriceData[] + ): Promise { + return await this.atomicPhase_(async (manager: EntityManager) => { + const moneyAmountRepo = manager.getCustomRepository( + this.moneyAmountRepository_ + ) + + const where = data.map((data_) => ({ + variant_id: data_.variantId, + region_id: data_.price.region_id, + price_list_id: IsNull(), + })) + + const moneyAmounts = await moneyAmountRepo.find({ + where, + }) + + const moneyAmountsMapToVariantId = new Map() + moneyAmounts.map((d) => { + const moneyAmounts = moneyAmountsMapToVariantId.get(d.variant_id) ?? [] + moneyAmounts.push(d) + moneyAmountsMapToVariantId.set(d.variant_id, moneyAmounts) + }) + + const dataToCreate: QueryDeepPartialEntity[] = [] + const dataToUpdate: QueryDeepPartialEntity[] = [] + + data.forEach(({ price, variantId }) => { + const variantMoneyAmounts = + moneyAmountsMapToVariantId.get(variantId) ?? [] + + const moneyAmount: MoneyAmount = variantMoneyAmounts.find( + (ma) => ma.region_id === price.region_id + ) + + if (moneyAmount) { + // No need to update if the amount is the same + if (moneyAmount.amount !== price.amount) { + dataToUpdate.push({ + id: moneyAmount.id, + amount: price.amount, + }) + } + } else { + dataToCreate.push( + moneyAmountRepo.create({ + ...price, + variant_id: variantId, + }) as QueryDeepPartialEntity + ) + } + }) + + const promises: Promise[] = [] + + if (dataToCreate.length) { + promises.push(moneyAmountRepo.insertBulk(dataToCreate)) + } + + if (dataToUpdate.length) { + dataToUpdate.forEach((data) => { + const { id, ...rest } = data + promises.push(moneyAmountRepo.update({ id: data.id as string }, rest)) + }) + } + + await Promise.all(promises) + }) + } + + async addOrUpdateCurrencyPrices( + data: { + variantId: string + price: WithRequiredProperty + }[] + ): Promise { + return await this.atomicPhase_(async (manager: EntityManager) => { + const moneyAmountRepo = manager.getCustomRepository( + this.moneyAmountRepository_ + ) + + const where = data.map((data_) => ({ + variant_id: data_.variantId, + currency_code: data_.price.currency_code, + region_id: IsNull(), + price_list_id: IsNull(), + })) + + const moneyAmounts = await moneyAmountRepo.find({ + where, + }) + + const moneyAmountsMapToVariantId = new Map() + moneyAmounts.map((d) => { + const moneyAmounts = moneyAmountsMapToVariantId.get(d.variant_id) ?? [] + moneyAmounts.push(d) + moneyAmountsMapToVariantId.set(d.variant_id, moneyAmounts) + }) + + const dataToCreate: QueryDeepPartialEntity[] = [] + const dataToUpdate: QueryDeepPartialEntity[] = [] + + data.forEach(({ price, variantId }) => { + const variantMoneyAmounts = + moneyAmountsMapToVariantId.get(variantId) ?? [] + + const moneyAmount: MoneyAmount = variantMoneyAmounts.find( + (ma) => ma.currency_code === price.currency_code + ) + + if (moneyAmount) { + // No need to update if the amount is the same + if (moneyAmount.amount !== price.amount) { + dataToUpdate.push({ + id: moneyAmount.id, + amount: price.amount, + }) + } + } else { + dataToCreate.push( + moneyAmountRepo.create({ + ...price, + variant_id: variantId, + currency_code: price.currency_code.toLowerCase(), + }) as QueryDeepPartialEntity + ) + } + }) + + const promises: Promise[] = [] + + if (dataToCreate.length) { + promises.push(moneyAmountRepo.insertBulk(dataToCreate)) + } + + if (dataToUpdate.length) { + dataToUpdate.forEach((data) => { + const { id, ...rest } = data + promises.push(moneyAmountRepo.update({ id: data.id as string }, rest)) + }) + } + + await Promise.all(promises) }) } @@ -394,6 +715,7 @@ class ProductVariantService extends TransactionBaseService { } /** + * @deprecated use addOrUpdateRegionPrices instead * Sets the default price of a specific region * @param variantId - the id of the variant to update * @param price - the price for the variant. @@ -430,6 +752,7 @@ class ProductVariantService extends TransactionBaseService { } /** + * @deprecated use addOrUpdateCurrencyPrices instead * Sets the default price for the given currency. * @param variantId - the id of the variant to set prices for * @param price - the price for the variant @@ -641,36 +964,43 @@ class ProductVariantService extends TransactionBaseService { } /** - * Deletes variant. + * Deletes variant or variants. * Will never fail due to delete being idempotent. - * @param variantId - the id of the variant to delete. Must be + * @param variantIds - the id of the variant to delete. Must be * castable as an ObjectId * @return empty promise */ - async delete(variantId: string): Promise { + async delete(variantIds: string | string[]): Promise { + const variantIds_ = isString(variantIds) ? [variantIds] : variantIds + return await this.atomicPhase_(async (manager: EntityManager) => { const variantRepo = manager.getCustomRepository( this.productVariantRepository_ ) - const variant = await variantRepo.findOne({ - where: { id: variantId }, + const variants = await variantRepo.find({ + where: { id: In(variantIds_) }, relations: ["prices", "options", "inventory_items"], }) - if (!variant) { + if (!variants.length) { return Promise.resolve() } - await variantRepo.softRemove(variant) + await variantRepo.softRemove(variants) - await this.eventBus_ - .withTransaction(manager) - .emit(ProductVariantService.Events.DELETED, { - id: variant.id, - product_id: variant.product_id, - metadata: variant.metadata, - }) + const events = variants.map((variant) => { + return { + eventName: ProductVariantService.Events.DELETED, + data: { + id: variant.id, + product_id: variant.product_id, + metadata: variant.metadata, + }, + } + }) + + await this.eventBus_.withTransaction(manager).emit(events) }) } diff --git a/packages/medusa/src/services/product.ts b/packages/medusa/src/services/product.ts index 15afb58055..31318e5c61 100644 --- a/packages/medusa/src/services/product.ts +++ b/packages/medusa/src/services/product.ts @@ -495,9 +495,6 @@ class ProductService extends TransactionBaseService { ): Promise { return await this.atomicPhase_(async (manager) => { const productRepo = manager.getCustomRepository(this.productRepository_) - const productVariantRepo = manager.getCustomRepository( - this.productVariantRepository_ - ) const productTagRepo = manager.getCustomRepository( this.productTagRepository_ ) @@ -541,20 +538,32 @@ class ProductService extends TransactionBaseService { product.thumbnail = images[0] } + const promises: Promise[] = [] + if (images) { - product.images = await imageRepo.upsertImages(images) + promises.push( + imageRepo + .upsertImages(images) + .then((image) => (product.images = image)) + ) } if (metadata) { product.metadata = setMetadata(product, metadata) } - if (typeof type !== `undefined`) { - product.type_id = (await productTypeRepo.upsertType(type))?.id || null + if (isDefined(type)) { + promises.push( + productTypeRepo + .upsertType(type) + .then((type) => (product.type_id = type?.id ?? null)) + ) } if (tags) { - product.tags = await productTagRepo.upsertTags(tags) + promises.push( + productTagRepo.upsertTags(tags).then((tags) => (product.tags = tags)) + ) } if (isDefined(categories)) { @@ -562,11 +571,9 @@ class ProductService extends TransactionBaseService { if (categories?.length) { const categoryIds = categories.map((c) => c.id) - const categoryRecords = categoryIds.map( + product.categories = categoryIds.map( (id) => ({ id } as ProductCategory) ) - - product.categories = categoryRecords } } @@ -590,6 +597,8 @@ class ProductService extends TransactionBaseService { } } + await Promise.all(promises) + const result = await productRepo.save(product) await this.eventBus_ @@ -598,6 +607,7 @@ class ProductService extends TransactionBaseService { id: result.id, fields: Object.keys(update), }) + return result }) } diff --git a/packages/medusa/src/strategies/batch-jobs/product/import.ts b/packages/medusa/src/strategies/batch-jobs/product/import.ts index 042e987bcb..f16e00e8b7 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/import.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/import.ts @@ -12,14 +12,11 @@ import { ProductVariantService, RegionService, SalesChannelService, - ShippingProfileService + ShippingProfileService, } from "../../../services" import CsvParser from "../../../services/csv-parser" import { CreateProductInput } from "../../../types/product" -import { - CreateProductVariantInput, - UpdateProductVariantInput -} from "../../../types/product-variant" +import { CreateProductVariantInput } from "../../../types/product-variant" import { FlagRouter } from "../../../utils/flag-router" import { OperationType, @@ -27,11 +24,11 @@ import { ProductImportCsvSchema, ProductImportInjectedProps, ProductImportJobContext, - TParsedProductImportRowData + TParsedProductImportRowData, } from "./types" import { productImportColumnsDefinition, - productImportSalesChannelsColumnsDefinition + productImportSalesChannelsColumnsDefinition, } from "./types/columns-definition" import { transformProductData, transformVariantData } from "./utils" @@ -584,12 +581,13 @@ class ProductImportStrategy extends AbstractBatchJobStrategy { await this.prepareVariantOptions(variantOp, product.id) + const updateData = transformVariantData(variantOp) + delete updateData.product + delete updateData["product.handle"] + await this.productVariantService_ .withTransaction(transactionManager) - .update( - variantOp["variant.id"] as string, - transformVariantData(variantOp) as UpdateProductVariantInput - ) + .update(variantOp["variant.id"] as string, updateData) } catch (e) { ProductImportStrategy.throwDescriptiveError(variantOp, e.message) } diff --git a/packages/medusa/src/types/product-variant.ts b/packages/medusa/src/types/product-variant.ts index 2407765801..3a09841679 100644 --- a/packages/medusa/src/types/product-variant.ts +++ b/packages/medusa/src/types/product-variant.ts @@ -12,8 +12,10 @@ import { DateComparisonOperator, NumericalComparisonOperator, StringComparisonOperator, + WithRequiredProperty, } from "./common" import { XorConstraint } from "./validators/xor" +import { ProductVariant } from "../models" export type ProductVariantPrice = { id?: string @@ -84,6 +86,30 @@ export type UpdateProductVariantInput = { metadata?: Record } +export type UpdateProductVariantData = { + variant: ProductVariant + updateData: UpdateProductVariantInput +} + +export type UpdateVariantPricesData = { + variantId: string + prices: ProductVariantPrice[] +} + +export type UpdateVariantRegionPriceData = { + variantId: string + price: { + currency_code: string + region_id: string + amount: number + } +} + +export type UpdateVariantCurrencyPriceData = { + variantId: string + price: WithRequiredProperty +} + export class FilterableProductVariantProps { @ValidateNested() @IsType([String, [String], StringComparisonOperator]) diff --git a/packages/medusa/src/utils/__tests__/has-changes.spec.ts b/packages/medusa/src/utils/__tests__/has-changes.spec.ts new file mode 100644 index 0000000000..ac543f5fe5 --- /dev/null +++ b/packages/medusa/src/utils/__tests__/has-changes.spec.ts @@ -0,0 +1,59 @@ +import { hasChanges } from "../has-changes" + +describe("hasChanges", function () { + it("should return true the data differ and false otherwise", () => { + const objToCompareTo = { + prop1: "test", + prop2: "test", + prop3: "test", + prop4: { + prop4_1: "test", + prop4_2: "test", + prop4_3: "test", + }, + } + + const obj = { + prop1: "test", + prop2: "test", + prop3: "test", + prop4: { + prop4_1: "test", + prop4_2: "test", + prop4_3: "test", + }, + } + + let res = hasChanges(objToCompareTo, obj) + expect(res).toBeFalsy() + + const obj2 = { + ...obj, + prop3: "tes", + } + + res = hasChanges(objToCompareTo, obj2) + expect(res).toBeTruthy() + + const obj3 = { + ...obj, + prop4: { + prop4_1: "", + prop4_2: "test", + prop4_3: "test", + }, + } + + res = hasChanges(objToCompareTo, obj3) + expect(res).toBeTruthy() + + const obj4 = { + ...obj, + } + /* @ts-ignore */ + delete obj4.prop4 + + res = hasChanges(objToCompareTo, obj4) + expect(res).toBeFalsy() + }) +}) diff --git a/packages/medusa/src/utils/has-changes.ts b/packages/medusa/src/utils/has-changes.ts new file mode 100644 index 0000000000..c353149a3f --- /dev/null +++ b/packages/medusa/src/utils/has-changes.ts @@ -0,0 +1,23 @@ +import { isObject } from "./is-object" + +/** + * Compare two objects and return true if there is changes detected from obj2 compared to obj1 + * @param obj1 + * @param obj2 + */ +export function hasChanges( + obj1: T1, + obj2: T2 +): boolean { + for (const [key, value] of Object.entries(obj2)) { + if (isObject(obj1[key])) { + return hasChanges(obj1[key], value) + } + + if (obj1[key] !== value) { + return true + } + } + + return false +} diff --git a/packages/medusa/src/utils/index.ts b/packages/medusa/src/utils/index.ts index d78a7356c7..432bf399c8 100644 --- a/packages/medusa/src/utils/index.ts +++ b/packages/medusa/src/utils/index.ts @@ -8,4 +8,5 @@ export * from "./calculate-price-tax-amount" export * from "./csv-cell-content-formatter" export * from "./exception-formatter" export * from "./db-aware-column" -export * from "./row-sql-results-to-entity-transformer" +export * from "./is-object" +export * from "./has-changes" diff --git a/packages/medusa/src/utils/is-object.ts b/packages/medusa/src/utils/is-object.ts new file mode 100644 index 0000000000..778a601412 --- /dev/null +++ b/packages/medusa/src/utils/is-object.ts @@ -0,0 +1,3 @@ +export function isObject(obj: unknown): obj is object { + return typeof obj === "object" && !!obj +} diff --git a/packages/medusa/src/utils/row-sql-results-to-entity-transformer.ts b/packages/medusa/src/utils/row-sql-results-to-entity-transformer.ts deleted file mode 100644 index a91e5ed46e..0000000000 --- a/packages/medusa/src/utils/row-sql-results-to-entity-transformer.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { RelationIdLoader } from "typeorm/query-builder/relation-id/RelationIdLoader" -import { RawSqlResultsToEntityTransformer } from "typeorm/query-builder/transformer/RawSqlResultsToEntityTransformer" -import { QueryBuilder, QueryRunner } from "typeorm" - -export async function rowSqlResultsToEntityTransformer( - rows: any[], - queryBuilder: QueryBuilder, - queryRunner: QueryRunner -): Promise { - const relationIdLoader = new RelationIdLoader( - queryBuilder.connection, - queryRunner, - queryBuilder.expressionMap.relationIdAttributes - ) - const transformer = new RawSqlResultsToEntityTransformer( - queryBuilder.expressionMap, - queryBuilder.connection.driver, - [], - [], - queryRunner - ) - - return transformer.transform( - rows, - queryBuilder.expressionMap.mainAlias! - ) as T[] -}