From 6f4b2219711c1d9b018969ff3ada70da93592ee4 Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Thu, 15 Sep 2022 17:14:10 +0200 Subject: [PATCH] fix(medusa): Cleanup Tax lines in case of a failed cart completion (#2212) --- .../api/__tests__/store/orders.js | 126 +++++++++++++++++- .../api/src/services/test-pay.js | 6 + packages/medusa/src/services/cart.ts | 27 +++- .../medusa/src/services/idempotency-key.ts | 3 +- .../strategies/__tests__/cart-completion.js | 3 +- .../medusa/src/strategies/cart-completion.ts | 11 ++ 6 files changed, 170 insertions(+), 6 deletions(-) diff --git a/integration-tests/api/__tests__/store/orders.js b/integration-tests/api/__tests__/store/orders.js index 307063f5a0..c32a1b288d 100644 --- a/integration-tests/api/__tests__/store/orders.js +++ b/integration-tests/api/__tests__/store/orders.js @@ -8,12 +8,18 @@ const { ProductVariant, LineItem, Payment, + PaymentSession, } = require("@medusajs/medusa") const setupServer = require("../../../helpers/setup-server") const { useApi } = require("../../../helpers/use-api") const { initDb, useDb } = require("../../../helpers/use-db") -const { simpleRegionFactory, simpleProductFactory } = require("../../factories") +const { + simpleRegionFactory, + simpleProductFactory, + simpleCartFactory, + simpleShippingOptionFactory, +} = require("../../factories") const { MedusaError } = require("medusa-core-utils") jest.setTimeout(30000) @@ -299,4 +305,122 @@ describe("/store/carts", () => { ) }) }) + + describe("Cart completion with failed payment removes taxlines", () => { + afterEach(async () => { + const db = useDb() + await db.teardown() + }) + + it("should remove taxlines when a cart completion fails", async () => { + expect.assertions(2) + + const api = useApi() + + const region = await simpleRegionFactory(dbConnection) + const product = await simpleProductFactory(dbConnection) + + const cartRes = await api.post("/store/carts", {}) + const cartId = cartRes.data.cart.id + + await api.post(`/store/carts/${cartId}/line-items`, { + variant_id: product.variants[0].id, + quantity: 1, + }) + await api.post(`/store/carts/${cartId}`, { + email: "testmailer@medusajs.com", + }) + await api.post(`/store/carts/${cartId}/payment-sessions`) + + const cartResponse = await api.get(`/store/carts/${cartId}`) + + await dbConnection.manager.remove( + PaymentSession, + cartResponse.data.cart.payment_session + ) + + await api.post(`/store/carts/${cartId}/complete`).catch((err) => { + expect(err.response.status).toEqual(400) + }) + + const lineItem = await dbConnection.manager.findOne(LineItem, { + where: { + id: cartResponse.data.cart.items[0].id, + }, + relations: ["tax_lines"], + }) + + expect(lineItem.tax_lines).toHaveLength(0) + }) + + it("should remove taxlines when a payment authorization is pending", async () => { + const cartId = "cart-id-tax-line-testing-for-pending-payment" + + const api = useApi() + + const region = await simpleRegionFactory(dbConnection) + const shippingOption = await simpleShippingOptionFactory(dbConnection, { + region_id: region.id, + }) + const shippingOption1 = await simpleShippingOptionFactory(dbConnection, { + region_id: region.id, + }) + const product = await simpleProductFactory(dbConnection) + + const cart = await simpleCartFactory(dbConnection, { + region: region.id, + id: cartId, + }) + + await api.post( + `/store/carts/${cartId}/shipping-methods`, + { + option_id: shippingOption.id, + }, + { withCredentials: true } + ) + + await api.post(`/store/carts/${cartId}/line-items`, { + variant_id: product.variants[0].id, + quantity: 1, + }) + await api.post(`/store/carts/${cartId}`, { + email: "testmailer@medusajs.com", + }) + await api.post(`/store/carts/${cartId}/payment-sessions`) + + const cartResponse = await api.get(`/store/carts/${cartId}`) + + const completedOrder = await api.post(`/store/carts/${cartId}/complete`) + + expect(completedOrder.status).toEqual(200) + expect(completedOrder.data).toEqual( + expect.objectContaining({ + data: expect.any(Object), + payment_status: "pending", + type: "cart", + }) + ) + + const lineItem = await dbConnection.manager.findOne(LineItem, { + where: { + id: cartResponse.data.cart.items[0].id, + }, + relations: ["tax_lines"], + }) + + const cartWithShippingMethod1 = await api.post( + `/store/carts/${cartId}/shipping-methods`, + { + option_id: shippingOption1.id, + }, + { withCredentials: true } + ) + + expect( + cartWithShippingMethod1.data.cart.shipping_methods[0].shipping_option_id + ).toEqual(shippingOption1.id) + expect(lineItem.tax_lines).toHaveLength(0) + }) + }) }) diff --git a/integration-tests/api/src/services/test-pay.js b/integration-tests/api/src/services/test-pay.js index eb202c01c2..d8ff3ca494 100644 --- a/integration-tests/api/src/services/test-pay.js +++ b/integration-tests/api/src/services/test-pay.js @@ -42,6 +42,12 @@ class TestPayService extends AbstractPaymentService { } async authorizePayment(sessionData, context = {}) { + if ( + sessionData.cart_id === "cart-id-tax-line-testing-for-pending-payment" + ) { + return { data: {}, status: "pending" } + } + return { data: {}, status: "authorized" } } diff --git a/packages/medusa/src/services/cart.ts b/packages/medusa/src/services/cart.ts index 18ae1aea76..1b9769a04b 100644 --- a/packages/medusa/src/services/cart.ts +++ b/packages/medusa/src/services/cart.ts @@ -919,7 +919,7 @@ class CartService extends TransactionBaseService { cart.discounts.length = 0 await Promise.all( - data.discounts.map(({ code }) => { + data.discounts.map(async ({ code }) => { return this.applyDiscount(cart, code) }) ) @@ -948,7 +948,7 @@ class CartService extends TransactionBaseService { cart.gift_cards = [] await Promise.all( - (data.gift_cards ?? []).map(({ code }) => { + (data.gift_cards ?? []).map(async ({ code }) => { return this.applyGiftCard_(cart, code) }) ) @@ -1021,7 +1021,7 @@ class CartService extends TransactionBaseService { if (itemsToRemove.length) { const results = await Promise.all( - itemsToRemove.map((item) => { + itemsToRemove.map(async (item) => { return this.removeLineItem(cart.id, item.id) }) ) @@ -2092,9 +2092,11 @@ class CartService extends TransactionBaseService { } const updatedCart = await cartRepo.save(cart) + this.eventBus_ .withTransaction(transactionManager) .emit(CartService.Events.UPDATED, updatedCart) + return updatedCart } ) @@ -2131,6 +2133,25 @@ class CartService extends TransactionBaseService { ) } + async deleteTaxLines(id: string): Promise { + return await this.atomicPhase_( + async (transactionManager: EntityManager) => { + const cart = await this.retrieve(id, { + relations: [ + "items", + "items.tax_lines", + "shipping_methods", + "shipping_methods.tax_lines", + ], + }) + await transactionManager.remove(cart.items.flatMap((i) => i.tax_lines)) + await transactionManager.remove( + cart.shipping_methods.flatMap((s) => s.tax_lines) + ) + } + ) + } + protected async refreshAdjustments_(cart: Cart): Promise { const transactionManager = this.transactionManager_ ?? this.manager_ diff --git a/packages/medusa/src/services/idempotency-key.ts b/packages/medusa/src/services/idempotency-key.ts index 12e4b8c37a..8bd7272d46 100644 --- a/packages/medusa/src/services/idempotency-key.ts +++ b/packages/medusa/src/services/idempotency-key.ts @@ -20,7 +20,8 @@ class IdempotencyKeyService extends TransactionBaseService { protected readonly idempotencyKeyRepository_: typeof IdempotencyKeyRepository constructor({ manager, idempotencyKeyRepository }: InjectedDependencies) { - super({ manager, idempotencyKeyRepository }) + // eslint-disable-next-line prefer-rest-params + super(arguments[0]) this.manager_ = manager this.idempotencyKeyRepository_ = idempotencyKeyRepository diff --git a/packages/medusa/src/strategies/__tests__/cart-completion.js b/packages/medusa/src/strategies/__tests__/cart-completion.js index 492281b5e9..996d719b65 100644 --- a/packages/medusa/src/strategies/__tests__/cart-completion.js +++ b/packages/medusa/src/strategies/__tests__/cart-completion.js @@ -181,6 +181,7 @@ describe("CartCompletionStrategy", () => { return this }, createTaxLines: jest.fn(() => Promise.resolve(cart)), + deleteTaxLines: jest.fn(() => Promise.resolve(cart)), authorizePayment: jest.fn(() => Promise.resolve(cart)), retrieve: jest.fn(() => Promise.resolve(cart)), } @@ -205,7 +206,7 @@ describe("CartCompletionStrategy", () => { idempotencyKeyService: idempotencyKeyServiceMock, orderService: orderServiceMock, swapService: swapServiceMock, - manager: MockManager + manager: MockManager, }) const val = await completionStrat.complete(cart.id, idempotencyKey, {}) diff --git a/packages/medusa/src/strategies/cart-completion.ts b/packages/medusa/src/strategies/cart-completion.ts index 0ff7ea662f..74888b5c79 100644 --- a/packages/medusa/src/strategies/cart-completion.ts +++ b/packages/medusa/src/strategies/cart-completion.ts @@ -120,6 +120,10 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { cart.payment_session.status === "requires_more" || cart.payment_session.status === "pending" ) { + await cartService + .withTransaction(transactionManager) + .deleteTaxLines(id) + return { response_code: 200, response_body: { @@ -322,6 +326,13 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy { } if (err) { + if (idempotencyKey.recovery_point !== "started") { + await this.manager_.transaction(async (transactionManager) => { + await cartService + .withTransaction(transactionManager) + .deleteTaxLines(id) + }) + } throw err }