From 10bf05c147cb65a263465129790edd44a6d8948b Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Wed, 15 Mar 2023 10:12:46 +0100 Subject: [PATCH] Fix(inventory, stock-location): Remove orphaned location levels and reservations (#3460) **What** - Remove related inventory levels and reservation items when a stock location is removed **How** - Add bulk deletion methods for both inventory levels and reservation items to the inventory service api - invoke both on location removal Fixes CORE-1232 --- .changeset/many-papayas-judge.md | 6 ++ .../inventory/inventory-items/index.js | 65 +++++++++++++++++++ .../components/location-card/index.tsx | 2 +- .../inventory/src/services/inventory-level.ts | 18 +++++ packages/inventory/src/services/inventory.ts | 12 ++++ .../src/services/reservation-item.ts | 38 +++++++++-- .../stock-locations/delete-stock-location.ts | 19 +++++- .../src/interfaces/services/inventory.ts | 4 ++ 8 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 .changeset/many-papayas-judge.md diff --git a/.changeset/many-papayas-judge.md b/.changeset/many-papayas-judge.md new file mode 100644 index 0000000000..56de28b8cf --- /dev/null +++ b/.changeset/many-papayas-judge.md @@ -0,0 +1,6 @@ +--- +"@medusajs/inventory": patch +"@medusajs/medusa": patch +--- + +Fix(inventory, medusa): ensure no orphaned reservations and invenotry levels on location removal diff --git a/integration-tests/plugins/__tests__/inventory/inventory-items/index.js b/integration-tests/plugins/__tests__/inventory/inventory-items/index.js index 1eae8976b0..e0baad38f1 100644 --- a/integration-tests/plugins/__tests__/inventory/inventory-items/index.js +++ b/integration-tests/plugins/__tests__/inventory/inventory-items/index.js @@ -475,6 +475,71 @@ describe("Inventory Items endpoints", () => { }) }) + it("When deleting an inventory item it removes associated levels and reservations", async () => { + const api = useApi() + const inventoryService = appContainer.resolve("inventoryService") + + const invItem2 = await inventoryService.createInventoryItem({ + sku: "1234567", + }) + + const stockRes = await api.post( + `/admin/stock-locations`, + { + name: "Fake Warehouse 1", + }, + adminHeaders + ) + + locationId = stockRes.data.stock_location.id + + const level = await inventoryService.createInventoryLevel({ + inventory_item_id: invItem2.id, + location_id: locationId, + stocked_quantity: 10, + }) + + const reservation = await inventoryService.createReservationItem({ + inventory_item_id: invItem2.id, + location_id: locationId, + quantity: 5, + }) + + const [, reservationCount] = await inventoryService.listReservationItems({ + location_id: locationId, + }) + + expect(reservationCount).toEqual(1) + + const [, inventoryLevelCount] = + await inventoryService.listInventoryLevels({ + location_id: locationId, + }) + + expect(inventoryLevelCount).toEqual(1) + + const res = await api.delete( + `/admin/stock-locations/${locationId}`, + adminHeaders + ) + + expect(res.status).toEqual(200) + + const [, reservationCountPostDelete] = + await inventoryService.listReservationItems({ + location_id: locationId, + }) + + expect(reservationCountPostDelete).toEqual(0) + + const [, inventoryLevelCountPostDelete] = + await inventoryService.listInventoryLevels({ + location_id: locationId, + }) + + expect(inventoryLevelCountPostDelete).toEqual(0) + }) + it("When deleting an inventory item it removes the product variants associated to it", async () => { const api = useApi() diff --git a/packages/admin-ui/ui/src/domain/inventory/locations/components/location-card/index.tsx b/packages/admin-ui/ui/src/domain/inventory/locations/components/location-card/index.tsx index 6c0c846332..609d3680e1 100644 --- a/packages/admin-ui/ui/src/domain/inventory/locations/components/location-card/index.tsx +++ b/packages/admin-ui/ui/src/domain/inventory/locations/components/location-card/index.tsx @@ -37,7 +37,7 @@ const LocationCard: React.FC = ({ location }) => { const onDelete = async () => { const shouldDelete = await dialog({ heading: "Delete Location", - text: "Are you sure you want to delete this location", + text: "Are you sure you want to delete this location. This will also delete all inventory levels and reservations associated with this location.", extraConfirmation: true, entityName: location.name, }) diff --git a/packages/inventory/src/services/inventory-level.ts b/packages/inventory/src/services/inventory-level.ts index 5b691e7129..eee5337320 100644 --- a/packages/inventory/src/services/inventory-level.ts +++ b/packages/inventory/src/services/inventory-level.ts @@ -227,6 +227,24 @@ export default class InventoryLevelService extends TransactionBaseService { }) } + /** + * Deletes inventory levels by location ID. + * @param locationId - The ID of the location to delete inventory levels for. + */ + async deleteByLocationId(locationId: string): Promise { + return await this.atomicPhase_(async (manager) => { + const levelRepository = manager.getRepository(InventoryLevel) + + await levelRepository.delete({ location_id: locationId }) + + await this.eventBusService_ + .withTransaction(manager) + .emit(InventoryLevelService.Events.DELETED, { + location_id: locationId, + }) + }) + } + /** * Gets the total stocked quantity for a specific inventory item at multiple locations. * @param inventoryItemId - The ID of the inventory item. diff --git a/packages/inventory/src/services/inventory.ts b/packages/inventory/src/services/inventory.ts index 86f9277c2d..1b865b7bfe 100644 --- a/packages/inventory/src/services/inventory.ts +++ b/packages/inventory/src/services/inventory.ts @@ -254,6 +254,18 @@ export default class InventoryService .delete(inventoryItemId) } + async deleteInventoryItemLevelByLocationId(locationId: string): Promise { + return await this.inventoryLevelService_ + .withTransaction(this.activeManager_) + .deleteByLocationId(locationId) + } + + async deleteReservationItemByLocationId(locationId: string): Promise { + return await this.reservationItemService_ + .withTransaction(this.activeManager_) + .deleteByLocationId(locationId) + } + /** * Deletes an inventory level * @param inventoryItemId - the id of the inventory item associated with the level diff --git a/packages/inventory/src/services/reservation-item.ts b/packages/inventory/src/services/reservation-item.ts index 1796f2d243..f6c4da81a6 100644 --- a/packages/inventory/src/services/reservation-item.ts +++ b/packages/inventory/src/services/reservation-item.ts @@ -24,7 +24,6 @@ export default class ReservationItemService extends TransactionBaseService { CREATED: "reservation-item.created", UPDATED: "reservation-item.updated", DELETED: "reservation-item.deleted", - DELETED_BY_LINE_ITEM: "reservation-item.deleted-by-line-item", } protected readonly eventBusService_: IEventBusService @@ -95,7 +94,10 @@ export default class ReservationItemService extends TransactionBaseService { const manager = this.activeManager_ const reservationItemRepository = manager.getRepository(ReservationItem) - const query = buildQuery({ id: reservationItemId }, config) as FindManyOptions + const query = buildQuery( + { id: reservationItemId }, + config + ) as FindManyOptions const [reservationItem] = await reservationItemRepository.find(query) if (!reservationItem) { @@ -165,8 +167,7 @@ export default class ReservationItemService extends TransactionBaseService { isDefined(data.quantity) && data.quantity !== item.quantity const shouldUpdateLocation = - isDefined(data.location_id) && - data.location_id !== item.location_id + isDefined(data.location_id) && data.location_id !== item.location_id const ops: Promise[] = [] @@ -243,12 +244,35 @@ export default class ReservationItemService extends TransactionBaseService { await this.eventBusService_ .withTransaction(manager) - .emit(ReservationItemService.Events.DELETED_BY_LINE_ITEM, { + .emit(ReservationItemService.Events.DELETED, { line_item_id: lineItemId, }) }) } + /** + * Deletes reservation items by location ID. + * @param locationId - The ID of the location to delete reservations for. + */ + async deleteByLocationId(locationId: string): Promise { + return await this.atomicPhase_(async (manager) => { + const itemRepository = manager.getRepository(ReservationItem) + + await itemRepository + .createQueryBuilder("reservation_item") + .softDelete() + .where("location_id = :locationId", { locationId }) + .andWhere("deleted_at IS NULL") + .execute() + + await this.eventBusService_ + .withTransaction(manager) + .emit(ReservationItemService.Events.DELETED, { + location_id: locationId, + }) + }) + } + /** * Deletes a reservation item by id. * @param reservationItemId - the id of the reservation item to delete. @@ -272,8 +296,8 @@ export default class ReservationItemService extends TransactionBaseService { await this.eventBusService_ .withTransaction(manager) .emit(ReservationItemService.Events.DELETED, { - id: reservationItemId, - }) + id: reservationItemId, + }) }) } } diff --git a/packages/medusa/src/api/routes/admin/stock-locations/delete-stock-location.ts b/packages/medusa/src/api/routes/admin/stock-locations/delete-stock-location.ts index 88c4207875..b523066089 100644 --- a/packages/medusa/src/api/routes/admin/stock-locations/delete-stock-location.ts +++ b/packages/medusa/src/api/routes/admin/stock-locations/delete-stock-location.ts @@ -1,5 +1,8 @@ import { EntityManager } from "typeorm" -import { IStockLocationService } from "../../../../interfaces" +import { + IInventoryService, + IStockLocationService, +} from "../../../../interfaces" import { SalesChannelLocationService } from "../../../../services" /** @@ -60,6 +63,9 @@ export default async (req, res) => { "stockLocationService" ) + const inventoryService: IInventoryService = + req.scope.resolve("inventoryService") + const salesChannelLocationService: SalesChannelLocationService = req.scope.resolve("salesChannelLocationService") @@ -70,6 +76,17 @@ export default async (req, res) => { .removeLocation(id) await stockLocationService.withTransaction(transactionManager).delete(id) + + if (inventoryService) { + await Promise.all([ + inventoryService + .withTransaction(transactionManager) + .deleteInventoryItemLevelByLocationId(id), + inventoryService + .withTransaction(transactionManager) + .deleteReservationItemByLocationId(id), + ]) + } }) res.status(200).send({ diff --git a/packages/medusa/src/interfaces/services/inventory.ts b/packages/medusa/src/interfaces/services/inventory.ts index 5e1aa6571c..b2452dc729 100644 --- a/packages/medusa/src/interfaces/services/inventory.ts +++ b/packages/medusa/src/interfaces/services/inventory.ts @@ -77,6 +77,10 @@ export interface IInventoryService { deleteInventoryItem(inventoryItemId: string): Promise + deleteInventoryItemLevelByLocationId(locationId: string): Promise + + deleteReservationItemByLocationId(locationId: string): Promise + deleteInventoryLevel( inventoryLevelId: string, locationId: string