Skip to content

Implement support for deferred PaymenIntent confirmation#249

Merged
elia merged 10 commits into
masterfrom
elia+rainer/thanks-stripe-beta-team
Mar 31, 2023
Merged

Implement support for deferred PaymenIntent confirmation#249
elia merged 10 commits into
masterfrom
elia+rainer/thanks-stripe-beta-team

Conversation

@elia

@elia elia commented Mar 23, 2023

Copy link
Copy Markdown
Member

Summary

Fixes #209, Fixes #185, Fixes #190

This PR improves the payment process by replacing the double flow of Payment Intent/Setup Intent with a Payment Intent flow in a specific scenario described in the Stripe documentation:
Build a two-step confirmation experience

With the new approach, the two-step subdivision achieves the following issues:

  • The creation of the Stripe payment method remains in the payment step (as before)
  • The Payment intent creation will be invoked during the confirm step rendering.
  • The payment authorization occurs when the order is submitted on the confirm page.

The benefits of this update include:

  • Removing compromises in the payment intent flow that previously required "skipping" the confirm step due to the premature authorization executed in the payment step.
  • Aligning the authorization process with the Solidus checkout flow.
  • Removing the authorize and purchase methods from the gateway, as they are no longer needed.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch 2 times, most recently from 62fe534 to ed6c05a Compare March 23, 2023 17:05

@waiting-for-dev waiting-for-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So much simpler!

Comment thread db/migrate/20230323154931_drop_solidus_stripe_setup_intent.rb
@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch 4 times, most recently from 48c9078 to f956b85 Compare March 27, 2023 16:54
@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch 2 times, most recently from 6401306 to 61400f8 Compare March 28, 2023 09:39
@codecov

codecov Bot commented Mar 28, 2023

Copy link
Copy Markdown

Codecov Report

Merging #249 (1cab1dc) into master (7396336) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
- Coverage   99.79%   99.77%   -0.03%     
==========================================
  Files          25       24       -1     
  Lines         487      442      -45     
==========================================
- Hits          486      441      -45     
  Misses          1        1              
Impacted Files Coverage Δ
app/models/solidus_stripe/gateway.rb 100.00% <ø> (ø)
app/models/solidus_stripe/payment_method.rb 100.00% <ø> (ø)
lib/solidus_stripe/testing_support/factories.rb 100.00% <ø> (ø)
...p/controllers/solidus_stripe/intents_controller.rb 100.00% <100.00%> (ø)
app/models/solidus_stripe/payment_intent.rb 100.00% <100.00%> (ø)
...lates/app/views/checkouts/payment/_stripe.html.erb 100.00% <100.00%> (ø)
...tes/app/views/orders/payment_info/_stripe.html.erb 100.00% <100.00%> (ø)
lib/solidus_stripe/engine.rb 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch 3 times, most recently from f8a1929 to e7339e6 Compare March 28, 2023 11:44
@rainerdema rainerdema added the enhancement New feature or request label Mar 28, 2023
@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch 2 times, most recently from 29f4ead to a51c98c Compare March 28, 2023 14:56
@elia elia mentioned this pull request Mar 28, 2023
3 tasks
@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch from a51c98c to 15d972e Compare March 28, 2023 15:53
@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch from 15d972e to 10ac57d Compare March 28, 2023 16:31
@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch 2 times, most recently from fcdad41 to 3e9b132 Compare March 29, 2023 08:38
@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch 2 times, most recently from 8c14d2d to 45e6d94 Compare March 29, 2023 15:45
@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch from 45e6d94 to 1e1a6aa Compare March 29, 2023 16:04
@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch from 1e1a6aa to 64caa28 Compare March 29, 2023 16:12
@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch from 64caa28 to 265aa6e Compare March 29, 2023 16:42
@rainerdema rainerdema marked this pull request as ready for review March 30, 2023 08:48

@kennyadsl kennyadsl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe spotted a method that we are not using, other than that, 👍

Comment thread app/models/solidus_stripe/payment_intent.rb Outdated
@rainerdema rainerdema requested a review from kennyadsl March 30, 2023 10:05
elia and others added 2 commits March 30, 2023 12:44
Co-Authored-By: Rainer Dema <rainer.dema@gmail.com>
Co-Authored-By: Rainer Dema <rainer.dema@gmail.com>
@rainerdema rainerdema force-pushed the elia+rainer/thanks-stripe-beta-team branch from 265aa6e to 0723ed7 Compare March 30, 2023 10:44

@waiting-for-dev waiting-for-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, @elia & @rainerdema! ❤️ Left one concern about the removal of the gateway methods.

I'm also wondering if we need the SolidusStripe::PaymentIntent model at all. It's not needed from the webhook handlers, as the response_code is filled on payment intent creation. Can you help me understand? Is it only needed in the case a user refreshes the confirm page?

Comment thread app/controllers/solidus_stripe/intents_controller.rb Outdated
Comment thread app/models/solidus_stripe/gateway.rb
elia and others added 8 commits March 30, 2023 13:46
Co-Authored-By: Rainer Dema <rainer.dema@gmail.com>
Since we're memoizing and the nature of the intent is pretty dynamic
(e.g. it can change its status) we want to reset it whenever the
record is reloaded.
…ment sources

After a successful payment confirmation through a payment intent with `setup_future_usage`
set to 'on_session' or 'off_session', the payment source (Stripe payment method)
can be considered valid for future reuse.

For more details, refer to the Stripe documentation:
https://stripe.com/docs/api/payment_intents/update#update_payment_intent-setup_future_usage

This commit reintroduces the functionality of adding the payment source to the user's
wallet after a successful payment, so that the user can reuse it for future
transactions without having to enter payment information again.
This commit removes the `authorize` and `purchase` methods as they are now handled client-side.

Co-Authored-By: Elia Schito <elia@schito.me>
It now doesn't handle intents anymore.
This was fixed by #213.
@elia elia force-pushed the elia+rainer/thanks-stripe-beta-team branch from 0723ed7 to 1cab1dc Compare March 30, 2023 11:57
@elia elia merged commit 6736f4a into master Mar 31, 2023
@elia elia deleted the elia+rainer/thanks-stripe-beta-team branch March 31, 2023 10:48
@elia

elia commented Mar 31, 2023

Copy link
Copy Markdown
Member Author

I'm also wondering if we need the SolidusStripe::PaymentIntent model at all. It's not needed from the webhook handlers, as the response_code is filled on payment intent creation. Can you help me understand? Is it only needed in the case a user refreshes the confirm page?

@waiting-for-dev sorry, I merged before replying to this question. And yes, it's there to ensure we have a single payment intent for each order. As you pointed out there might be indirect ways to do the same through Payment#response_code but as we discussed when we introduced the class this way it should be more reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

4 participants