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

Define payment option filtering algorithm. #420

Closed
wants to merge 1 commit into from

Conversation

rsolomakhin
Copy link
Collaborator

Define the algorithm for filtering the payment options in the payment
app. These payment options are used to invoke the payment app that the
user selects.

Define the algorithm for filtering the payment options in the payment
app. These payment options are used to invoke the payment app that the
user selects.
@rsolomakhin
Copy link
Collaborator Author

@adamroach: This is essentially your algorithm, but without the addition of the new filters member. Does it fairly represent your idea?

@domenic: This pull request is especially tricky because it intends to interact with the payment app spec. Please take a look.

Copy link
Contributor

@adamroach adamroach left a comment

Choose a reason for hiding this comment

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

Thanks! The text looks mostly good, but I think you cut a bit too deep. I'd also like to get on the same page about how to indicate filters in the PaymentRequest object.

<var>request</var>.<a>[[\serializedMethodData]]</a>:
<ol type="I">
<li>Let <var>filters</var> be <var>requestMethod.data</var>
members whose values are lists of strings.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm. I thought, after our last discussion on this point, we had reached agreement that this was problematic, in that it precluded payment methods from defining data parameters whose values were a list of strings, but which were not filters.

<ol type="i">
<li>Let <var>optionMatch</var> be true.</li>
<li>For <var>filterName</var> in the keys of
<var>requestMethod</var>:
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be in the keys of filters rather than keys of requestMethod?

@@ -628,30 +628,21 @@
<li>Return <var>acceptPromise</var> and perform the remaining steps
<a>in parallel</a>.
</li>
<li>For each <var>paymentMethod</var> in
Copy link
Contributor

Choose a reason for hiding this comment

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

You're removing the part of the current document that deals with paymentMethod matching. The algorithm I thumbnailed in issue 96 was intended to supplement paymentMethod matching rather than replace it. Basically, you need to put the filter payments options algorithm into a loop that evaluates it once for each paymentMethod in the request, and only those options that match the paymentMethod should be evaluated on each iteration.

Copy link
Collaborator

@domenic domenic left a comment

Choose a reason for hiding this comment

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

I'm rather confused, so I tried to ask clarifying questions...

<li>If this consultation produced no supported method of paying, then
reject <var>acceptPromise</var> with a "<a>NotSupportedError</a>" <a>
DOMException</a>, and abort this algorithm.
<li>Let <var>matchingOptions</var> be the result of running the
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a little unclear to me what the result of this algorithm is, i.e. what a "matching option" is. The algorithm steps seem to only add option to the return value, and option is a payment app. So is matchingOptions a list of payment apps? But then the subsequent sentence

show a user interface to allow the user to interact with the payment request process, using those payment apps, matchingOptions, and payment methods which the above step identified as feasible

is confusing, since the above step only identified payment apps = matchingOptions, whereas this seems to be saying that there are three different lists determined by the above step: the payment apps, matchingOptions, and payment methods.

<li>For each <var>requestMethod</var> in
<var>request</var>.<a>[[\serializedMethodData]]</a>:
<ol type="I">
<li>Let <var>filters</var> be <var>requestMethod.data</var>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This step doesn't make sense to me. requestMethod is a tuple containing supportedMethods, a sequence<DOMString>, and data, a string of serialized JSON. So requestMethod.data does not have any members.

<li>For <var>filterName</var> in the keys of
<var>requestMethod</var>:
<ol type="a">
<li>If <var>option.capabilities</var> does not have key
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is option.capabilities defined? I thought a payment app was an abstract concept, not an object which had properties.

@ianbjacobs
Copy link
Collaborator

We discussed the proposal today at the WPWG call:
https://www.w3.org/2017/02/23-wpwg-minutes#item02

Rouslan is planning to make some updates regarding (1) explicit identification of filters with (2) limited backwards compatibility for managing the way basic card has been implemented up to now. We also want to check with MS on the proposal.

@zkoch
Copy link
Contributor

zkoch commented Mar 15, 2017

I think we've decided on calls that this particular direction is not what the group wants, so going to go ahead and close this PR.

@zkoch zkoch closed this Mar 15, 2017
@adrianhopebailie
Copy link
Collaborator

adrianhopebailie commented Mar 16, 2017

@zkoch for completeness, can you link to the minutes where this decision was made as I think this may be premature (unless there is another issue tracking this?)

@ianbjacobs
Copy link
Collaborator

Hi @adrianhopebailie,

Here are the most recent minutes:
https://www.w3.org/2017/03/09-wpwg-minutes#item02

In the payment handler spec we will define a way for payment apps to provide capabilities to the user agent.

My proposal was first to ensure that PR API explicitly takes those payment app capabilities into account in the matching algorithm(s). The spec has been edited to say that.

The specification does not provide details about how that is done (e.g., by string set matching). We can make suggestions in the payment app spec.

Ian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants