Add payment canceled exception #82

stsc3000 · 2021-08-16T07:13:36Z

👋 Hi,

as outlined in #61, it would be nice to have consistent feedback when the user cancels payment authorization by closing the native payment selector.

The proposed changes end up throwing a PlatformException(code: 'paymentCanceled').
The Android implementation is straightforward, as Activity.RESULT_CANCELED makes it obvious when the user canceled.

The iOS implementation is more involved, as it requires a more detailed tracking via delegate methods to make sure that the user canceled voluntarily, i.e. user canceled if:

native payment selector was shown (paymentAuthorizationControllerWillAuthorizePayment)
user does not try to authorize a payment (didAuthorizePayment)
native payment selector is closed (paymentAuthorizationControllerDidFinish)

The implementation also fixes #77.

google-cla · 2021-08-16T07:13:43Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

stsc3000 · 2021-08-17T04:40:26Z

@googlebot I signed it!

tomarra · 2021-08-24T13:34:48Z

@stsc3000 @JlUgia what other steps need to be followed to get this reviewed and merged? This fix would really help me out with a client project :)

stsc3000 · 2021-08-28T07:10:43Z

@tomarra @JlUgia I'm unsure whether there is something left for me to do to proceed on this issue. I'll be afk for about a week, but feel free to proceed in the meantime.

jamesblasco

@JlUgia I think this PR is a very useful and demanded feature. Could you take a quick look into it?

pay_android/android/src/main/kotlin/io/flutter/plugins/pay_android/GooglePayHandler.kt

Co-authored-by: Jaime Blasco <jaimeblasco97@gmail.com>

stsc3000 · 2021-09-07T05:18:58Z

@jamesblasco thx! I fixed the error message.

xmany · 2021-09-15T00:19:13Z

Huge thanks for the commits! however I just tested the update on Android, it seems it won't throw the exception when payment sheet is dragged down and dismissed, the function did not return either, any idea?

stsc3000 · 2021-09-16T06:11:59Z

@xmany Thanks for trying it out!
I just tried to drag down the payment sheet on my device (Pixel 4a) and it appears that this is not an option. The only ways to cancel appear the X button and pressing the back button / gesture. I'm not sure what determines the Google Pay UI and which variants there are. Does it work when cancelling in a different way? Do you mind sharing your device and android version? I'll look into the native code anyway.

xmany · 2021-09-16T07:06:44Z

@xmany Thanks for trying it out!
I just tried to drag down the payment sheet on my device (Pixel 4a) and it appears that this is not an option. The only ways to cancel appear the X button and pressing the back button / gesture. I'm not sure what determines the Google Pay UI and which variants there are. Does it work when cancelling in a different way? Do you mind sharing your device and android version? I'll look into the native code anyway.

Thanks! My device is Samsung S20 plus, Android 11

xmany · 2021-09-16T09:20:49Z

It might have something to do with debug/release mode (Flutter), on iOS, when I was in debug mode, the fix was working, in release mode it was not.
iPhone 11, iOS 14.2

Haven't test the debug/release difference on Android, will test soon. However the above issue about not working on Android happened in release mode.

stsc3000 · 2021-09-16T09:43:27Z

@xmany I think the Android GooglePayHandler handles all possible return cases from the native call (PaymentHandler conforms to AutoResolveHelper results), so I don't know why or how it should handle differently on your device.

Please share the results of your tests, I am not aware of issues with release mode.

xmany · 2021-09-19T05:57:31Z

@xmany I think the Android GooglePayHandler handles all possible return cases from the native call (PaymentHandler conforms to AutoResolveHelper results), so I don't know why or how it should handle differently on your device.

Please share the results of your tests, I am not aware of issues with release mode.

You are right, After cleaning up and referencing the repo locally, the commits you had worked correctly, it's my environment a bit messed up,

Sorry

tomarra · 2021-09-20T13:42:37Z

@jamesblasco @JlUgia still looking to get this in as it's blocking the release of a project for me. This has been opened for over a month now. Can this be merged and released?

JlUgia · 2021-09-20T15:11:02Z

Thank you everyone.
Let's aim to get this merged this week at the latest, and add it to the next minor release.

JlUgia · 2021-09-24T09:39:46Z

pay_ios/ios/Classes/PaymentHandler.swift

@@ -33,7 +38,7 @@ typealias PaymentCompletionHandler = (Bool) -> Void
 class PaymentHandler: NSObject {

  /// Holds the current status of the payment process.
-  var paymentStatus = PKPaymentAuthorizationStatus.failure
+  var paymentHandlerStatus: PaymentHandlerStatus = .started


I'd suggest to use implicit unwrapping (PaymentHandlerStatus!) instead of initializing to an arbitrary value. Now that we are using startPayment to set the status to .started we can treat the declaration as a late initialization instead.

JlUgia · 2021-09-24T09:41:36Z

pay_ios/ios/Classes/PaymentHandler.swift

-    paymentStatus = .success
-    completion(PKPaymentAuthorizationResult(status: paymentStatus, errors: nil))
+    paymentHandlerStatus = .authorized
+


I think removing this line would be useful to conceptually connect calling the completion handler and the status update.

JlUgia · 2021-09-24T10:13:56Z

Left two comments.
The other changes LGTM

stsc3000 · 2021-09-27T09:40:57Z

@JlUgia I updated the PR with your recommended changes (I hope).

JlUgia · 2021-09-28T12:27:22Z

pay_ios/ios/Classes/PaymentHandler.swift

@@ -194,8 +194,6 @@ extension PaymentHandler: PKPaymentAuthorizationControllerDelegate {
    // Return the result back to the channel
    self.paymentResult(String(decoding: paymentResultData, as: UTF8.self))

-    paymentHandlerStatus = .authorized
-


I actually meant to only remove this white line to group lines of code according to their role in the class.
I think that the .authorized state has its place there and helps with clarity.
In other words, I'd suggest we keep lines 197 and 26 as they were prior to this commit.
Apologies for the confusion.

Oh, alright. Will fix ASAP.

@JlUgia Thanks for the feedback, I "reverted" the changes.

JlUgia · 2021-09-28T12:28:29Z

Hi @stsc3000! Thank you for the commit.
I left one comment inline.

stsc3000 added 2 commits August 13, 2021 07:50

feat(pay_android): throw paymentCanceled exception

71ad1f3

feat(pay_ios): throw paymentCanceled exception and reset status

8236276

jamesblasco reviewed Aug 30, 2021

View reviewed changes

pay_android/android/src/main/kotlin/io/flutter/plugins/pay_android/GooglePayHandler.kt Outdated Show resolved Hide resolved

jamesblasco requested a review from JlUgia August 30, 2021 11:48

fix(pay_android): use consistent error message

b03891d

Co-authored-by: Jaime Blasco <jaimeblasco97@gmail.com>

xmany mentioned this pull request Sep 14, 2021

function [Pay.showPaymentSelector] does not return if I close the payment sheet #90

Closed

JlUgia reviewed Sep 24, 2021

View reviewed changes

refactor(pay_ios): Improve paymentHandlerStatus update

b55b149

stsc3000 requested a review from JlUgia September 28, 2021 11:06

JlUgia reviewed Sep 28, 2021

View reviewed changes

fix(pay_ios): Clarify authorized status

c064730

stsc3000 requested a review from JlUgia September 28, 2021 13:11

JlUgia approved these changes Oct 4, 2021

View reviewed changes

JlUgia merged commit 2a8f056 into google-pay:main Oct 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add payment canceled exception #82

Add payment canceled exception #82

stsc3000 commented Aug 16, 2021

google-cla bot commented Aug 16, 2021

stsc3000 commented Aug 17, 2021

tomarra commented Aug 24, 2021

stsc3000 commented Aug 28, 2021

jamesblasco left a comment •

edited

Loading

stsc3000 commented Sep 7, 2021

xmany commented Sep 15, 2021

stsc3000 commented Sep 16, 2021

xmany commented Sep 16, 2021

xmany commented Sep 16, 2021 •

edited

Loading

stsc3000 commented Sep 16, 2021

xmany commented Sep 19, 2021

tomarra commented Sep 20, 2021

JlUgia commented Sep 20, 2021

JlUgia Sep 24, 2021

JlUgia Sep 24, 2021

JlUgia commented Sep 24, 2021

stsc3000 commented Sep 27, 2021

JlUgia Sep 28, 2021

stsc3000 Sep 28, 2021

stsc3000 Sep 28, 2021

JlUgia commented Sep 28, 2021

Add payment canceled exception #82

Add payment canceled exception #82

Conversation

stsc3000 commented Aug 16, 2021

google-cla bot commented Aug 16, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

stsc3000 commented Aug 17, 2021

tomarra commented Aug 24, 2021

stsc3000 commented Aug 28, 2021

jamesblasco left a comment • edited Loading

Choose a reason for hiding this comment

stsc3000 commented Sep 7, 2021

xmany commented Sep 15, 2021

stsc3000 commented Sep 16, 2021

xmany commented Sep 16, 2021

xmany commented Sep 16, 2021 • edited Loading

stsc3000 commented Sep 16, 2021

xmany commented Sep 19, 2021

tomarra commented Sep 20, 2021

JlUgia commented Sep 20, 2021

JlUgia Sep 24, 2021

Choose a reason for hiding this comment

JlUgia Sep 24, 2021

Choose a reason for hiding this comment

JlUgia commented Sep 24, 2021

stsc3000 commented Sep 27, 2021

JlUgia Sep 28, 2021

Choose a reason for hiding this comment

stsc3000 Sep 28, 2021

Choose a reason for hiding this comment

stsc3000 Sep 28, 2021

Choose a reason for hiding this comment

JlUgia commented Sep 28, 2021

jamesblasco left a comment •

edited

Loading

xmany commented Sep 16, 2021 •

edited

Loading