Skip to content

Commit

Permalink
fix: Minor fixes and cleanup to the payments setup (#11356)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sradevski authored Feb 9, 2025
1 parent 3dbef51 commit 702d338
Show file tree
Hide file tree
Showing 17 changed files with 344 additions and 262 deletions.
2 changes: 1 addition & 1 deletion integration-tests/http/__tests__/claims/claims.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,7 @@ medusaIntegrationTestRunner({
expect.objectContaining({
currency_code: "usd",
amount: paymentDelta,
status: "authorized",
status: "completed",
authorized_amount: paymentDelta,
captured_amount: paymentDelta,
refunded_amount: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,7 @@ medusaIntegrationTestRunner({
expect(paymentCollection).toEqual(
expect.objectContaining({
amount: 200,
// Q: Shouldn't this be paid?
status: "authorized",
status: "completed",
payment_sessions: [
expect.objectContaining({
status: "authorized",
Expand Down Expand Up @@ -639,8 +638,7 @@ medusaIntegrationTestRunner({
expect(paymentCollection).toEqual(
expect.objectContaining({
amount: 115.9,
// Q: Shouldn't this be paid?
status: "authorized",
status: "completed",
payment_sessions: [
expect.objectContaining({
status: "authorized",
Expand Down
2 changes: 2 additions & 0 deletions packages/core/core-flows/src/payment/steps/capture-payment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ export const capturePaymentStep = createStep(

return new StepResponse(payment)
}
// We don't want to compensate a capture automatically as the actual funds have already been taken.
// The only want to compensate here is to issue a refund, but it's better to leave that as a manual operation for now.
)
2 changes: 2 additions & 0 deletions packages/core/core-flows/src/payment/steps/refund-payment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ export const refundPaymentStep = createStep(

return new StepResponse(payment)
}
// We don't want to compensate a refund automatically as the actual funds have already been sent
// And in most cases we can't simply do another capture/authorization
)
3 changes: 2 additions & 1 deletion packages/core/types/src/http/payment/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export type BasePaymentCollectionStatus =
| "authorized"
| "partially_authorized"
| "canceled"

| "completed"
| "failed"
/**
*
* The status of a payment session.
Expand Down
2 changes: 2 additions & 0 deletions packages/core/types/src/payment/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export type PaymentCollectionStatus =
| "authorized"
| "partially_authorized"
| "canceled"
| "failed"
| "completed"

export type PaymentSessionStatus =
| "authorized"
Expand Down
Loading

0 comments on commit 702d338

Please sign in to comment.