diff --git a/.changeset/curly-mayflies-sell.md b/.changeset/curly-mayflies-sell.md new file mode 100644 index 0000000000000..479b7f7a5132b --- /dev/null +++ b/.changeset/curly-mayflies-sell.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": minor +--- + +feat(medusa): remove reservations if cart completion fails after reservation creation diff --git a/integration-tests/api/__tests__/store/cart/__snapshots__/cart.js.snap b/integration-tests/api/__tests__/store/cart/__snapshots__/cart.js.snap index dbf6afc647e2b..6df7864b25873 100644 --- a/integration-tests/api/__tests__/store/cart/__snapshots__/cart.js.snap +++ b/integration-tests/api/__tests__/store/cart/__snapshots__/cart.js.snap @@ -2,17 +2,25 @@ exports[`/store/carts POST /store/carts/:id fails to complete cart with items inventory not/partially covered 1`] = ` Object { - "code": "insufficient_inventory", - "message": "Variant with id: test-variant-2 does not have the required inventory", - "type": "not_allowed", + "errors": Array [ + Object { + "code": "insufficient_inventory", + "message": "Variant with id: test-variant-2 does not have the required inventory", + "type": "not_allowed", + }, + ], } `; exports[`/store/carts POST /store/carts/:id fails to complete swap cart with items inventory not/partially covered 1`] = ` Object { - "code": "insufficient_inventory", - "message": "Variant with id: test-variant-2 does not have the required inventory", - "type": "not_allowed", + "errors": Array [ + Object { + "code": "insufficient_inventory", + "message": "Variant with id: test-variant-2 does not have the required inventory", + "type": "not_allowed", + }, + ], } `; diff --git a/integration-tests/api/__tests__/store/cart/cart.js b/integration-tests/api/__tests__/store/cart/cart.js index 233abf535b9e0..2c4bdc8fb8308 100644 --- a/integration-tests/api/__tests__/store/cart/cart.js +++ b/integration-tests/api/__tests__/store/cart/cart.js @@ -1948,7 +1948,7 @@ describe("/store/carts", () => { await api.post(`/store/carts/test-cart-2/complete-cart`) } catch (e) { expect(e.response.data).toMatchSnapshot({ - code: "insufficient_inventory", + errors: [{ code: "insufficient_inventory" }], }) expect(e.response.status).toBe(409) } @@ -1984,7 +1984,7 @@ describe("/store/carts", () => { await api.post(`/store/carts/swap-cart/complete-cart`) } catch (e) { expect(e.response.data).toMatchSnapshot({ - code: "insufficient_inventory", + errors: [{ code: "insufficient_inventory" }], }) expect(e.response.status).toBe(409) } diff --git a/integration-tests/api/__tests__/store/orders.js b/integration-tests/api/__tests__/store/orders.js index 0238ad0c5f76d..5a7127534e56c 100644 --- a/integration-tests/api/__tests__/store/orders.js +++ b/integration-tests/api/__tests__/store/orders.js @@ -343,8 +343,8 @@ describe("/store/carts", () => { }) expect(responseFail.status).toEqual(409) - expect(responseFail.data.type).toEqual("not_allowed") - expect(responseFail.data.code).toEqual( + expect(responseFail.data.errors[0].type).toEqual("not_allowed") + expect(responseFail.data.errors[0].code).toEqual( MedusaError.Codes.INSUFFICIENT_INVENTORY ) diff --git a/integration-tests/plugins/__tests__/inventory/cart/cart.js b/integration-tests/plugins/__tests__/inventory/cart/cart.js index 6898bad7b1c11..a4a2675dd6144 100644 --- a/integration-tests/plugins/__tests__/inventory/cart/cart.js +++ b/integration-tests/plugins/__tests__/inventory/cart/cart.js @@ -191,6 +191,47 @@ describe("/store/carts", () => { expect(stockLevel.stocked_quantity).toEqual(5) }) + it("removes reserved quantity when failing to complete the cart", async () => { + const api = useApi() + + const cartRes = await api.post( + `/store/carts`, + { + region_id: "test-region", + items: [ + { + variant_id: variantId, + quantity: 3, + }, + ], + }, + { withCredentials: true } + ) + + const cartId = cartRes.data.cart.id + + await api.post(`/store/carts/${cartId}/payment-sessions`) + await api.post(`/store/carts/${cartId}/payment-session`, { + provider_id: "test-pay", + }) + + const getRes = await api + .post(`/store/carts/${cartId}/complete`) + .catch((err) => err) + + expect(getRes.response.status).toEqual(400) + expect(getRes.response.data).toEqual({ + type: "invalid_data", + message: + "Can't insert null value in field customer_id on insert in table order", + }) + + const inventoryService = appContainer.resolve("inventoryService") + const [, count] = await inventoryService.listReservationItems({ + line_item_id: cartRes.data.cart.items.map((i) => i.id), + }) + expect(count).toEqual(0) + }) it("fails to add a item on the cart if the inventory isn't enough", async () => { const api = useApi() @@ -249,10 +290,10 @@ describe("/store/carts", () => { .catch((e) => e) expect(completeCartRes.response.status).toEqual(409) - expect(completeCartRes.response.data.code).toEqual( + expect(completeCartRes.response.data.errors[0].code).toEqual( "insufficient_inventory" ) - expect(completeCartRes.response.data.message).toEqual( + expect(completeCartRes.response.data.errors[0].message).toEqual( `Variant with id: ${variantId} does not have the required inventory` ) diff --git a/packages/medusa/src/strategies/cart-completion.ts b/packages/medusa/src/strategies/cart-completion.ts index 634d08674db0d..0c09627a8dedd 100644 --- a/packages/medusa/src/strategies/cart-completion.ts +++ b/packages/medusa/src/strategies/cart-completion.ts @@ -1,24 +1,24 @@ -import { MedusaError } from "medusa-core-utils" -import { EntityManager } from "typeorm" - -import { IdempotencyKey, Order } from "../models" -import CartService from "../services/cart" -import IdempotencyKeyService from "../services/idempotency-key" -import OrderService, { - ORDER_CART_ALREADY_EXISTS_ERROR, -} from "../services/order" -import SwapService from "../services/swap" -import { RequestContext } from "../types/request" - import { AbstractCartCompletionStrategy, CartCompletionResponse, } from "../interfaces" +import { IInventoryService, ReservationItemDTO } from "@medusajs/types" +import { IdempotencyKey, Order } from "../models" +import OrderService, { + ORDER_CART_ALREADY_EXISTS_ERROR, +} from "../services/order" import { PaymentProviderService, ProductVariantInventoryService, } from "../services" +import CartService from "../services/cart" +import { EntityManager } from "typeorm" +import IdempotencyKeyService from "../services/idempotency-key" +import { MedusaError } from "medusa-core-utils" +import { RequestContext } from "../types/request" +import SwapService from "../services/swap" + type InjectedDependencies = { productVariantInventoryService: ProductVariantInventoryService paymentProviderService: PaymentProviderService @@ -27,6 +27,7 @@ type InjectedDependencies = { orderService: OrderService swapService: SwapService manager: EntityManager + inventoryService: IInventoryService } class CartCompletionStrategy extends AbstractCartCompletionStrategy { @@ -37,6 +38,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { protected readonly cartService_: CartService protected readonly orderService_: OrderService protected readonly swapService_: SwapService + protected readonly inventoryService_: IInventoryService constructor({ productVariantInventoryService, @@ -45,6 +47,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { cartService, orderService, swapService, + inventoryService, }: InjectedDependencies) { // eslint-disable-next-line prefer-rest-params super(arguments[0]) @@ -55,6 +58,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { this.cartService_ = cartService this.orderService_ = orderService this.swapService_ = swapService + this.inventoryService_ = inventoryService } async complete( @@ -247,6 +251,23 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { } } + protected async removeReservations(reservations) { + if (this.inventoryService_) { + await Promise.all( + reservations.map(async ([reservations]) => { + if (reservations) { + return reservations.map(async (reservation) => { + return await this.inventoryService_.deleteReservationItem( + reservation.id + ) + }) + } + return Promise.resolve() + }) + ) + } + } + protected async handlePaymentAuthorized( id: string, { manager }: { manager: EntityManager } @@ -276,14 +297,18 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { allowBackorder = swap.allow_backorder } + let reservations: [ + ReservationItemDTO[] | void | undefined, + MedusaError | undefined + ][] = [] if (!allowBackorder) { const productVariantInventoryServiceTx = this.productVariantInventoryService_.withTransaction(manager) - try { - await Promise.all( - cart.items.map(async (item) => { - if (item.variant_id) { + reservations = await Promise.all( + cart.items.map(async (item) => { + if (item.variant_id) { + try { const inventoryConfirmed = await productVariantInventoryServiceTx.confirmInventory( item.variant_id, @@ -299,19 +324,42 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { ) } - await productVariantInventoryServiceTx.reserveQuantity( - item.variant_id, - item.quantity, - { - lineItemId: item.id, - salesChannelId: cart.sales_channel_id, - } - ) + return [ + await productVariantInventoryServiceTx.reserveQuantity( + item.variant_id, + item.quantity, + { + lineItemId: item.id, + salesChannelId: cart.sales_channel_id, + } + ), + undefined, + ] + } catch (error) { + return [undefined, error] } - }) - ) - } catch (error) { - if (error && error.code === MedusaError.Codes.INSUFFICIENT_INVENTORY) { + } + return [undefined, undefined] + }) + ) + + if (reservations.some(([_, error]) => error)) { + await this.removeReservations(reservations) + + const errors = reservations.reduce((acc, [_, error]) => { + if (error) { + acc.push(error) + } + return acc + }, [] as MedusaError[]) + + const error = errors[0] + + if ( + errors.some( + (error) => error.code === MedusaError.Codes.INSUFFICIENT_INVENTORY + ) + ) { if (cart.payment) { await this.paymentProviderService_ .withTransaction(manager) @@ -324,9 +372,13 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { return { response_code: 409, response_body: { - message: error.message, - type: error.type, - code: error.code, + errors: errors.map((error) => { + return { + message: error.message, + type: error.type, + code: error.code, + } + }), }, } } else { @@ -350,6 +402,8 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { response_body: { data: swap, type: "swap" }, } } catch (error) { + await this.removeReservations(reservations) + if (error && error.code === MedusaError.Codes.INSUFFICIENT_INVENTORY) { return { response_code: 409, @@ -376,6 +430,8 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { try { order = await orderServiceTx.createFromCart(cart) } catch (error) { + await this.removeReservations(reservations) + if (error && error.message === ORDER_CART_ALREADY_EXISTS_ERROR) { order = await orderServiceTx.retrieveByCartId(id, { relations: ["shipping_address", "payments"], diff --git a/packages/medusa/src/utils/exception-formatter.ts b/packages/medusa/src/utils/exception-formatter.ts index 685ee3cf424da..c54b3ba487a50 100644 --- a/packages/medusa/src/utils/exception-formatter.ts +++ b/packages/medusa/src/utils/exception-formatter.ts @@ -4,6 +4,7 @@ export enum PostgresError { DUPLICATE_ERROR = "23505", FOREIGN_KEY_ERROR = "23503", SERIALIZATION_FAILURE = "40001", + NULL_VIOLATION = "23502", } export const formatException = (err): MedusaError => { @@ -43,6 +44,12 @@ export const formatException = (err): MedusaError => { err?.detail ?? err?.message ) } + case PostgresError.NULL_VIOLATION: { + return new MedusaError( + MedusaError.Types.INVALID_DATA, + `Can't insert null value in field ${err?.column} on insert in table ${err?.table}` + ) + } default: return err }