Problem/Motivation

Commerce implemented a concept into orders recording the total amount paid and the remaining amount which has to be paid, called balance. #2804227-6: Add getTotalPaid() and getBalance() methods to orders:

The order tracks its paid_total. getBalance() then subtracts the paid_total from the total.

This concept has not yet been implemented into commerce_stripe and is needed to solve issues like #2935337-4: Native support for deposits/later payment of balance.

Proposed resolution

Do the house keeping mentioned by mglaman in #2935337-7: Native support for deposits/later payment of balance by implementing the use of those methods, especially of getBalance() when creating and updating payment intents and payments, instead of getTotalPrice().

CommentFileSizeAuthor
#13 commerce_stripe-implement-getbalance-3100093-13.patch4.25 KBmjb3141
#9 interdiff-7-9.txt844 bytesyanniboi
#9 commerce_stripe-implement-getbalance-3100093-9.patch4.19 KByanniboi
#7 interdiff-2-7.txt2.02 KByanniboi
#7 commerce_stripe-implement-getbalance-3100093-7.patch4.18 KByanniboi
#5 3100093-5-ensure-payment-matches.patch2.89 KBandrewbelcher
#4 INTERDIFF.txt747 bytesandrewbelcher
#4 3100093-4-ensure-payment-matches.patch5.13 KBandrewbelcher
#2 commerce_stripe-implement-getbalance-3100093-2.patch2.91 KBmvonfrie

Issue fork commerce_stripe-3100093

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mvonfrie created an issue. See original summary.

mvonfrie’s picture

mvonfrie commented
Status: Active » Needs review
StatusFileSize
new commerce_stripe-implement-getbalance-3100093-2.patch2.91 KB
mvonfrie’s picture

mvonfrie commented

The tests of #2 are failing because they are missing the patch #3100709-3: Update tests for Commerce 2.15.

andrewbelcher’s picture

andrewbelcher commented
StatusFileSize
new 3100093-4-ensure-payment-matches.patch5.13 KB
new INTERDIFF.txt747 bytes

The patch in 2 works, but leaves one specific scenario not covered, which is that the payment passed in may not be for the balance of the order (see #3086504: Allow the PraymentProcess pane to process an amount other than the balance - I take responsibility for that being a scenario!).

The danger of this is that the payment recorded in Drupal can be for a different amount to the payment recorded in Stripe, with both showing as completed. That is not a good state to be in.

So I have added an update to the intent that happens immediately before processing the payment. That should ensure the two are always in sync.

andrewbelcher’s picture

andrewbelcher commented
StatusFileSize
new 3100093-5-ensure-payment-matches.patch2.89 KB

My previous patch got a weird diff, this is it correctly formed!

Status: Needs review » Needs work

The last submitted patch, 5: 3100093-5-ensure-payment-matches.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

yanniboi’s picture

yanniboi commented
StatusFileSize
new commerce_stripe-implement-getbalance-3100093-7.patch4.18 KB
new interdiff-2-7.txt2.02 KB

After some discussion with @andrewbelcher we decided that his approach was not quite right as by the time \Drupal\commerce_stripe\Plugin\Commerce\PaymentGateway\Stripe::createPayment is executed, the payment intent has often already been confirmed in Stripe and should not be updated with a new price.

Instead, the amount of the Drupal Payment entity created should definitely match the amount of the payment made in Stripe, so for the scenario that the Intent amount is different to the Payment amount, the payment should be updated with the amount stored in Stripe. So this is what my patch adds to #2.

yanniboi’s picture

yanniboi commented
Status: Needs work » Needs review
yanniboi’s picture

yanniboi commented
StatusFileSize
new commerce_stripe-implement-getbalance-3100093-9.patch4.19 KB
new interdiff-7-9.txt844 bytes

Sorry, I forgot that currency code's are case sensitive...

MrDaleSmith made their first commit to this issue’s fork.

MrDaleSmith opened merge request !25

johnpitcairn’s picture

johnpitcairn commented
Assigned: mvonfrie » Unassigned
mjb3141’s picture

mjb3141 commented
StatusFileSize
new commerce_stripe-implement-getbalance-3100093-13.patch4.25 KB

Re-rolling patch for head.

Status: Needs review » Needs work

The last submitted patch, 13: commerce_stripe-implement-getbalance-3100093-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tomtech’s picture

tomtech commented
Title: Implement use of getTotalPaid() and getBalance() methods » Use getBalance() rather than getTotalPaid() method
Assigned: Unassigned » tomtech

tomtech opened merge request !147

  • tomtech committed 8f74aac0 on 8.x-1.x 
    Issue #3100093 by yanniboi, andrewbelcher, mrdalesmith, tomtech,...
tomtech’s picture

tomtech commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.