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

feat(medusa-payment-stripe-processor): Start implementing the new stripe payment plugins #3257

Merged
merged 42 commits into from
Feb 28, 2023

Conversation

adrien2p
Copy link
Member

@adrien2p adrien2p commented Feb 14, 2023

  • Updating stripe plugin to the new processor API
  • Adjusting the plugin loader to allow loading a plugin which output the build into dist
  • Allow a plugin to output in dist and still being load
  • Fix stripe webhook to be aligned with the code base and use the cart completion strategy

TODO

  • Align the legacy stripe plugin web hook to do the same as the new plugin

TEST

  • The webhook has been tested on a complete flow multiple times
  • rewrite all the unit tests

RESOLVE CORE-1092
RESOLVE CORE-876
RESOLVE CORE-1120

@changeset-bot
Copy link

changeset-bot bot commented Feb 14, 2023

🦋 Changeset detected

Latest commit: 3534810

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

This PR includes changesets to release 8 packages
Name Type
medusa-payment-paypal Minor
medusa-payment-stripe Major
@medusajs/medusa Minor
@medusajs/inventory Major
@medusajs/medusa-js Major
medusa-plugin-restock-notification Major
medusa-react Major
@medusajs/stock-location Major

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

@adrien2p adrien2p force-pushed the feat/stripe-payment-processor branch 2 times, most recently from 5aef163 to 96b155e Compare February 15, 2023 11:24
@adrien2p adrien2p force-pushed the feat/stripe-payment-processor branch from 1e7e844 to 25ebe66 Compare February 20, 2023 14:00
@adrien2p adrien2p changed the base branch from feat/payment-processor-loader-provider to develop February 21, 2023 08:52
@adrien2p adrien2p force-pushed the feat/stripe-payment-processor branch from fae866b to 5307b8a Compare February 21, 2023 08:55
@pepijn-vanvlaanderen
Copy link
Contributor

Thanks for the quick reply! There is indeed an order creation when the webhook receives an event for payment_intent.amount_capturable_updated, however this event is never called for iDEAL payments.

This also is an issue for the payment methods Blik or Przelewy24 as mentioned by @chemicalkosek in his reply here: #2948 (comment)

@adrien2p
Copy link
Member Author

adrien2p commented Feb 25, 2023

@pepijn-vanvlaanderen thank you for the info, i ll discuss it and have a look to implement this 👌

cc @olivermrbl

@olivermrbl
Copy link
Contributor

Thanks for sharing @pepijn-vanvlaanderen. I recall you added a similar comment on a previous PR. Sorry for not getting back on that one - I completely sweat it out.

My immediate reflection on your request is that it would be reasonable to create an order in the webhook when receiving the payment_intent.succeeded, as this is what will be called in the scenario of a customer closing their tab, losing connection etc. after having submitted their request to complete cart.

The implementation specifics should be identical to the one of the payment_intent.amount_capturable_updated event, so it will be straightforward to add.

@adrien2p and I will discuss and run a couple of test early next week of the different flows and get back :)

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Strong work @adrien2p! 💪

The first review is centered around the webhook functionality. Will do a follow-up review for the services later today or tomorrow.

packages/medusa-payment-stripe/src/api/hooks/index.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/core/stripe-base.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/__fixtures__/data.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/api/hooks/stripe.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/api/hooks/stripe.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/api/hooks/stripe.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/api/hooks/stripe.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/api/hooks/stripe.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/api/hooks/stripe.ts Outdated Show resolved Hide resolved
@adrien2p
Copy link
Member Author

@olivermrbl I added all your feedback and some more cleanup. Let me know what you think 💪

@olivermrbl
Copy link
Contributor

@adrien2p Will do! In the meantime, please add a changeset to the PR :)

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Strong work man!

packages/medusa/src/api/middlewares/index.ts Outdated Show resolved Hide resolved
packages/medusa/src/loaders/helpers/plugins.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/core/stripe-base.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/core/stripe-base.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/core/stripe-base.ts Outdated Show resolved Hide resolved
packages/medusa-payment-stripe/src/core/stripe-base.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Very nice! Only minor nitpicking. Feel free to merge afterward :)

packages/medusa-payment-stripe/src/types.ts Outdated Show resolved Hide resolved
packages/medusa/src/services/idempotency-key.ts Outdated Show resolved Hide resolved
packages/medusa/src/services/idempotency-key.ts Outdated Show resolved Hide resolved
@olivermrbl
Copy link
Contributor

@adrien2p Merging this one - cool?

@olivermrbl olivermrbl merged commit 589d1c0 into develop Feb 28, 2023
@olivermrbl olivermrbl deleted the feat/stripe-payment-processor branch February 28, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants