From e0b14519f1c9c4d2888f78d06dc108da7f3783f2 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 18 Jun 2024 12:58:54 +0200 Subject: [PATCH] fix: Medusa service base method transaction (#7758) **What** Remove transaction decorator from the base medusa service method, the transaction will always be coming from the shared context. It fixes the issue that when you consume a base method directly it will return a proper tuple from the DB instead of the one from the entity map cc @VariableVic **NOTE** This pr also fix some categories issues in the product module which was preventing the tests from working. if @sradevski you could have a look later, in the mean time we can still merge it FIXES CORE-2342 --- .../__tests__/medusa-service.spec.ts | 2 +- .../utils/src/modules-sdk/medusa-service.ts | 40 ++++--------------- .../__tests__/product-category.ts | 4 +- .../product-categories.spec.ts | 2 +- .../src/repositories/product-category.ts | 28 +++++++++++-- .../product/src/services/product-category.ts | 4 +- .../integration-tests/__tests__/index.spec.ts | 2 +- .../integration-tests/__tests__/index.spec.ts | 2 +- 8 files changed, 42 insertions(+), 42 deletions(-) diff --git a/packages/core/utils/src/modules-sdk/__tests__/medusa-service.spec.ts b/packages/core/utils/src/modules-sdk/__tests__/medusa-service.spec.ts index c0a30e1744..c7c92451ea 100644 --- a/packages/core/utils/src/modules-sdk/__tests__/medusa-service.spec.ts +++ b/packages/core/utils/src/modules-sdk/__tests__/medusa-service.spec.ts @@ -9,7 +9,7 @@ const baseRepoMock = { const defaultContext = { __type: "MedusaContext", manager: baseRepoMock } const defaultTransactionContext = { __type: "MedusaContext", - transactionManager: "transactionManager", + manager: baseRepoMock, } describe("Abstract Module Service Factory", () => { diff --git a/packages/core/utils/src/modules-sdk/medusa-service.ts b/packages/core/utils/src/modules-sdk/medusa-service.ts index 112b6721a3..a46aac5963 100644 --- a/packages/core/utils/src/modules-sdk/medusa-service.ts +++ b/packages/core/utils/src/modules-sdk/medusa-service.ts @@ -20,11 +20,7 @@ import { pluralize, upperCaseFirst, } from "../common" -import { - InjectManager, - InjectTransactionManager, - MedusaContext, -} from "./decorators" +import { InjectManager, MedusaContext } from "./decorators" import { ModuleRegistrationName } from "./definition" type BaseMethods = @@ -384,11 +380,7 @@ export function MedusaService< MedusaContext()(klassPrototype, methodName, contextIndex) - const ManagerDecorator = readMethods.includes(method as BaseMethods) - ? InjectManager - : InjectTransactionManager - - ManagerDecorator("baseRepository_")( + InjectManager("baseRepository_")( klassPrototype, methodName, descriptorMockRef @@ -413,9 +405,7 @@ export function MedusaService< serviceRegistrationName ].retrieve(id, config, sharedContext) - return await this.baseRepository_.serialize(entities, { - populate: true, - }) + return await this.baseRepository_.serialize(entities) } applyMethod(methodImplementation, 2) @@ -432,9 +422,7 @@ export function MedusaService< const entities = await service.create(serviceData, sharedContext) const response = Array.isArray(data) ? entities : entities[0] - return await this.baseRepository_.serialize(response, { - populate: true, - }) + return await this.baseRepository_.serialize(response) } applyMethod(methodImplementation, 1) @@ -451,9 +439,7 @@ export function MedusaService< const entities = await service.update(serviceData, sharedContext) const response = Array.isArray(data) ? entities : entities[0] - return await this.baseRepository_.serialize(response, { - populate: true, - }) + return await this.baseRepository_.serialize(response) } applyMethod(methodImplementation, 1) @@ -469,9 +455,7 @@ export function MedusaService< const service = this.__container__[serviceRegistrationName] const entities = await service.list(filters, config, sharedContext) - return await this.baseRepository_.serialize(entities, { - populate: true, - }) + return await this.baseRepository_.serialize(entities) } applyMethod(methodImplementation, 2) @@ -488,12 +472,7 @@ export function MedusaService< serviceRegistrationName ].listAndCount(filters, config, sharedContext) - return [ - await this.baseRepository_.serialize(entities, { - populate: true, - }), - count, - ] + return [await this.baseRepository_.serialize(entities), count] } applyMethod(methodImplementation, 2) @@ -542,10 +521,7 @@ export function MedusaService< ].softDelete(primaryKeyValues_, sharedContext) const softDeletedEntities = await this.baseRepository_.serialize( - entities, - { - populate: true, - } + entities ) await this.eventBusModuleService_?.emit( diff --git a/packages/modules/product/integration-tests/__tests__/product-category.ts b/packages/modules/product/integration-tests/__tests__/product-category.ts index 95aa671336..082b1df6f0 100644 --- a/packages/modules/product/integration-tests/__tests__/product-category.ts +++ b/packages/modules/product/integration-tests/__tests__/product-category.ts @@ -1015,7 +1015,7 @@ moduleIntegrationTestRunner({ } expect(error.message).toEqual( - `ProductCategory not found ({ id: 'does-not-exist' })` + `ProductCategory with id: does-not-exist was not found` ) }) @@ -1162,7 +1162,7 @@ moduleIntegrationTestRunner({ } expect(error.message).toEqual( - `ProductCategory not found ({ id: 'does-not-exist' })` + `ProductCategory with id: does-not-exist was not found` ) }) diff --git a/packages/modules/product/integration-tests/__tests__/product-module-service/product-categories.spec.ts b/packages/modules/product/integration-tests/__tests__/product-module-service/product-categories.spec.ts index 1b8064ae1a..4bda341642 100644 --- a/packages/modules/product/integration-tests/__tests__/product-module-service/product-categories.spec.ts +++ b/packages/modules/product/integration-tests/__tests__/product-module-service/product-categories.spec.ts @@ -565,7 +565,7 @@ moduleIntegrationTestRunner({ } expect(error.message).toEqual( - `ProductCategory not found ({ id: 'does-not-exist' })` + `ProductCategory with id: does-not-exist was not found` ) }) diff --git a/packages/modules/product/src/repositories/product-category.ts b/packages/modules/product/src/repositories/product-category.ts index 9ac171d828..9d4101dd0d 100644 --- a/packages/modules/product/src/repositories/product-category.ts +++ b/packages/modules/product/src/repositories/product-category.ts @@ -274,9 +274,17 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito const categories = await Promise.all( ids.map(async (id) => { - const productCategory = await manager.findOneOrFail(ProductCategory, { + const productCategory = await manager.findOne(ProductCategory, { id, }) + + if (!productCategory) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `ProductCategory with id: ${id} was not found` + ) + } + manager.assign(productCategory, { deleted_at: new Date() }) return productCategory }) @@ -310,7 +318,7 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito await Promise.all( ids.map(async (id) => { - const productCategory = await manager.findOneOrFail( + const productCategory = await manager.findOne( ProductCategory, { id }, { @@ -318,6 +326,13 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito } ) + if (!productCategory) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `ProductCategory with id: ${id} was not found` + ) + } + if (productCategory.category_children.length > 0) { throw new MedusaError( MedusaError.Types.NOT_ALLOWED, @@ -389,10 +404,17 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito const categories = await Promise.all( data.map(async (entry, i) => { const categoryData: Partial = { ...entry } - const productCategory = await manager.findOneOrFail(ProductCategory, { + const productCategory = await manager.findOne(ProductCategory, { id: categoryData.id, }) + if (!productCategory) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `ProductCategory with id: ${categoryData.id} was not found` + ) + } + // If the parent or rank are not changed, no need to reorder anything. if ( !isDefined(categoryData.parent_category_id) && diff --git a/packages/modules/product/src/services/product-category.ts b/packages/modules/product/src/services/product-category.ts index 86ce979ce7..4ba1b685ce 100644 --- a/packages/modules/product/src/services/product-category.ts +++ b/packages/modules/product/src/services/product-category.ts @@ -3,10 +3,10 @@ import { FreeTextSearchFilterKey, InjectManager, InjectTransactionManager, + isDefined, MedusaContext, MedusaError, ModulesSdkUtils, - isDefined, } from "@medusajs/utils" import { ProductCategory } from "@models" import { ProductCategoryRepository } from "@repositories" @@ -169,6 +169,7 @@ export default class ProductCategoryService< await this.productCategoryRepository_.delete(ids, sharedContext) } + @InjectTransactionManager("productCategoryRepository_") async softDelete( ids: string[], @MedusaContext() sharedContext?: Context @@ -178,6 +179,7 @@ export default class ProductCategoryService< ).softDelete(ids, sharedContext)) as any } + @InjectTransactionManager("productCategoryRepository_") async restore( ids: string[], @MedusaContext() sharedContext?: Context diff --git a/packages/modules/workflow-engine-inmemory/integration-tests/__tests__/index.spec.ts b/packages/modules/workflow-engine-inmemory/integration-tests/__tests__/index.spec.ts index a5a1b983bb..5646a4fdcf 100644 --- a/packages/modules/workflow-engine-inmemory/integration-tests/__tests__/index.spec.ts +++ b/packages/modules/workflow-engine-inmemory/integration-tests/__tests__/index.spec.ts @@ -31,7 +31,7 @@ const afterEach_ = async () => { await TestDatabase.clearTables(sharedPgConnection) } -jest.setTimeout(50000) +jest.setTimeout(100000) describe("Workflow Orchestrator module", function () { let workflowOrcModule: IWorkflowEngineService diff --git a/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts b/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts index 6eb789b5e3..da3a061615 100644 --- a/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts +++ b/packages/modules/workflow-engine-redis/integration-tests/__tests__/index.spec.ts @@ -12,7 +12,7 @@ import "../__fixtures__" import { createScheduled } from "../__fixtures__/workflow_scheduled" import { DB_URL, TestDatabase } from "../utils" -jest.setTimeout(50000) +jest.setTimeout(100000) const sharedPgConnection = knex({ client: "pg",