chore(): Prevent from soft deleting all entities (#6396)

**What**
The internal service abstraction should return idempotently if an empty array is given in order to prevent full soft deletion of the full data
This commit is contained in:
Adrien de Peretti
2024-02-14 15:29:10 +01:00
committed by GitHub
parent 4d51f095b3
commit 6500f18b9b
4 changed files with 54 additions and 28 deletions

View File

@@ -0,0 +1,5 @@
---
"@medusajs/utils": patch
---
chore(): Prevent from soft deleting all entities

View File

@@ -85,7 +85,7 @@ describe("Abstract Module Service Factory", () => {
instance = new abstractModuleService(containerMock)
})
test("should have retrieve method", async () => {
it("should have retrieve method", async () => {
const result = await instance.retrieve("1")
expect(result).toEqual({ id: "1", name: "Item" })
expect(containerMock.mainModelMockService.retrieve).toHaveBeenCalledWith(
@@ -95,7 +95,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have list method", async () => {
it("should have list method", async () => {
const result = await instance.list()
expect(result).toEqual([{ id: "1", name: "Item" }])
expect(containerMock.mainModelMockService.list).toHaveBeenCalledWith(
@@ -105,7 +105,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have delete method", async () => {
it("should have delete method", async () => {
await instance.delete("1")
expect(containerMock.mainModelMockService.delete).toHaveBeenCalledWith(
["1"],
@@ -113,7 +113,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have softDelete method", async () => {
it("should have softDelete method", async () => {
const result = await instance.softDelete("1")
expect(result).toEqual(undefined)
expect(
@@ -121,7 +121,7 @@ describe("Abstract Module Service Factory", () => {
).toHaveBeenCalledWith(["1"], defaultTransactionContext)
})
test("should have restore method", async () => {
it("should have restore method", async () => {
const result = await instance.restore("1")
expect(result).toEqual(undefined)
expect(containerMock.mainModelMockService.restore).toHaveBeenCalledWith(
@@ -130,7 +130,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have delete method with selector", async () => {
it("should have delete method with selector", async () => {
await instance.delete({ selector: { id: "1" } })
expect(containerMock.mainModelMockService.delete).toHaveBeenCalledWith(
[{ selector: { id: "1" } }],
@@ -147,7 +147,7 @@ describe("Abstract Module Service Factory", () => {
instance = new abstractModuleService(containerMock)
})
test("should have retrieve method for other models", async () => {
it("should have retrieve method for other models", async () => {
const result = await instance.retrieveOtherModelMock1("1")
expect(result).toEqual({ id: "1", name: "Item" })
expect(
@@ -155,7 +155,7 @@ describe("Abstract Module Service Factory", () => {
).toHaveBeenCalledWith("1", undefined, defaultContext)
})
test("should have list method for other models", async () => {
it("should have list method for other models", async () => {
const result = await instance.listOtherModelMock1s()
expect(result).toEqual([{ id: "1", name: "Item" }])
expect(containerMock.otherModelMock1Service.list).toHaveBeenCalledWith(
@@ -165,7 +165,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have delete method for other models", async () => {
it("should have delete method for other models", async () => {
await instance.deleteOtherModelMock1s("1")
expect(containerMock.otherModelMock1Service.delete).toHaveBeenCalledWith(
["1"],
@@ -173,7 +173,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have softDelete method for other models", async () => {
it("should have softDelete method for other models", async () => {
const result = await instance.softDeleteOtherModelMock1s("1")
expect(result).toEqual(undefined)
expect(
@@ -181,7 +181,7 @@ describe("Abstract Module Service Factory", () => {
).toHaveBeenCalledWith(["1"], defaultTransactionContext)
})
test("should have restore method for other models", async () => {
it("should have restore method for other models", async () => {
const result = await instance.restoreOtherModelMock1s("1")
expect(result).toEqual(undefined)
expect(containerMock.otherModelMock1Service.restore).toHaveBeenCalledWith(
@@ -190,7 +190,7 @@ describe("Abstract Module Service Factory", () => {
)
})
test("should have delete method for other models with selector", async () => {
it("should have delete method for other models with selector", async () => {
await instance.deleteOtherModelMock1s({ selector: { id: "1" } })
expect(containerMock.otherModelMock1Service.delete).toHaveBeenCalledWith(
[{ selector: { id: "1" } }],

View File

@@ -44,12 +44,12 @@ describe("Internal Module Service Factory", () => {
instance = new internalModuleService(containerMock)
})
test("should throw model id undefined error on retrieve if id is not defined", async () => {
it("should throw model id undefined error on retrieve if id is not defined", async () => {
const err = await instance.retrieve().catch((e) => e)
expect(err.message).toBe("model - id must be defined")
})
test("should throw an error on retrieve if composite key values are not defined", async () => {
it("should throw an error on retrieve if composite key values are not defined", async () => {
class CompositeModel {
id: string
name: string
@@ -66,14 +66,14 @@ describe("Internal Module Service Factory", () => {
expect(err.message).toBe("compositeModel - id, name must be defined")
})
test("should throw NOT_FOUND error on retrieve if entity not found", async () => {
it("should throw NOT_FOUND error on retrieve if entity not found", async () => {
containerMock[modelRepositoryName].find.mockResolvedValueOnce([])
const err = await instance.retrieve("1").catch((e) => e)
expect(err.message).toBe("Model with id: 1 was not found")
})
test("should retrieve entity successfully", async () => {
it("should retrieve entity successfully", async () => {
const entity = { id: "1", name: "Item" }
containerMock[modelRepositoryName].find.mockResolvedValueOnce([entity])
@@ -81,7 +81,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual(entity)
})
test("should retrieve entity successfully with composite key", async () => {
it("should retrieve entity successfully with composite key", async () => {
class CompositeModel {
id: string
name: string
@@ -103,7 +103,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual(entity)
})
test("should list entities successfully", async () => {
it("should list entities successfully", async () => {
const entities = [
{ id: "1", name: "Item" },
{ id: "2", name: "Item2" },
@@ -114,7 +114,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual(entities)
})
test("should list and count entities successfully", async () => {
it("should list and count entities successfully", async () => {
const entities = [
{ id: "1", name: "Item" },
{ id: "2", name: "Item2" },
@@ -129,7 +129,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual([entities, count])
})
test("should create entity successfully", async () => {
it("should create entity successfully", async () => {
const entity = { id: "1", name: "Item" }
containerMock[modelRepositoryName].find.mockReturnValue([entity])
@@ -142,7 +142,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual(entity)
})
test("should create entities successfully", async () => {
it("should create entities successfully", async () => {
const entities = [
{ id: "1", name: "Item" },
{ id: "2", name: "Item2" },
@@ -156,7 +156,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual(entities)
})
test("should update entity successfully", async () => {
it("should update entity successfully", async () => {
const updateData = { id: "1", name: "UpdatedItem" }
containerMock[modelRepositoryName].find.mockResolvedValueOnce([
@@ -171,7 +171,7 @@ describe("Internal Module Service Factory", () => {
expect(result).toEqual([updateData])
})
test("should update entities successfully", async () => {
it("should update entities successfully", async () => {
const updateData = { id: "1", name: "UpdatedItem" }
const entitiesToUpdate = [{ id: "1", name: "Item" }]
@@ -189,7 +189,7 @@ describe("Internal Module Service Factory", () => {
])
})
test("should delete entity successfully", async () => {
it("should delete entity successfully", async () => {
await instance.delete("1")
expect(containerMock[modelRepositoryName].delete).toHaveBeenCalledWith(
{
@@ -203,7 +203,7 @@ describe("Internal Module Service Factory", () => {
)
})
test("should delete entities successfully", async () => {
it("should delete entities successfully", async () => {
const entitiesToDelete = [{ id: "1", name: "Item" }]
containerMock[modelRepositoryName].find.mockResolvedValueOnce(
entitiesToDelete
@@ -222,14 +222,31 @@ describe("Internal Module Service Factory", () => {
)
})
test("should soft delete entity successfully", async () => {
it("should prevent from deleting all entities", async () => {
const entitiesToDelete = [{ id: "1", name: "Item" }]
containerMock[modelRepositoryName].find.mockResolvedValueOnce(
entitiesToDelete
)
await instance.delete([])
expect(containerMock[modelRepositoryName].delete).not.toHaveBeenCalled()
})
it("should soft delete entity successfully", async () => {
await instance.softDelete("1")
expect(
containerMock[modelRepositoryName].softDelete
).toHaveBeenCalledWith("1", defaultContext)
})
test("should restore entity successfully", async () => {
it("should prevent from soft deleting all data", async () => {
await instance.softDelete([])
expect(
containerMock[modelRepositoryName].softDelete
).not.toHaveBeenCalled()
})
it("should restore entity successfully", async () => {
await instance.restore("1")
expect(containerMock[modelRepositoryName].restore).toHaveBeenCalledWith(
"1",

View File

@@ -312,7 +312,7 @@ export function internalModuleServiceFactory<
): Promise<void> {
if (
!isDefined(idOrSelector) ||
(Array.isArray(idOrSelector) && idOrSelector.length === 0)
(Array.isArray(idOrSelector) && !idOrSelector.length)
) {
return
}
@@ -402,6 +402,10 @@ export function internalModuleServiceFactory<
idsOrFilter: string[] | InternalFilterQuery,
@MedusaContext() sharedContext: Context = {}
): Promise<[TEntity[], Record<string, unknown[]>]> {
if (Array.isArray(idsOrFilter) && !idsOrFilter.length) {
return [[], {}]
}
return await this[propertyRepositoryName].softDelete(
idsOrFilter,
sharedContext