From 62d103b44f6402529a156202e37ec5ece01244b4 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 18 Nov 2025 10:50:50 +0100 Subject: [PATCH] =?UTF-8?q?fix(payment):=20Double=20idempotent=20capture?= =?UTF-8?q?=20called=20with=20auto=20capture=20beha=E2=80=A6=20(#14073)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(payment): Double idempotent capture called with auto capture behaviour * Create sweet-peaches-dress.md * naming * feedback * naming --- .changeset/sweet-peaches-dress.md | 6 + packages/core/types/src/payment/mutations.ts | 11 +- .../services/payment-module/index.spec.ts | 118 ++++++++++++++++++ .../payment/src/services/payment-module.ts | 78 ++++++++---- 4 files changed, 187 insertions(+), 26 deletions(-) create mode 100644 .changeset/sweet-peaches-dress.md diff --git a/.changeset/sweet-peaches-dress.md b/.changeset/sweet-peaches-dress.md new file mode 100644 index 0000000000..959427e1fb --- /dev/null +++ b/.changeset/sweet-peaches-dress.md @@ -0,0 +1,6 @@ +--- +"@medusajs/payment": patch +"@medusajs/types": patch +--- + +fix(payment): Double idempotent capture called with auto capture behavior diff --git a/packages/core/types/src/payment/mutations.ts b/packages/core/types/src/payment/mutations.ts index 7fdbf68df7..b3a7635739 100644 --- a/packages/core/types/src/payment/mutations.ts +++ b/packages/core/types/src/payment/mutations.ts @@ -1,6 +1,10 @@ import { BigNumberInput } from "../totals" import { PaymentCollectionStatus, PaymentSessionStatus } from "./common" -import { PaymentAccountHolderDTO, PaymentCustomerDTO, PaymentProviderContext, } from "./provider" +import { + PaymentAccountHolderDTO, + PaymentCustomerDTO, + PaymentProviderContext, +} from "./provider" /** * The payment collection to be created. @@ -147,6 +151,11 @@ export interface CreateCaptureDTO { * a user's ID. */ captured_by?: string + + /** + * Whether the capture was automatically captured. + */ + is_captured?: boolean } /** diff --git a/packages/modules/payment/integration-tests/__tests__/services/payment-module/index.spec.ts b/packages/modules/payment/integration-tests/__tests__/services/payment-module/index.spec.ts index 3343e54d09..27842b494d 100644 --- a/packages/modules/payment/integration-tests/__tests__/services/payment-module/index.spec.ts +++ b/packages/modules/payment/integration-tests/__tests__/services/payment-module/index.spec.ts @@ -631,6 +631,62 @@ moduleIntegrationTestRunner({ }) ) }) + + it("should auto capture payment when provider returns captured status", async () => { + const collection = await service.createPaymentCollections({ + amount: 200, + currency_code: "usd", + }) + + const session = await service.createPaymentSession(collection.id, { + provider_id: "pp_system_default", + amount: 100, + currency_code: "usd", + data: {}, + context: { + customer: { id: "cus-id-1", email: "new@test.tsst" }, + }, + }) + + // Mock the provider to return CAPTURED status + const authorizePaymentMock = jest + .spyOn( + (service as any).paymentProviderService_, + "authorizePayment" + ) + .mockResolvedValueOnce({ + data: { payment_id: "external_payment_id" }, + status: "captured", + }) + + const capturePaymentMock = jest.spyOn( + (service as any).paymentProviderService_, + "capturePayment" + ) + + const payment = await service.authorizePaymentSession( + session.id, + {} + ) + + expect(authorizePaymentMock).toHaveBeenCalledTimes(1) + expect(capturePaymentMock).not.toHaveBeenCalled() + + expect(payment).toEqual( + expect.objectContaining({ + id: expect.any(String), + amount: 100, + currency_code: "usd", + provider_id: "pp_system_default", + captured_at: expect.any(Date), + captures: [expect.objectContaining({})], + payment_session: expect.objectContaining({ + status: "authorized", + authorized_at: expect.any(Date), + }), + }) + ) + }) }) }) @@ -780,6 +836,68 @@ moduleIntegrationTestRunner({ "The payment: pay-id-1 has been canceled." ) }) + + it("should not call provider capturePayment for auto-captured payments", async () => { + const collection = await service.createPaymentCollections({ + amount: 200, + currency_code: "usd", + }) + + const session = await service.createPaymentSession(collection.id, { + provider_id: "pp_system_default", + amount: 100, + currency_code: "usd", + data: {}, + context: { + customer: { id: "cus-id-1", email: "new@test.tsst" }, + }, + }) + + // Mock the provider to return CAPTURED status for auto-capture + jest + .spyOn( + (service as any).paymentProviderService_, + "authorizePayment" + ) + .mockResolvedValueOnce({ + data: { payment_id: "external_payment_id" }, + status: "captured", + }) + + const payment = await service.authorizePaymentSession( + session.id, + {} + ) + + // Spy on capturePayment provider method + const capturePaymentMock = jest.spyOn( + (service as any).paymentProviderService_, + "capturePayment" + ) + + // Try to capture the already auto-captured payment + const capturedPayment = await service.capturePayment({ + amount: 100, + payment_id: payment.id, + }) + + // Provider's capturePayment should NOT be called since it was auto-captured + expect(capturePaymentMock).not.toHaveBeenCalled() + + // Verify data consistency + expect(capturedPayment).toEqual( + expect.objectContaining({ + id: payment.id, + amount: 100, + captured_at: expect.any(Date), + captures: [ + expect.objectContaining({ + amount: 100, + }), + ], + }) + ) + }) }) describe("refund", () => { diff --git a/packages/modules/payment/src/services/payment-module.ts b/packages/modules/payment/src/services/payment-module.ts index 2fe00e2615..8ec67f48a2 100644 --- a/packages/modules/payment/src/services/payment-module.ts +++ b/packages/modules/payment/src/services/payment-module.ts @@ -585,10 +585,10 @@ export default class PaymentModuleService status: PaymentSessionStatus, @MedusaContext() sharedContext?: Context ): Promise> { - let autoCapture = false + let isCaptured = false if (status === PaymentSessionStatus.CAPTURED) { status = PaymentSessionStatus.AUTHORIZED - autoCapture = true + isCaptured = true } await this.paymentSessionService_.update( @@ -617,9 +617,13 @@ export default class PaymentModuleService sharedContext ) - if (autoCapture) { + if (isCaptured) { await this.capturePayment( - { payment_id: payment.id, amount: session.amount as BigNumberInput }, + { + payment_id: payment.id, + amount: session.amount as BigNumberInput, + is_captured: isCaptured, + }, sharedContext ) } @@ -646,8 +650,9 @@ export default class PaymentModuleService data: CreateCaptureDTO, @MedusaContext() sharedContext: Context = {} ): Promise { + let { is_captured, ...data_ } = data const payment = await this.paymentService_.retrieve( - data.payment_id, + data_.payment_id, { select: [ "id", @@ -665,8 +670,14 @@ export default class PaymentModuleService sharedContext ) + let isCaptured = is_captured + if (!isCaptured) { + const isAutoCaptured = !!payment?.captured_at + isCaptured = isAutoCaptured + } + const { isFullyCaptured, capture } = await this.capturePayment_( - data, + data_, payment, sharedContext ) @@ -675,7 +686,7 @@ export default class PaymentModuleService await this.capturePaymentFromProvider_( payment, capture, - isFullyCaptured, + { isFullyCaptured, isCaptured }, sharedContext ) } catch (error) { @@ -763,27 +774,44 @@ export default class PaymentModuleService protected async capturePaymentFromProvider_( payment: InferEntityType, capture: InferEntityType | undefined, - isFullyCaptured: boolean, + options: { + isFullyCaptured?: boolean + isCaptured?: boolean + } = {}, @MedusaContext() sharedContext: Context = {} ) { - const paymentData = await this.paymentProviderService_.capturePayment( - payment.provider_id, - { - data: payment.data!, - context: { - idempotency_key: capture?.id, - }, - } - ) + if (!options.isCaptured) { + const paymentData = await this.paymentProviderService_.capturePayment( + payment.provider_id, + { + data: payment.data!, + context: { + idempotency_key: capture?.id, + }, + } + ) - await this.paymentService_.update( - { - id: payment.id, - data: paymentData.data, - captured_at: isFullyCaptured ? new Date() : undefined, - }, - sharedContext - ) + await this.paymentService_.update( + { + id: payment.id, + data: paymentData.data, + captured_at: options.isFullyCaptured ? new Date() : undefined, + }, + sharedContext + ) + } else if (options.isFullyCaptured && !payment.captured_at) { + /** + * In the case of auto capture, we need to update the payment to set the captured_at date + * only if fully captured. + */ + await this.paymentService_.update( + { + id: payment.id, + captured_at: new Date(), + }, + sharedContext + ) + } return payment }