chore(): Improve internal repository delete algo (#11601)

* chore(): Improve internal repository delete algo

* chore(): Improve internal repository delete algo

* chore(): Improve internal repository delete algo

* update tests

* Create purple-donkeys-learn.md

* update tests

.changeset/purple-donkeys-learn.md

@@ -0,0 +1,9 @@
+---
+"@medusajs/inventory": patch
+"@medusajs/link-modules": patch
+"@medusajs/product": patch
+"@medusajs/types": patch
+"@medusajs/utils": patch
+---
+
+chore(): Improve internal repository delete algo

packages/core/types/src/dal/repository-service.ts

@@ -70,7 +70,10 @@ export interface RepositoryService<T = any> extends BaseRepositoryService {
     context?: Context
   ): Promise<InferRepositoryReturnType<T>[]>
 
-  delete(idsOrPKs: FindOptions<T>["where"], context?: Context): Promise<void>
+  delete(
+    idsOrPKs: FindOptions<T>["where"],
+    context?: Context
+  ): Promise<string[]>
 
   /**
    * Soft delete entities and cascade to related entities if configured.
@@ -127,7 +130,7 @@ export interface TreeRepositoryService<T = any> extends BaseRepositoryService {
     context?: Context
   ): Promise<InferRepositoryReturnType<T>[]>
 
-  delete(ids: string[], context?: Context): Promise<void>
+  delete(ids: string[], context?: Context): Promise<string[]>
 }
 
 /**

packages/core/types/src/modules-sdk/medusa-internal-service.ts

@@ -64,16 +64,16 @@ export interface IMedusaInternalService<
     sharedContext?: Context
   ): Promise<InferEntityType<TEntity>[]>
 
-  delete(idOrSelector: string, sharedContext?: Context): Promise<void>
+  delete(idOrSelector: string, sharedContext?: Context): Promise<string[]>
-  delete(idOrSelector: string[], sharedContext?: Context): Promise<void>
+  delete(idOrSelector: string[], sharedContext?: Context): Promise<string[]>
-  delete(idOrSelector: object, sharedContext?: Context): Promise<void>
+  delete(idOrSelector: object, sharedContext?: Context): Promise<string[]>
-  delete(idOrSelector: object[], sharedContext?: Context): Promise<void>
+  delete(idOrSelector: object[], sharedContext?: Context): Promise<string[]>
   delete(
     idOrSelector: {
       selector: FilterQuery<any> | BaseFilterable<FilterQuery<any>>
     },
     sharedContext?: Context
-  ): Promise<void>
+  ): Promise<string[]>
 
   softDelete(
     idsOrFilter:

packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts

@@ -162,7 +162,10 @@ export class MikroOrmBaseRepository<const T extends object = object>
     throw new Error("Method not implemented.")
   }
 
-  delete(idsOrPKs: FindOptions<T>["where"], context?: Context): Promise<void> {
+  delete(
+    idsOrPKs: FindOptions<T>["where"],
+    context?: Context
+  ): Promise<string[]> {
     throw new Error("Method not implemented.")
   }
 
@@ -285,7 +288,7 @@ export class MikroOrmBaseTreeRepository<
     throw new Error("Method not implemented.")
   }
 
-  delete(ids: string[], context?: Context): Promise<void> {
+  delete(ids: string[], context?: Context): Promise<string[]> {
     throw new Error("Method not implemented.")
   }
 }
@@ -301,6 +304,10 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
 
   class MikroOrmAbstractBaseRepository_ extends MikroOrmBaseRepository<T> {
     entity = mikroOrmEntity
+    tableName = (
+      (mikroOrmEntity as unknown as EntitySchema).meta ??
+      (mikroOrmEntity as EntityClass<any>).prototype.__meta
+    ).collection
 
     // @ts-ignore
     constructor(...args: any[]) {
@@ -428,9 +435,29 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
     async delete(
       filters: FindOptions<T>["where"],
       context?: Context
-    ): Promise<void> {
+    ): Promise<string[]> {
-      const manager = this.getActiveManager<EntityManager>(context)
+      const manager = this.getActiveManager<SqlEntityManager>(context)
-      await manager.nativeDelete<T>(this.entity, filters)
+
+      const whereSqlInfo = manager
+        .createQueryBuilder(this.entity.name, this.tableName)
+        .where(filters)
+        .getKnexQuery()
+        .toSQL()
+
+      const where = [
+        whereSqlInfo.sql.split("where ")[1],
+        whereSqlInfo.bindings,
+      ] as [string, any[]]
+
+      return await (manager.getTransactionContext() ?? manager.getKnex())
+        .queryBuilder()
+        .from(this.tableName)
+        .delete()
+        .where(manager.getKnex().raw(...where))
+        .returning("id")
+        .then((rows: { id: string }[]) => {
+          return rows.map((row: { id: string }) => row.id)
+        })
     }
 
     async find(

packages/core/utils/src/modules-sdk/__tests__/medusa-internal-service.ts

@@ -267,12 +267,7 @@ describe("Internal Module Service Factory", () => {
     })
 
     it("should delete entities successfully", async () => {
-      const entitiesToDelete = [{ id: "1", name: "Item" }]
+      await instance.delete({ selector: { id: "1" } })
-      containerMock[modelRepositoryName].find.mockResolvedValueOnce(
-        entitiesToDelete
-      )
-
-      await instance.delete({ selector: {} })
       expect(containerMock[modelRepositoryName].delete).toHaveBeenCalledWith(
         {
           $or: [

packages/core/utils/src/modules-sdk/__tests__/medusa-service.spec.ts

@@ -29,21 +29,21 @@ describe("Abstract Module Service Factory", () => {
     mainModelMockService: {
       retrieve: jest.fn().mockResolvedValue({ id: "1", name: "Item" }),
       list: jest.fn().mockResolvedValue([{ id: "1", name: "Item" }]),
-      delete: jest.fn().mockResolvedValue(undefined),
+      delete: jest.fn().mockResolvedValue([]),
       softDelete: jest.fn().mockResolvedValue([[], {}]),
       restore: jest.fn().mockResolvedValue([[], {}]),
     },
     otherModelMock1Service: {
       retrieve: jest.fn().mockResolvedValue({ id: "1", name: "Item" }),
       list: jest.fn().mockResolvedValue([{ id: "1", name: "Item" }]),
-      delete: jest.fn().mockResolvedValue(undefined),
+      delete: jest.fn().mockResolvedValue([]),
       softDelete: jest.fn().mockResolvedValue([[], {}]),
       restore: jest.fn().mockResolvedValue([[], {}]),
     },
     otherModelMock2Service: {
       retrieve: jest.fn().mockResolvedValue({ id: "1", name: "Item" }),
       list: jest.fn().mockResolvedValue([{ id: "1", name: "Item" }]),
-      delete: jest.fn().mockResolvedValue(undefined),
+      delete: jest.fn().mockResolvedValue([]),
       softDelete: jest.fn().mockResolvedValue([[], {}]),
       restore: jest.fn().mockResolvedValue([[], {}]),
     },

packages/core/utils/src/modules-sdk/medusa-internal-service.ts

@@ -367,16 +367,16 @@ export function MedusaInternalService<
       )
     }
 
-    delete(idOrSelector: string, sharedContext?: Context): Promise<void>
+    delete(idOrSelector: string, sharedContext?: Context): Promise<string[]>
-    delete(idOrSelector: string[], sharedContext?: Context): Promise<void>
+    delete(idOrSelector: string[], sharedContext?: Context): Promise<string[]>
-    delete(idOrSelector: object, sharedContext?: Context): Promise<void>
+    delete(idOrSelector: object, sharedContext?: Context): Promise<string[]>
-    delete(idOrSelector: object[], sharedContext?: Context): Promise<void>
+    delete(idOrSelector: object[], sharedContext?: Context): Promise<string[]>
     delete(
       idOrSelector: {
         selector: FilterQuery<any> | BaseFilterable<FilterQuery<any>>
       },
       sharedContext?: Context
-    ): Promise<void>
+    ): Promise<string[]>
 
     @InjectTransactionManager(propertyRepositoryName)
     async delete(
@@ -389,12 +389,12 @@ export function MedusaInternalService<
             selector: FilterQuery<any> | BaseFilterable<FilterQuery<any>>
           },
       @MedusaContext() sharedContext: Context = {}
-    ): Promise<void> {
+    ): Promise<string[]> {
       if (
         !isDefined(idOrSelector) ||
         (Array.isArray(idOrSelector) && !idOrSelector.length)
       ) {
-        return
+        return []
       }
 
       const primaryKeys = AbstractService_.retrievePrimaryKeys(model)
@@ -420,21 +420,7 @@ export function MedusaInternalService<
       }
 
       if (isObject(idOrSelector) && "selector" in idOrSelector) {
-        const entitiesToDelete = await this.list(
+        deleteCriteria.$or.push(idOrSelector.selector)
-          idOrSelector.selector as FilterQuery<any>,
-          {
-            select: primaryKeys,
-          },
-          sharedContext
-        )
-
-        for (const entity of entitiesToDelete) {
-          const criteria = {}
-          primaryKeys.forEach((key) => {
-            criteria[key] = entity[key]
-          })
-          deleteCriteria.$or.push(criteria)
-        }
       } else {
         const primaryKeysValues = Array.isArray(idOrSelector)
           ? idOrSelector
@@ -451,34 +437,18 @@ export function MedusaInternalService<
             criteria[primaryKeys[0]] = primaryKeyValue
           }
 
-          // TODO: Revisit
-          /*primaryKeys.forEach((key) => {
-            /!*if (
-              isObject(primaryKeyValue) &&
-              !isDefined(primaryKeyValue[key]) &&
-              // primaryKeys.length > 1
-            ) {
-              throw new MedusaError(
-                MedusaError.Types.INVALID_DATA,
-                `Composite key must contain all primary key fields: ${primaryKeys.join(
-                  ", "
-                )}. Found: ${Object.keys(primaryKeyValue)}`
-              )
-            }*!/
-
-            criteria[key] = isObject(primaryKeyValue)
-              ? primaryKeyValue[key]
-              : primaryKeyValue
-          })*/
           return criteria
         })
       }
 
       if (!deleteCriteria.$or.length) {
-        return
+        return []
       }
 
-      await this[propertyRepositoryName].delete(deleteCriteria, sharedContext)
+      return await this[propertyRepositoryName].delete(
+        deleteCriteria,
+        sharedContext
+      )
     }
 
     @InjectTransactionManager(propertyRepositoryName)

packages/core/utils/src/modules-sdk/medusa-service.ts

@@ -277,18 +277,16 @@ export function MedusaService<
             ? primaryKeyValues
             : [primaryKeyValues]
 
-          await this.__container__[serviceRegistrationName].delete(
+          const ids = await this.__container__[serviceRegistrationName].delete(
             primaryKeyValues_,
             sharedContext
           )
 
-          primaryKeyValues_.map((primaryKeyValue) =>
+          ids.map((id) =>
             klassPrototype.aggregatedEvents.bind(this)({
               action: CommonEvents.DELETED,
               object: camelToSnakeCase(modelName).toLowerCase(),
-              data: isString(primaryKeyValue)
+              data: isString(id) ? { id: id } : id,
-                ? { id: primaryKeyValue }
-                : primaryKeyValue,
               context: sharedContext,
             })
           )

packages/modules/inventory/src/services/inventory-module.ts

@@ -547,7 +547,7 @@ export default class InventoryModuleService
       return
     }
 
-    return await this.inventoryLevelService_.delete(inventoryLevel.id, context)
+    await this.inventoryLevelService_.delete(inventoryLevel.id, context)
   }
 
   // @ts-ignore

packages/modules/link-modules/src/repositories/link.ts

@@ -17,7 +17,7 @@ export function getLinkRepository(model: EntitySchema) {
       this.joinerConfig_ = joinerConfig
     }
 
-    async delete(data: any, context: Context = {}): Promise<void> {
+    async delete(data: any, context: Context = {}): Promise<string[]> {
       const filter = {}
       for (const key in data) {
         filter[key] = {
@@ -25,8 +25,7 @@ export function getLinkRepository(model: EntitySchema) {
         }
       }
 
-      const manager = this.getActiveManager<SqlEntityManager>(context)
+      return await super.delete(filter, context)
-      await manager.nativeDelete(model, data, {})
     }
 
     async create(data: object[], context: Context = {}): Promise<object[]> {

packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts

@@ -1312,9 +1312,12 @@ moduleIntegrationTestRunner<IProductModuleService>({
             relations: ["images"],
           })
 
-          const retrievedProductAgain = await service.retrieveProduct(product.id, {
+          const retrievedProductAgain = await service.retrieveProduct(
+            product.id,
+            {
               relations: ["images"],
-          })
+            }
+          )
 
           expect(retrievedProduct.images).toEqual(retrievedProductAgain.images)
 
@@ -1332,7 +1335,9 @@ moduleIntegrationTestRunner<IProductModuleService>({
           // Explicitly verify sequential order
           retrievedProduct.images.forEach((img, idx) => {
             if (idx > 0) {
-              expect(img.rank).toBeGreaterThan(retrievedProduct.images[idx - 1].rank)
+              expect(img.rank).toBeGreaterThan(
+                retrievedProduct.images[idx - 1].rank
+              )
             }
           })
         })

packages/modules/product/package.json

@@ -29,7 +29,7 @@
     "resolve:aliases": "tsc --showConfig -p tsconfig.json > tsconfig.resolved.json && tsc-alias -p tsconfig.resolved.json && rimraf tsconfig.resolved.json",
     "build": "rimraf dist && tsc --build && npm run resolve:aliases",
     "test": "jest --runInBand --bail --forceExit -- src/**/__tests__/**/*.ts",
-    "test:integration": "jest --forceExit",
+    "test:integration": "jest --forceExit -- integration-tests/__tests__/**/*.ts",
     "migration:initial": " MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:create --initial",
     "migration:create": " MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:create",
     "migration:up": " MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:up",

packages/modules/product/src/repositories/product-category.ts

@@ -272,10 +272,11 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
     return [this.sortCategoriesByRank(categoriesTree), count]
   }
 
-  async delete(ids: string[], context: Context = {}): Promise<void> {
+  async delete(ids: string[], context: Context = {}): Promise<string[]> {
     const manager = super.getActiveManager<SqlEntityManager>(context)
     await this.baseDelete(ids, context)
     await manager.nativeDelete(ProductCategory.name, { id: ids }, {})
+    return ids
   }
 
   async softDelete(

packages/modules/product/src/services/product-category.ts

@@ -163,8 +163,8 @@ export default class ProductCategoryService {
   async delete(
     ids: string[],
     @MedusaContext() sharedContext: Context = {}
-  ): Promise<void> {
+  ): Promise<string[]> {
-    await this.productCategoryRepository_.delete(ids, sharedContext)
+    return await this.productCategoryRepository_.delete(ids, sharedContext)
   }
 
   @InjectTransactionManager("productCategoryRepository_")