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
This commit is contained in:
Philip Korsholm
2023-03-16 10:47:54 +01:00
committed by GitHub
parent 38c8d49f46
commit 02c77d7059
9 changed files with 392 additions and 102 deletions

View File

@@ -0,0 +1,5 @@
---
"@medusajs/medusa-js": patch
---
Fix(medusa): Adjust reservations correctly

View File

@@ -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)
})

View File

@@ -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()

View File

@@ -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<void> {
async deleteReservationItem(reservationItemId: string | string[]): Promise<void> {
return await this.reservationItemService_
.withTransaction(this.activeManager_)
.delete(reservationItemId)

View File

@@ -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<void> {
await this.atomicPhase_(async (manager) => {
async delete(reservationItemId: string | string[]): Promise<void> {
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)

View File

@@ -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)
}
}

View File

@@ -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"
)

View File

@@ -75,7 +75,7 @@ export interface IInventoryService {
deleteReservationItemsByLineItem(lineItemId: string): Promise<void>
deleteReservationItem(reservationItemId: string): Promise<void>
deleteReservationItem(reservationItemId: string | string[]): Promise<void>
deleteInventoryItem(inventoryItemId: string): Promise<void>

View File

@@ -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
)
})
))