asyavee/medusa-store
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: Minor fixes and cleanup to the payments setup (#11356)

Browse Source 
This PR adds a couple new statuses to the payment collection and payment webhook results. The payment collection will now be marked as "completed" once the captured amount is the full amount of the payment collection.

There are several things left to improve the payment setup, so non-happy-path cases are handled correctly.
1. Currently the payment session and payment models serve a very similar purpose. Part of the information is found on one, and the other part on the other model, without any clear reason for doing so. We can simplify the payment module and the data models simply by merging the two.
2. We need to handle failures more gracefully, such as setting the payment session status to failed when such a webhook comes in.
3. We should convert the payment collection status and the different amounts to calculated fields from the payment session, captures, and refunds, as they can easily be a source of inconsistencies.
This commit is contained in:
Stevche Radevski
2025-02-09 16:42:02 +01:00
committed by GitHub
parent 3dbef519d9
commit 702d338284
17 changed files with 344 additions and 262 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/modules/payment/integration-tests/__tests__/services/payment-module/index.spec.ts
View File

@@ -139,7 +139,7 @@ moduleIntegrationTestRunner<IPaymentModuleService>({
amount: 200,
authorized_amount: 200,
captured_amount: 200,
status: "authorized",
status: "completed",
deleted_at: null,
completed_at: expect.any(Date),
payment_sessions: [

4
packages/modules/payment/src/migrations/.snapshot-medusa-payment.json
View File

@@ -209,7 +209,9 @@
"awaiting",
"authorized",
"partially_authorized",
"canceled"
"canceled",
"failed",
"completed"
],
"mappedType": "enum"
},

17
packages/modules/payment/src/migrations/Migration20250207132723.ts Normal file
View File

@@ -0,0 +1,17 @@
import { Migration } from '@mikro-orm/migrations';
export class Migration20250207132723 extends Migration {
override async up(): Promise<void> {
this.addSql(`alter table if exists "payment_collection" drop constraint if exists "payment_collection_status_check";`);
this.addSql(`alter table if exists "payment_collection" add constraint "payment_collection_status_check" check("status" in ('not_paid', 'awaiting', 'authorized', 'partially_authorized', 'canceled', 'failed', 'completed'));`);
}
override async down(): Promise<void> {
this.addSql(`alter table if exists "payment_collection" drop constraint if exists "payment_collection_status_check";`);
this.addSql(`alter table if exists "payment_collection" add constraint "payment_collection_status_check" check("status" in ('not_paid', 'awaiting', 'authorized', 'partially_authorized', 'canceled'));`);
}
}

2
packages/modules/payment/src/models/payment.ts
View File

@@ -4,6 +4,8 @@ import PaymentCollection from "./payment-collection"
import PaymentSession from "./payment-session"
import Refund from "./refund"
// TODO: We should remove the `Payment` model and use the `PaymentSession` model instead.
// We just need to move the refunds, captures, canceled_at, and captured_at to it.
const Payment = model
.define("Payment", {
id: model.id({ prefix: "pay" }).primaryKey(),

327
packages/modules/payment/src/services/payment-module.ts
View File

@@ -12,7 +12,6 @@ import {
FilterablePaymentCollectionProps,
FilterablePaymentMethodProps,
FilterablePaymentProviderProps,
FilterablePaymentSessionProps,
FindConfig,
InferEntityType,
InternalModuleDeclaration,
@@ -303,6 +302,7 @@ export default class PaymentModuleService
sharedContext?: Context
): Promise<PaymentCollectionDTO[]>
// Should we remove this and use `updatePaymentCollections` instead?
@InjectManager()
async completePaymentCollections(
paymentCollectionId: string | string[],
@@ -415,6 +415,12 @@ export default class PaymentModuleService
sharedContext
)
await this.paymentProviderService_.updateSession(session.provider_id, {
data: data.data,
amount: data.amount,
currency_code: data.currency_code,
})
const updated = await this.paymentSessionService_.update(
{
id: session.id,
@@ -491,7 +497,7 @@ export default class PaymentModuleService
) {
throw new MedusaError(
MedusaError.Types.NOT_ALLOWED,
`Session: ${session.id} is not authorized with the provider.`
`Session: ${session.id} was not authorized with the provider.`
)
}
@@ -516,11 +522,9 @@ export default class PaymentModuleService
sharedContext
)
return await this.retrievePayment(
payment.id,
{ relations: ["payment_collection"] },
sharedContext
)
return await this.baseRepository_.serialize(payment, {
populate: true,
})
}
@InjectTransactionManager()
@@ -541,8 +545,11 @@ export default class PaymentModuleService
id: session.id,
data,
status,
authorized_at:
status === PaymentSessionStatus.AUTHORIZED ? new Date() : null,
...(session.authorized_at === null
? {
authorized_at: new Date(),
}
: {}),
},
sharedContext
)
@@ -569,38 +576,6 @@ export default class PaymentModuleService
return payment
}
@InjectManager()
// @ts-expect-error
async retrievePaymentSession(
id: string,
config: FindConfig<PaymentSessionDTO> = {},
@MedusaContext() sharedContext?: Context
): Promise<PaymentSessionDTO> {
const session = await this.paymentSessionService_.retrieve(
id,
config,
sharedContext
)
return await this.baseRepository_.serialize(session)
}
@InjectManager()
// @ts-expect-error
async listPaymentSessions(
filters?: FilterablePaymentSessionProps,
config?: FindConfig<PaymentSessionDTO>,
sharedContext?: Context
): Promise<PaymentSessionDTO[]> {
const sessions = await this.paymentSessionService_.list(
filters,
config,
sharedContext
)
return await this.baseRepository_.serialize<PaymentSessionDTO[]>(sessions)
}
@InjectManager()
async updatePayment(
data: UpdatePaymentDTO,
@@ -612,13 +587,33 @@ export default class PaymentModuleService
return await this.baseRepository_.serialize<PaymentDTO>(result[0])
}
// TODO: This method should return a capture, not a payment
@InjectManager()
async capturePayment(
data: CreateCaptureDTO,
@MedusaContext() sharedContext: Context = {}
): Promise<PaymentDTO> {
const { payment, isFullyCaptured, capture } = await this.capturePayment_(
const payment = await this.paymentService_.retrieve(
data.payment_id,
{
select: [
"id",
"data",
"provider_id",
"payment_collection_id",
"amount",
"raw_amount",
"captured_at",
"canceled_at",
],
relations: ["captures.raw_amount"],
},
sharedContext
)
const { isFullyCaptured, capture } = await this.capturePayment_(
data,
payment,
sharedContext
)
@@ -640,45 +635,20 @@ export default class PaymentModuleService
sharedContext
)
return await this.retrievePayment(
payment.id,
{ relations: ["captures"] },
sharedContext
)
return await this.baseRepository_.serialize(payment, {
populate: true,
})
}
@InjectTransactionManager()
private async capturePayment_(
data: CreateCaptureDTO,
payment: InferEntityType<typeof Payment>,
@MedusaContext() sharedContext: Context = {}
): Promise<{
payment: InferEntityType<typeof Payment>
isFullyCaptured: boolean
capture?: InferEntityType<typeof Capture>
}> {
const payment = await this.paymentService_.retrieve(
data.payment_id,
{
select: [
"id",
"data",
"provider_id",
"payment_collection_id",
"amount",
"raw_amount",
"captured_at",
"canceled_at",
],
relations: ["captures.raw_amount"],
},
sharedContext
)
// If no custom amount is passed, we assume the full amount needs to be captured
if (!data.amount) {
data.amount = payment.amount as number
}
if (payment.canceled_at) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
@@ -687,7 +657,12 @@ export default class PaymentModuleService
}
if (payment.captured_at) {
return { payment, isFullyCaptured: true }
return { isFullyCaptured: true }
}
// If no custom amount is passed, we assume the full amount needs to be captured
if (!data.amount) {
data.amount = payment.amount as number
}
const capturedAmount = payment.captures.reduce((captureAmount, next) => {
@@ -720,7 +695,7 @@ export default class PaymentModuleService
sharedContext
)
return { payment, isFullyCaptured, capture }
return { isFullyCaptured, capture }
}
@InjectManager()
private async capturePaymentFromProvider_(
@@ -875,15 +850,79 @@ export default class PaymentModuleService
}
@InjectManager()
async getWebhookActionAndData(
eventData: ProviderWebhookPayload,
@MedusaContext() sharedContext?: Context
): Promise<WebhookActionResult> {
const providerId = `pp_${eventData.provider}`
private async maybeUpdatePaymentCollection_(
paymentCollectionId: string,
sharedContext?: Context
) {
const paymentCollection = await this.paymentCollectionService_.retrieve(
paymentCollectionId,
{
select: ["amount", "raw_amount", "status"],
relations: [
"payment_sessions.amount",
"payment_sessions.raw_amount",
"payments.captures.amount",
"payments.captures.raw_amount",
"payments.refunds.amount",
"payments.refunds.raw_amount",
],
},
sharedContext
)
return await this.paymentProviderService_.getWebhookActionAndData(
providerId,
eventData.payload
const paymentSessions = paymentCollection.payment_sessions
const captures = paymentCollection.payments
.map((pay) => [...pay.captures])
.flat()
const refunds = paymentCollection.payments
.map((pay) => [...pay.refunds])
.flat()
let authorizedAmount = MathBN.convert(0)
let capturedAmount = MathBN.convert(0)
let refundedAmount = MathBN.convert(0)
let completedAt: Date | undefined
for (const ps of paymentSessions) {
if (ps.status === PaymentSessionStatus.AUTHORIZED) {
authorizedAmount = MathBN.add(authorizedAmount, ps.amount)
}
}
for (const capture of captures) {
capturedAmount = MathBN.add(capturedAmount, capture.amount)
}
for (const refund of refunds) {
refundedAmount = MathBN.add(refundedAmount, refund.amount)
}
let status =
paymentSessions.length === 0
? PaymentCollectionStatus.NOT_PAID
: PaymentCollectionStatus.AWAITING
if (MathBN.gt(authorizedAmount, 0)) {
status = MathBN.gte(authorizedAmount, paymentCollection.amount)
? PaymentCollectionStatus.AUTHORIZED
: PaymentCollectionStatus.PARTIALLY_AUTHORIZED
}
if (MathBN.eq(paymentCollection.amount, capturedAmount)) {
status = PaymentCollectionStatus.COMPLETED
completedAt = new Date()
}
await this.paymentCollectionService_.update(
{
id: paymentCollectionId,
status,
authorized_amount: authorizedAmount,
captured_amount: capturedAmount,
refunded_amount: refundedAmount,
completed_at: completedAt,
},
sharedContext
)
}
@@ -939,44 +978,23 @@ export default class PaymentModuleService
let accountHolder: InferEntityType<typeof AccountHolder> | undefined
let providerAccountHolder: CreateAccountHolderOutput | undefined
try {
providerAccountHolder =
await this.paymentProviderService_.createAccountHolder(
input.provider_id,
{ context: input.context }
)
providerAccountHolder =
await this.paymentProviderService_.createAccountHolder(
input.provider_id,
{ context: input.context }
)
// This can be empty when either the method is not supported or an account holder wasn't created
if (isPresent(providerAccountHolder)) {
accountHolder = await this.accountHolderService_.create(
{
external_id: providerAccountHolder.id,
email: input.context.customer?.email,
data: providerAccountHolder.data,
provider_id: input.provider_id,
},
sharedContext
)
}
} catch (error) {
if (providerAccountHolder) {
await this.paymentProviderService_.deleteAccountHolder(
input.provider_id,
{
context: {
account_holder: providerAccountHolder as {
data: Record<string, unknown>
},
},
}
)
}
if (accountHolder) {
await this.accountHolderService_.delete(accountHolder.id, sharedContext)
}
throw error
// This can be empty when either the method is not supported or an account holder wasn't created
if (isPresent(providerAccountHolder)) {
accountHolder = await this.accountHolderService_.create(
{
external_id: providerAccountHolder.id,
email: input.context.customer?.email,
data: providerAccountHolder.data,
provider_id: input.provider_id,
},
sharedContext
)
}
return await this.baseRepository_.serialize(accountHolder)
@@ -1078,72 +1096,15 @@ export default class PaymentModuleService
}
@InjectManager()
private async maybeUpdatePaymentCollection_(
paymentCollectionId: string,
sharedContext?: Context
) {
const paymentCollection = await this.paymentCollectionService_.retrieve(
paymentCollectionId,
{
select: ["amount", "raw_amount", "status"],
relations: [
"payment_sessions.amount",
"payment_sessions.raw_amount",
"payments.captures.amount",
"payments.captures.raw_amount",
"payments.refunds.amount",
"payments.refunds.raw_amount",
],
},
sharedContext
)
async getWebhookActionAndData(
eventData: ProviderWebhookPayload,
@MedusaContext() sharedContext?: Context
): Promise<WebhookActionResult> {
const providerId = `pp_${eventData.provider}`
const paymentSessions = paymentCollection.payment_sessions
const captures = paymentCollection.payments
.map((pay) => [...pay.captures])
.flat()
const refunds = paymentCollection.payments
.map((pay) => [...pay.refunds])
.flat()
let authorizedAmount = MathBN.convert(0)
let capturedAmount = MathBN.convert(0)
let refundedAmount = MathBN.convert(0)
for (const ps of paymentSessions) {
if (ps.status === PaymentSessionStatus.AUTHORIZED) {
authorizedAmount = MathBN.add(authorizedAmount, ps.amount)
}
}
for (const capture of captures) {
capturedAmount = MathBN.add(capturedAmount, capture.amount)
}
for (const refund of refunds) {
refundedAmount = MathBN.add(refundedAmount, refund.amount)
}
let status =
paymentSessions.length === 0
? PaymentCollectionStatus.NOT_PAID
: PaymentCollectionStatus.AWAITING
if (MathBN.gt(authorizedAmount, 0)) {
status = MathBN.gte(authorizedAmount, paymentCollection.amount)
? PaymentCollectionStatus.AUTHORIZED
: PaymentCollectionStatus.PARTIALLY_AUTHORIZED
}
await this.paymentCollectionService_.update(
{
id: paymentCollectionId,
status,
authorized_amount: authorizedAmount,
captured_amount: capturedAmount,
refunded_amount: refundedAmount,
},
sharedContext
return await this.paymentProviderService_.getWebhookActionAndData(
providerId,
eventData.payload
)
}
}

65
packages/modules/providers/payment-stripe/src/core/stripe-base.ts
View File

@@ -125,22 +125,30 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
}
const paymentIntent = await this.stripe_.paymentIntents.retrieve(id)
const dataResponse = paymentIntent as unknown as Record<string, unknown>
switch (paymentIntent.status) {
case "requires_payment_method":
if (paymentIntent.last_payment_error) {
return { status: PaymentSessionStatus.ERROR, data: dataResponse }
}
return { status: PaymentSessionStatus.PENDING, data: dataResponse }
case "requires_confirmation":
case "processing":
return { status: PaymentSessionStatus.PENDING }
return { status: PaymentSessionStatus.PENDING, data: dataResponse }
case "requires_action":
return { status: PaymentSessionStatus.REQUIRES_MORE }
return {
status: PaymentSessionStatus.REQUIRES_MORE,
data: dataResponse,
}
case "canceled":
return { status: PaymentSessionStatus.CANCELED }
return { status: PaymentSessionStatus.CANCELED, data: dataResponse }
case "requires_capture":
return { status: PaymentSessionStatus.AUTHORIZED }
return { status: PaymentSessionStatus.AUTHORIZED, data: dataResponse }
case "succeeded":
return { status: PaymentSessionStatus.CAPTURED }
return { status: PaymentSessionStatus.CAPTURED, data: dataResponse }
default:
return { status: PaymentSessionStatus.PENDING }
return { status: PaymentSessionStatus.PENDING, data: dataResponse }
}
}
@@ -419,24 +427,23 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
const intent = event.data.object as Stripe.PaymentIntent
const { currency } = intent
switch (event.type) {
case "payment_intent.amount_capturable_updated":
case "payment_intent.created":
case "payment_intent.processing":
return {
action: PaymentActions.AUTHORIZED,
action: PaymentActions.PENDING,
data: {
session_id: intent.metadata.session_id,
amount: getAmountFromSmallestUnit(
intent.amount_capturable,
currency
), // NOTE: revisit when implementing multicapture
amount: getAmountFromSmallestUnit(intent.amount, currency),
},
}
case "payment_intent.succeeded":
case "payment_intent.canceled":
return {
action: PaymentActions.SUCCESSFUL,
action: PaymentActions.CANCELED,
data: {
session_id: intent.metadata.session_id,
amount: getAmountFromSmallestUnit(intent.amount_received, currency),
amount: getAmountFromSmallestUnit(intent.amount, currency),
},
}
case "payment_intent.payment_failed":
@@ -447,6 +454,34 @@ abstract class StripeBase extends AbstractPaymentProvider<StripeOptions> {
amount: getAmountFromSmallestUnit(intent.amount, currency),
},
}
case "payment_intent.requires_action":
return {
action: PaymentActions.REQUIRES_MORE,
data: {
session_id: intent.metadata.session_id,
amount: getAmountFromSmallestUnit(intent.amount, currency),
},
}
case "payment_intent.amount_capturable_updated":
return {
action: PaymentActions.AUTHORIZED,
data: {
session_id: intent.metadata.session_id,
amount: getAmountFromSmallestUnit(
intent.amount_capturable,
currency
),
},
}
case "payment_intent.succeeded":
return {
action: PaymentActions.SUCCESSFUL,
data: {
session_id: intent.metadata.session_id,
amount: getAmountFromSmallestUnit(intent.amount_received, currency),
},
}
default:
return { action: PaymentActions.NOT_SUPPORTED }
}