Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(medusa-payment-stripe): Resend capture event to ensure auto-capture #3100

Merged
merged 2 commits into from Jan 25, 2023

Conversation

olivermrbl
Copy link
Contributor

What
When medusa-payment-stripe is configured to auto-capture, Stripe should re-send the event to ensure order capturing is always performed - eventually.

@olivermrbl olivermrbl requested a review from a team as a code owner January 24, 2023 14:03
@changeset-bot
Copy link

changeset-bot bot commented Jan 24, 2023

🦋 Changeset detected

Latest commit: 4de3af8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
medusa-payment-stripe Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@olivermrbl
Copy link
Contributor Author

olivermrbl commented Jan 24, 2023

Solves #2629

@oxgen1
Copy link

oxgen1 commented Jan 25, 2023

Lets go your a legend!

@olivermrbl
Copy link
Contributor Author

/snapshot-this

1 similar comment
@olivermrbl
Copy link
Contributor Author

/snapshot-this

@nekszt
Copy link

nekszt commented Jan 31, 2023

Thanks 🙏

@nekszt
Copy link

nekszt commented Feb 1, 2023

I have some feedback about this fix.
It's working :) Before I had to resend the event or capture manually each order.

But now the Paid status is set between few seconds and 1h (or a little more). It becomes less reactive (confirmation, notifications). I have to fulfill the orders 50min - 1h later.

I dont't know if we have another solution. I don't think we can delay the webhooks on Stripe side.

As far as I understand

This problem only occurs when capture payment is set to automatic and the payment intent is immediately in succeeded status after the confirmation.
Payment are not captured (on Medusa side) because the order was not yet created.

What I suggest

  1. What do you think about delaying the handleCartPayments function only in the case where we configured capture to automatic (medusa conf or better directly in the event payload "capture_method": "automatic") and the event received is payment_intent.succeeded
    Delaying with BullMQ? For 2-3 seconds (response to Stripe is send immediately)

Or

  1. Maybe an addition to this fix could be to create the order directly with the paid status when we receive from /v1/payment_intents/:id/confirm the "status": "succeeded"

One more thing

I would have added in the fix (to prevent other issues)

else (!order) {...}

@@ -31,6 +31,9 @@ export default async (req, res) => {
await manager.transaction(async (manager) => {
await orderService.withTransaction(manager).capturePayment(order.id)
})
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else (!order) could prevent special case

@nekszt
Copy link

nekszt commented Feb 1, 2023

I have that special case, I don't know how it happened but an order is already in Paid status (maybe captured manually but I'm not sure) and stripe is trying again and again to send the event.

The condition should be added
else (!order) {...}

@olivermrbl
Copy link
Contributor Author

@nekszt I'll look into it and open a PR for it 🙌

@minusplusminus
Copy link

I still have the same issue. To solve this, I added a subscriber that checks payment

import { EntityManager } from "typeorm";
import { IEventBusService } from "@medusajs/types";
import { OrderService } from "@medusajs/medusa";

export default class PaymentUpdateSubscriber {
  protected readonly manager_: EntityManager;
  protected readonly eventBus_: IEventBusService;
  protected readonly orderService_: OrderService;

  constructor(
    {
      manager,
      orderService,
      eventBusService
    }: {
      manager: EntityManager;
      eventBusService: IEventBusService;
      orderService: OrderService;
    }
  ) {
    this.manager_ = manager;
    this.eventBus_ = eventBusService;
    this.orderService_ = orderService;

    eventBusService.subscribe(OrderService.Events.PLACED, this.handleOrderPlaced);

  }

  handleOrderPlaced = async (data): Promise<any> => {
    await this.orderService_.capturePayment(data.id);
    return true;
  };
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants