Re: [w3c/payment-request] Add hasEnrolledInstrument() (#833)

marcoscaceres requested changes on this pull request.

Hi @danyao, this is a great start - and the algorithm parts are looking pretty solid... 

However, I'd like to do this in two parts because there is some duplication in the two algorithms that I'd like to avoid (mostly an editorial/maintenance issue). Also note that the `hasEnrolledInstrument()` doesn't seem to integrate with the state machine like the other methods.

For example, right now:

```
pr = new PaymentRequest("")
await pr.abort();
await pr.canMakePayment(); //throws
await pr.hasEnrolledInstrument(); // doesn't throw.
``` 

I think we should have a single algorithm in the Algorithms sections. The algorithm should be called:

"can make payment algorithm" and have it take an argument Boolean `<var>checkForInstruments</var>` that if passed as "true", it does the instrument check.   

Then,  instead of:

```HTML
The <a>canMakePayment()</a> method MUST act as follows:
```

It would be:

```
The <a>canMakePayment()</a> method MUST run the <a>can make payment algorithm</a> with the <var>checkForInstruments</a> set to false.
```

Then,  instead of:

```HTML
The <a>hasEnrolledInstrument()</a> method MUST act as follows:
```

It would be:

```HTML
The <a>hasEnrolledInstrument()</a> method MUST run the <a>can make payment algorithm</a> with the <var>checkForInstruments</a> set to true.
```



> @@ -1300,20 +1302,71 @@ <h2>
         <h2>
           <dfn>canMakePayment()</dfn> method
         </h2>
-        <p class="note">
-          The <a>canMakePayment()</a> method can be used by the developer to
-          determine if the <a>PaymentRequest</a> object can be used to make a
-          payment, before they call <a>show()</a>. It returns a <a>Promise</a>
-          that will be fulfilled with true if the <a>user agent</a> supports
-          any of the desired <a>payment methods</a> supplied to the
-          <a>PaymentRequest</a> constructor, and false if none are supported.
-          If the method is called too often, the user agent might instead
-          return <a>a promise rejected with</a> a "<a>NotAllowedError</a>"
-          <a>DOMException</a>, at its discretion.
-        </p>
+        <div class="note">

```suggestion
        <div class="note" title="canMakePayment() vs hasEnrolledInstrument()">
```

> @@ -1300,20 +1302,71 @@ <h2>
         <h2>
           <dfn>canMakePayment()</dfn> method
         </h2>
-        <p class="note">
-          The <a>canMakePayment()</a> method can be used by the developer to
-          determine if the <a>PaymentRequest</a> object can be used to make a
-          payment, before they call <a>show()</a>. It returns a <a>Promise</a>
-          that will be fulfilled with true if the <a>user agent</a> supports
-          any of the desired <a>payment methods</a> supplied to the
-          <a>PaymentRequest</a> constructor, and false if none are supported.
-          If the method is called too often, the user agent might instead
-          return <a>a promise rejected with</a> a "<a>NotAllowedError</a>"
-          <a>DOMException</a>, at its discretion.
-        </p>
+        <div class="note">
+          <p>

I realize now that the first sentence of this paragraphs is somewhat misleading, but the second part is definitely correct. I'd suggest:

```HTML
          <p>
            The <a>canMakePayment()</a> method returns a <a>Promise</a>
            that is fulfilled with true if the <a>user agent</a> supports
            any of the <a data-link-for="PaymentMethodData">supportedMethods</a> supplied to the
            <a>PaymentRequest</a> <a data-lt="PaymentRequest.PaymentRequest()">constructor</a>, or false if none are supported.
          </p>
          <p>
            A true result from <a>canMakePayment()</a> does not imply that the
            user has an provisioned instrument ready for payment. For that, use
            <a>hasEnrolledInstrument()</a> instead.
          </p>
```

Needs a tidy :) 

> +              algorithm.
+              </li>
+            </ol>
+          </li>
+          <li>Resolve <var>hasHandlerPromise</var> with false.
+          </li>
+        </ol>
+      </section>
+      <section data-dfn-for="PaymentRequest" data-link-for="PaymentRequest">
+        <h2>
+          <dfn>hasEnrolledInstrument()</dfn> method
+        </h2>
+        <p class="note">
+          The <a>hasEnrolledInstrument()</a> method can be used by the developer
+          to determine if the <a>user agent</a> not only has support for one of
+          the desired <a>payment methods</a> but is also "ready for payment", (

```suggestion
          the desired <a>payment methods</a> but is also "ready for payment" (
```

> +          </li>
+          <li>Resolve <var>hasHandlerPromise</var> with false.
+          </li>
+        </ol>
+      </section>
+      <section data-dfn-for="PaymentRequest" data-link-for="PaymentRequest">
+        <h2>
+          <dfn>hasEnrolledInstrument()</dfn> method
+        </h2>
+        <p class="note">
+          The <a>hasEnrolledInstrument()</a> method can be used by the developer
+          to determine if the <a>user agent</a> not only has support for one of
+          the desired <a>payment methods</a> but is also "ready for payment", (
+          e.g., when showing a "buy now" button). If the method is called too
+          often, the user agent may return <a>a promise rejected
+          with </a> a "<a>NotAllowedError</a>" <a>DOMException</a>, at its

```suggestion
          with </a> a "<a>NotAllowedError</a>" <a>DOMException</a> at its
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/payment-request/pull/833#pullrequestreview-201936946