From 02c77d7059b7e4cb9f6ebfa8cbec2b84e86118ec Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Thu, 16 Mar 2023 10:47:54 +0100 Subject: [PATCH] Fix/adjust reservations correctly (#3474) **What** - Adjust reservations correctly according to the following heuristic: adjustment by addition: (i.e. positive quantity adjustment passed to the adjustment method) - if a reservation for the line-item in the location exists add quantity to that - if not create a new reservation adjustment by subtraction: - if a reservation with the exact quantity exists, delete it and return - if a reservation with a greater quantity exists, subtract from it and return - otherwise delete from reservations until a reservation with greater quantity than the remaining is found and adjust that with the remaining quantity OR there are no more reservations Fixes CORE-1247 --- .changeset/ten-steaks-tan.md | 5 + .../inventory/inventory-items/index.js | 12 +- .../__tests__/inventory/order/order.js | 269 +++++++++++++++++- packages/inventory/src/services/inventory.ts | 2 +- .../src/services/reservation-item.ts | 28 +- .../transaction/create-product-variant.ts | 6 +- .../routes/admin/variants/get-inventory.ts | 4 +- .../src/interfaces/services/inventory.ts | 2 +- .../src/services/product-variant-inventory.ts | 166 ++++++----- 9 files changed, 392 insertions(+), 102 deletions(-) create mode 100644 .changeset/ten-steaks-tan.md diff --git a/.changeset/ten-steaks-tan.md b/.changeset/ten-steaks-tan.md new file mode 100644 index 0000000000..efb30675b1 --- /dev/null +++ b/.changeset/ten-steaks-tan.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa-js": patch +--- + +Fix(medusa): Adjust reservations correctly diff --git a/integration-tests/plugins/__tests__/inventory/inventory-items/index.js b/integration-tests/plugins/__tests__/inventory/inventory-items/index.js index e28721e31d..43a3a8ca7e 100644 --- a/integration-tests/plugins/__tests__/inventory/inventory-items/index.js +++ b/integration-tests/plugins/__tests__/inventory/inventory-items/index.js @@ -278,7 +278,7 @@ describe("Inventory Items endpoints", () => { }) }) - it.skip("Creates an inventory item using the api", async () => { + it("Creates an inventory item using the api", async () => { const product = await simpleProductFactory(dbConnection, {}) const api = useApi() @@ -316,20 +316,14 @@ describe("Inventory Items endpoints", () => { ) expect(variantInventoryRes.data).toEqual({ - variant: { + variant: expect.objectContaining({ id: variantId, inventory: [ expect.objectContaining({ ...inventoryItemCreateRes.data.inventory_item, }), ], - sales_channel_availability: [ - expect.objectContaining({ - available_quantity: 0, - channel_name: "Default Sales Channel", - }), - ], - }, + }), }) expect(variantInventoryRes.status).toEqual(200) }) diff --git a/integration-tests/plugins/__tests__/inventory/order/order.js b/integration-tests/plugins/__tests__/inventory/order/order.js index 07f081e61e..2f6da0ae67 100644 --- a/integration-tests/plugins/__tests__/inventory/order/order.js +++ b/integration-tests/plugins/__tests__/inventory/order/order.js @@ -295,7 +295,7 @@ describe("/store/carts", () => { ) }) - it("Adjusts reservations on successful fulfillment with reservation", async () => { + it("Adjusts reservation on successful fulfillment with reservation", async () => { const api = useApi() await prodVarInventoryService.reserveQuantity(variantId, 1, { @@ -350,6 +350,273 @@ describe("/store/carts", () => { ) }) + it("Deletes multiple reservations on successful fulfillment with reservation", async () => { + const api = useApi() + + const a = await inventoryService.updateInventoryLevel( + invItemId, + locationId, + { stocked_quantity: 2 } + ) + + await prodVarInventoryService.reserveQuantity(variantId, 1, { + locationId: locationId, + lineItemId: order.items[0].id, + }) + + await prodVarInventoryService.reserveQuantity(variantId, 1, { + locationId: locationId, + lineItemId: order.items[0].id, + }) + + let inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + expect(inventoryItem.data.inventory_item.location_levels[0]).toEqual( + expect.objectContaining({ + stocked_quantity: 2, + reserved_quantity: 2, + available_quantity: 0, + }) + ) + + const fulfillmentRes = await api.post( + `/admin/orders/${order.id}/fulfillment`, + { + items: [{ item_id: lineItemId, quantity: 2 }], + location_id: locationId, + }, + adminHeaders + ) + + expect(fulfillmentRes.status).toBe(200) + expect(fulfillmentRes.data.order.fulfillment_status).toBe("fulfilled") + + inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + const reservations = await api.get( + `/admin/reservations?inventory_item_id[]=${invItemId}`, + adminHeaders + ) + + expect(reservations.data.reservations.length).toBe(0) + expect(inventoryItem.data.inventory_item.location_levels[0]).toEqual( + expect.objectContaining({ + stocked_quantity: 0, + reserved_quantity: 0, + available_quantity: 0, + }) + ) + }) + + it("Deletes single reservation on successful fulfillment with partial reservation", async () => { + const api = useApi() + + await inventoryService.updateInventoryLevel(invItemId, locationId, { + stocked_quantity: 2, + }) + + await prodVarInventoryService.reserveQuantity(variantId, 1, { + locationId: locationId, + lineItemId: order.items[0].id, + }) + + let inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + expect(inventoryItem.data.inventory_item.location_levels[0]).toEqual( + expect.objectContaining({ + stocked_quantity: 2, + reserved_quantity: 1, + available_quantity: 1, + }) + ) + + const fulfillmentRes = await api.post( + `/admin/orders/${order.id}/fulfillment`, + { + items: [{ item_id: lineItemId, quantity: 2 }], + location_id: locationId, + }, + adminHeaders + ) + + expect(fulfillmentRes.status).toBe(200) + expect(fulfillmentRes.data.order.fulfillment_status).toBe("fulfilled") + + inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + const reservations = await api.get( + `/admin/reservations?inventory_item_id[]=${invItemId}`, + adminHeaders + ) + + expect(reservations.data.reservations.length).toBe(0) + expect(inventoryItem.data.inventory_item.location_levels[0]).toEqual( + expect.objectContaining({ + stocked_quantity: 0, + reserved_quantity: 0, + available_quantity: 0, + }) + ) + }) + + it("Adjusts single reservation on successful fulfillment with over-reserved line item", async () => { + const api = useApi() + + const a = await inventoryService.updateInventoryLevel( + invItemId, + locationId, + { stocked_quantity: 3 } + ) + + await prodVarInventoryService.reserveQuantity(variantId, 3, { + locationId: locationId, + lineItemId: order.items[0].id, + }) + + let inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + expect(inventoryItem.data.inventory_item.location_levels[0]).toEqual( + expect.objectContaining({ + stocked_quantity: 3, + reserved_quantity: 3, + available_quantity: 0, + }) + ) + + const fulfillmentRes = await api.post( + `/admin/orders/${order.id}/fulfillment`, + { + items: [{ item_id: lineItemId, quantity: 2 }], + location_id: locationId, + }, + adminHeaders + ) + + expect(fulfillmentRes.status).toBe(200) + expect(fulfillmentRes.data.order.fulfillment_status).toBe("fulfilled") + + inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + const reservations = await api.get( + `/admin/reservations?inventory_item_id[]=${invItemId}`, + adminHeaders + ) + + expect(reservations.data.reservations.length).toBe(1) + expect(reservations.data.reservations).toEqual([ + expect.objectContaining({ + quantity: 1, + }), + ]) + expect(inventoryItem.data.inventory_item.location_levels[0]).toEqual( + expect.objectContaining({ + stocked_quantity: 1, + reserved_quantity: 1, + available_quantity: 0, + }) + ) + }) + + it("Prioritizes adjusting reservations at the chosen location", async () => { + const api = useApi() + + const sl = await stockLocationService.create({ + name: "test-location 1", + }) + + await inventoryService.createInventoryLevel({ + inventory_item_id: invItemId, + location_id: sl.id, + stocked_quantity: 3, + }) + + const a = await inventoryService.updateInventoryLevel( + invItemId, + locationId, + { stocked_quantity: 3 } + ) + + await prodVarInventoryService.reserveQuantity(variantId, 1, { + locationId: locationId, + lineItemId: order.items[0].id, + }) + + await prodVarInventoryService.reserveQuantity(variantId, 2, { + locationId: sl.id, + lineItemId: order.items[0].id, + }) + + let inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + expect(inventoryItem.data.inventory_item.location_levels).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + location_id: locationId, + stocked_quantity: 3, + reserved_quantity: 1, + available_quantity: 2, + }), + expect.objectContaining({ + location_id: sl.id, + stocked_quantity: 3, + reserved_quantity: 2, + available_quantity: 1, + }), + ]) + ) + + const fulfillmentRes = await api.post( + `/admin/orders/${order.id}/fulfillment`, + { + items: [{ item_id: lineItemId, quantity: 2 }], + location_id: locationId, + }, + adminHeaders + ) + + expect(fulfillmentRes.status).toBe(200) + expect(fulfillmentRes.data.order.fulfillment_status).toBe("fulfilled") + + inventoryItem = await api.get( + `/admin/inventory-items/${invItemId}`, + adminHeaders + ) + + const reservations = await api.get( + `/admin/reservations?inventory_item_id[]=${invItemId}`, + adminHeaders + ) + + expect(reservations.data.reservations.length).toBe(1) + expect(reservations.data.reservations).toEqual([ + expect.objectContaining({ + quantity: 1, + location_id: sl.id, + }), + ]) + }) + it("increases stocked quantity when return is received at location", async () => { const api = useApi() diff --git a/packages/inventory/src/services/inventory.ts b/packages/inventory/src/services/inventory.ts index b05454d54c..22fb3cd14f 100644 --- a/packages/inventory/src/services/inventory.ts +++ b/packages/inventory/src/services/inventory.ts @@ -350,7 +350,7 @@ export default class InventoryService * Deletes a reservation item * @param reservationItemId - the id of the reservation item to delete */ - async deleteReservationItem(reservationItemId: string): Promise { + async deleteReservationItem(reservationItemId: string | string[]): Promise { return await this.reservationItemService_ .withTransaction(this.activeManager_) .delete(reservationItemId) diff --git a/packages/inventory/src/services/reservation-item.ts b/packages/inventory/src/services/reservation-item.ts index f6c4da81a6..fd6703d57e 100644 --- a/packages/inventory/src/services/reservation-item.ts +++ b/packages/inventory/src/services/reservation-item.ts @@ -277,21 +277,29 @@ export default class ReservationItemService extends TransactionBaseService { * Deletes a reservation item by id. * @param reservationItemId - the id of the reservation item to delete. */ - async delete(reservationItemId: string): Promise { - await this.atomicPhase_(async (manager) => { + async delete(reservationItemId: string | string[]): Promise { + const ids = Array.isArray(reservationItemId) + ? reservationItemId + : [reservationItemId] + return await this.atomicPhase_(async (manager) => { const itemRepository = manager.getRepository(ReservationItem) - const item = await this.retrieve(reservationItemId) - await Promise.all([ - itemRepository.softRemove({ id: reservationItemId }), - this.inventoryLevelService_ - .withTransaction(manager) - .adjustReservedQuantity( + const items = await this.list({ id: ids }) + + await itemRepository.softRemove(items) + + const inventoryServiceTx = + this.inventoryLevelService_.withTransaction(manager) + + await Promise.all( + items.map(async (item) => { + return inventoryServiceTx.adjustReservedQuantity( item.inventory_item_id, item.location_id, item.quantity * -1 - ), - ]) + ) + }) + ) await this.eventBusService_ .withTransaction(manager) diff --git a/packages/medusa/src/api/routes/admin/products/transaction/create-product-variant.ts b/packages/medusa/src/api/routes/admin/products/transaction/create-product-variant.ts index e0f1e594b1..d3710c54b8 100644 --- a/packages/medusa/src/api/routes/admin/products/transaction/create-product-variant.ts +++ b/packages/medusa/src/api/routes/admin/products/transaction/create-product-variant.ts @@ -74,8 +74,6 @@ export const createVariantTransaction = async ( productVariantInventoryService, } = dependencies - const inventoryServiceTx = inventoryService?.withTransaction(manager) - const productVariantInventoryServiceTx = productVariantInventoryService.withTransaction(manager) @@ -96,7 +94,7 @@ export const createVariantTransaction = async ( return } - return await inventoryServiceTx!.createInventoryItem({ + return await inventoryService!.createInventoryItem({ sku: variant.sku, origin_country: variant.origin_country, hs_code: variant.hs_code, @@ -111,7 +109,7 @@ export const createVariantTransaction = async ( async function removeInventoryItem(inventoryItem: InventoryItemDTO) { if (inventoryItem) { - await inventoryServiceTx!.deleteInventoryItem(inventoryItem.id) + await inventoryService!.deleteInventoryItem(inventoryItem.id) } } diff --git a/packages/medusa/src/api/routes/admin/variants/get-inventory.ts b/packages/medusa/src/api/routes/admin/variants/get-inventory.ts index c96d045486..8a3aa580a3 100644 --- a/packages/medusa/src/api/routes/admin/variants/get-inventory.ts +++ b/packages/medusa/src/api/routes/admin/variants/get-inventory.ts @@ -4,7 +4,6 @@ import { } from "../../../../types/inventory" import ProductVariantInventoryService from "../../../../services/product-variant-inventory" import { - SalesChannelInventoryService, SalesChannelLocationService, SalesChannelService, } from "../../../../services" @@ -76,8 +75,7 @@ export default async (req, res) => { const channelLocationService: SalesChannelLocationService = req.scope.resolve( "salesChannelLocationService" ) - const salesChannelInventoryService: SalesChannelInventoryService = - req.scope.resolve("salesChannelInventoryService") + const channelService: SalesChannelService = req.scope.resolve( "salesChannelService" ) diff --git a/packages/medusa/src/interfaces/services/inventory.ts b/packages/medusa/src/interfaces/services/inventory.ts index 58f97fbe70..b6a85e7cec 100644 --- a/packages/medusa/src/interfaces/services/inventory.ts +++ b/packages/medusa/src/interfaces/services/inventory.ts @@ -75,7 +75,7 @@ export interface IInventoryService { deleteReservationItemsByLineItem(lineItemId: string): Promise - deleteReservationItem(reservationItemId: string): Promise + deleteReservationItem(reservationItemId: string | string[]): Promise deleteInventoryItem(inventoryItemId: string): Promise diff --git a/packages/medusa/src/services/product-variant-inventory.ts b/packages/medusa/src/services/product-variant-inventory.ts index 4d3f997dde..28aa2d31da 100644 --- a/packages/medusa/src/services/product-variant-inventory.ts +++ b/packages/medusa/src/services/product-variant-inventory.ts @@ -115,13 +115,11 @@ class ProductVariantInventoryService extends TransactionBaseService { const hasInventory = await Promise.all( variantInventory.map(async (inventoryPart) => { const itemQuantity = inventoryPart.required_quantity * quantity - return await this.inventoryService_ - .withTransaction(this.activeManager_) - .confirmInventory( - inventoryPart.inventory_item_id, - locationIds, - itemQuantity - ) + return await this.inventoryService_.confirmInventory( + inventoryPart.inventory_item_id, + locationIds, + itemQuantity + ) }) ) @@ -253,11 +251,9 @@ class ProductVariantInventoryService extends TransactionBaseService { }) // Verify that item exists - await this.inventoryService_ - .withTransaction(this.activeManager_) - .retrieveInventoryItem(inventoryItemId, { - select: ["id"], - }) + await this.inventoryService_.retrieveInventoryItem(inventoryItemId, { + select: ["id"], + }) const variantInventoryRepo = this.activeManager_.getRepository( ProductVariantInventoryItem @@ -392,14 +388,12 @@ class ProductVariantInventoryService extends TransactionBaseService { return await Promise.all( variantInventory.map(async (inventoryPart) => { const itemQuantity = inventoryPart.required_quantity * quantity - return await this.inventoryService_ - .withTransaction(this.activeManager_) - .createReservationItem({ - ...toReserve, - location_id: locationId as string, - inventory_item_id: inventoryPart.inventory_item_id, - quantity: itemQuantity, - }) + return await this.inventoryService_.createReservationItem({ + ...toReserve, + location_id: locationId as string, + inventory_item_id: inventoryPart.inventory_item_id, + quantity: itemQuantity, + }) }) ) } @@ -435,9 +429,15 @@ class ProductVariantInventoryService extends TransactionBaseService { }) } - const [reservations, reservationCount] = await this.inventoryService_ - .withTransaction(this.activeManager_) - .listReservationItems( + if (quantity > 0) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + "You can only reduce reservation quantities using adjustReservationsQuantityByLineItem. If you wish to reserve more use update or create." + ) + } + + const [reservations, reservationCount] = + await this.inventoryService_.listReservationItems( { line_item_id: lineItemId, }, @@ -446,33 +446,56 @@ class ProductVariantInventoryService extends TransactionBaseService { } ) + reservations.sort((a, _) => { + if (a.location_id === locationId) { + return -1 + } + return 0 + }) + if (reservationCount) { - let reservation = reservations[0] + const inventoryItems = await this.listByVariant(variantId) + const productVariantInventory = inventoryItems[0] - reservation = - reservations.find( - (r) => r.location_id === locationId && r.quantity >= quantity - ) ?? reservation - - const productVariantInventory = await this.retrieve( - reservation.inventory_item_id, - variantId + const deltaUpdate = Math.abs( + quantity * productVariantInventory.required_quantity ) - const reservationQtyUpdate = - reservation.quantity + - quantity * productVariantInventory.required_quantity + const exactReservation = reservations.find( + (r) => r.quantity === deltaUpdate && r.location_id === locationId + ) + if (exactReservation) { + await this.inventoryService_.deleteReservationItem(exactReservation.id) + return + } - if (reservationQtyUpdate === 0) { - await this.inventoryService_ - .withTransaction(this.activeManager_) - .deleteReservationItem(reservation.id) - } else { - await this.inventoryService_ - .withTransaction(this.activeManager_) - .updateReservationItem(reservation.id, { - quantity: reservationQtyUpdate, - }) + let remainingQuantity = deltaUpdate + + const reservationsToDelete: ReservationItemDTO[] = [] + let reservationToUpdate: ReservationItemDTO | null = null + for (const reservation of reservations) { + if (reservation.quantity <= remainingQuantity) { + remainingQuantity -= reservation.quantity + reservationsToDelete.push(reservation) + } else { + reservationToUpdate = reservation + break + } + } + + if (reservationsToDelete.length) { + await this.inventoryService_.deleteReservationItem( + reservationsToDelete.map((r) => r.id) + ) + } + + if (reservationToUpdate) { + await this.inventoryService_.updateReservationItem( + reservationToUpdate.id, + { + quantity: reservationToUpdate.quantity - remainingQuantity, + } + ) } } } @@ -552,9 +575,7 @@ class ProductVariantInventoryService extends TransactionBaseService { }) } - await this.inventoryService_ - .withTransaction(this.activeManager_) - .deleteReservationItemsByLineItem(lineItemId) + await this.inventoryService_.deleteReservationItemsByLineItem(lineItemId) } /** @@ -584,26 +605,24 @@ class ProductVariantInventoryService extends TransactionBaseService { inventory_quantity: variant.inventory_quantity + quantity, }) }) - } else { - const variantInventory = await this.listByVariant(variantId) - - if (variantInventory.length === 0) { - return - } - - await Promise.all( - variantInventory.map(async (inventoryPart) => { - const itemQuantity = inventoryPart.required_quantity * quantity - return await this.inventoryService_ - .withTransaction(this.activeManager_) - .adjustInventory( - inventoryPart.inventory_item_id, - locationId, - itemQuantity - ) - }) - ) } + + const variantInventory = await this.listByVariant(variantId) + + if (variantInventory.length === 0) { + return + } + + await Promise.all( + variantInventory.map(async (inventoryPart) => { + const itemQuantity = inventoryPart.required_quantity * quantity + return await this.inventoryService_.adjustInventory( + inventoryPart.inventory_item_id, + locationId, + itemQuantity + ) + }) + ) } async setVariantAvailability( @@ -685,6 +704,9 @@ class ProductVariantInventoryService extends TransactionBaseService { ) } + const salesChannelInventoryServiceTx = + this.salesChannelInventoryService_.withTransaction(this.activeManager_) + return Math.min( ...(await Promise.all( variantInventoryItems.map(async (variantInventory) => { @@ -694,12 +716,10 @@ class ProductVariantInventoryService extends TransactionBaseService { // can fulfill and set that as quantity return ( // eslint-disable-next-line max-len - (await this.salesChannelInventoryService_ - .withTransaction(this.activeManager_) - .retrieveAvailableItemQuantity( - channelId, - variantInventory.inventory_item_id - )) / variantInventory.required_quantity + (await salesChannelInventoryServiceTx.retrieveAvailableItemQuantity( + channelId, + variantInventory.inventory_item_id + )) / variantInventory.required_quantity ) }) ))