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 #129 #153

Closed
wants to merge 8 commits into from
Closed

Fix #129 #153

wants to merge 8 commits into from

Conversation

adrianhopebailie
Copy link
Collaborator

Addresses #129 by dropping the algorithm step that passes a value to the payment app.
As the boolean passed to complete is not used for anything else it has been dropped too.

@adrianba
Copy link
Contributor

This breaks WICG/paymentrequest#23 and should not be merged.

@dlongley
Copy link

https://github.com/WICG/paymentrequest/issues/23 may be payment method specific -- or could be implemented that way. An Apple Pay payment method spec could indicate that the merchant needs to provide an endpoint that the Payment App can hit to ask the merchant if they should complete the payment.

@adrianhopebailie
Copy link
Collaborator Author

#129 suggests that https://github.com/WICG/paymentrequest/issues/23 is not actually supported anyway unless the payment app is built into the browser.

I think @dlongley is correct and the only way to cater for the requirements laid out in https://github.com/WICG/paymentrequest/issues/23 would be some out of band channel like a callback URL

@zkoch
Copy link
Contributor

zkoch commented Apr 23, 2016

Agreed with @adrianba here, that this shouldn't be merged without some sort of replacement to pass in to complete() for now. That said, I'm happy with the removal of the step that says it gets passed back to payment app.

@adrianhopebailie
Copy link
Collaborator Author

Should we drop that step without having some purpose for the parameter? Makes the spec seem a bit confused to me.

If you can give me some guidance on what the purpose of the parameter is I'll update the PR to replace the step that passes the value to the Payment App with something more relevant.

@zkoch
Copy link
Contributor

zkoch commented Apr 25, 2016

It was originally added in as a response to https://github.com/WICG/paymentrequest/issues/23. That still seems like a valid use case to me, so I would not support dropping it. It also provides a nice, explicit mechanism to tell the user agent to close down the UI. The value of complete gives the ability to do some sort of "success" or "fail" UI before the closing (or no UI at all, depends on the merchant).

@adrianhopebailie
Copy link
Collaborator Author

It was originally added in as a response to WICG/paymentrequest#23.

Yes, but that is clearly not supported anymore or is it?

It also provides a nice, explicit mechanism to tell the user agent to close down the UI. The value of complete gives the ability to do some sort of "success" or "fail" UI before the closing (or no UI at all, depends on the merchant).

So should a change the text to indicate that the value is meant to be consumed by the User Agent to provide a hint when it pulls down the UI that the payment succeeded or failed?

In the absence of a concrete counter-proposal I'm struggling to deduce what you think this new spec text should say.

@hober
Copy link
Member

hober commented Apr 26, 2016

Please don't break PassKit compatibility.

@adrianhopebailie
Copy link
Collaborator Author

Please don't break PassKit compatibility.

There is nothing to break right now. This part of the spec makes no sense because it talks about passing data to a payment app at a stage when we expect there to be no open channel between the payment app and the browser.

Any value passed to complete() only goes as far as the browser.

@adrianhopebailie
Copy link
Collaborator Author

Counter-proposal in PR #161

@adrianba
Copy link
Contributor

PR #161 was adopted on the telcon instead of this proposal.

@adrianba adrianba closed this Apr 28, 2016
@hober
Copy link
Member

hober commented Apr 28, 2016

I think maybe we're talking past each other? In order to implement this API on top of PassKit, you need a complete() function and the caller minimally needs to be able to indicate success or failure.

@mattsaxon
Copy link
Contributor

@hober as this PR has not been accepted and I'm not sure if your concerns have been addressed, I wonder if you'd consider submitting a sequence diagram to show this issue.

You can see the ApplePay flow represented here http://www.plantuml.com/plantuml/proxy?fmt=svg&src=https://raw.githubusercontent.com/w3c/webpayments-flows/gh-pages/PaymentFlows/ApplePay/ApplePay-Native-Current.pml

It would be good to represent this correctly if it it not, and to show how the flow will work under the API as has been done in the Basic Card Payment spec.

If you're willing I can assist.

The basic flow for the new API is here https://raw.githubusercontent.com/w3c/webpayments-flows/gh-pages/TargetFlows/PaymentRequestAPI.pml

@nickjshearer
Copy link

@mattsaxon - I think what @hober is getting at is this statement:

Any value passed to complete() only goes as far as the browser.

This is going to significantly degrade the experience for some payment instruments. I actually think some payment apps need to have the completion value passed back to it. An example is UnionPay, where transactions may require a PIN. This PIN (in encrypted form) is passed from the payment app via the user agent and payment processor to the issuing bank, where it's validated. If it's incorrect, the payment processor is instead of providing a "Success" or "Failure" going to return an "Incorrect PIN" status.

Ideally, the payment app is still visible at this point and can simply prompt the user to try again. But under the modified PR #161 that was taken, the payment app never gets the result of this. So the user has to go through the entire payment flow again, selecting the China UnionPay app, entering the PIN, etc. It's a bad user experience.

Now, I can see that some payment apps won't need to know or care about the completion value. But some do, and that should be accounted for.

@hober
Copy link
Member

hober commented May 2, 2016

Sorry, I was replying to @adrianhopebailie, who seemed to want to remove the parameter to complete() earlier in the thread. The reply just got in after @adrianba closed the issue. AFAICT the change in #161 is fine.

@adrianhopebailie
Copy link
Collaborator Author

@hober and @nickjshearer, there is no guarantee that the browser will be able to pass any data to a payment app after the payment response has been received by the website. On some platforms this may be impossible and on others result in a terrible user experience.

complete() is just an instruction to the browser to tear down the payment app UI (or any other UI that is still visible) as the website is now taking focus.

There are only two ways we can guarantee that a more complex interaction is possible between website and payment app:

  1. The payment app is built into the browser so we can guarantee that the payment app is able to interact directly with the website.
  2. The payment processing is done BEFORE the response is returned to the browser through some out of band channel.

To expand on option 2 and use @nickjshearer 's example:

This PIN (in encrypted form) is passed from the payment app via the user agent and payment processor to the issuing bank, where it's validated. If it's incorrect, the payment processor is instead of providing a "Success" or "Failure" going to return an "Incorrect PIN" status.

It would be better (and arguably more secure) if the payment request contained a callback URL that the payment app used to submit the PIN. This would allow the payment app to complete the entire payment processing flow and only return focus to the browser when it has confirmed the payment.

This was raised in #109 where there is a flow diagram that explains the concept in more detail.

Since this callback requirement is not shared by all payment methods it's probably best to leave it to be defined in the payment method specific data for now and standardize if we feel it's required later.

@mattsaxon
Copy link
Contributor

@nickjshearer

This is going to significantly degrade the experience for some payment instruments. I actually think some payment apps need to have the completion value passed back to it.

I agree with you, though I though the group consensus (which I disagree with) was that this sort of interaction would be done out of band.

An example is UnionPay, where transactions may require a PIN. This PIN (in encrypted form) is passed from the payment app via the user agent and payment processor to the issuing bank, where it's validated. If it's incorrect, the payment processor is instead of providing a "Success" or "Failure" going to return an "Incorrect PIN" status.

In the agreed model, the PIN validation would occur out of band from within the PaymentApp with the success of the authorisation being passed back in the PaymentResponse. Any retries would be done in the app. We do need a method to passback failure though.

Note: I dislike this model and would prefer a conversational approach to the API as you describe.

@adrianhopebailie adrianhopebailie deleted the complete branch May 31, 2016 07:33
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.

7 participants