-
Notifications
You must be signed in to change notification settings - Fork 135
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
feat: put request id in details dictionary #426
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
94a4396
put request id in details dictionary
rvm4 9095b7f
Lowercase d in Id, as per platform conventions
32b698f
Add paymentRequestId back in as "id"
0b8352e
Feat: Add PaymentResponse.requestId
3efc14e
fix: id is not nullable
6456c61
rename paymentRequestId to just id
a475df5
fix: use id instead of paymentRequestId
9d3e59f
update text to match discussion
marcoscaceres d46f8f7
Set the response id attribute
marcoscaceres 009f63a
use [[requestId]] internal slot instead
marcoscaceres aacea2d
Editorial: improve custom details.id usage example
marcoscaceres ef429cb
editorial: make it clear we are adding an .id member
marcoscaceres 8d79319
fix rebase issue
marcoscaceres 113fb60
transplant id to PaymentDetailsInit
marcoscaceres File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,7 +202,7 @@ <h2> | |
Promise<void> abort(); | ||
Promise<boolean> canMakePayment(); | ||
|
||
readonly attribute DOMString? paymentRequestId; | ||
readonly attribute DOMString id; | ||
readonly attribute PaymentAddress? shippingAddress; | ||
readonly attribute DOMString? shippingOption; | ||
readonly attribute PaymentShippingType? shippingType; | ||
|
@@ -299,6 +299,7 @@ <h2> | |
</p> | ||
<pre class="example" title="The 'details' argument"> | ||
const details = { | ||
id: "super-store-order-123-12312", | ||
displayItems: [ | ||
{ | ||
label: "Sub-total", | ||
|
@@ -343,6 +344,17 @@ <h2> | |
</li> | ||
<li>Let <var>serializedMethodData</var> be an empty list. | ||
</li> | ||
<li>Establish the request's id: | ||
<ol> | ||
<li>If <var>details</var>.<a data-lt="PaymentDetailsInit.id">id</a> | ||
is missing, add an <a data-lt="PaymentDetailsInit.id">id</a> member | ||
to <var>details</var> and set its value to string that uniquely | ||
identifies this payment request. It is RECOMMENDED that the | ||
string be a <abbr title= | ||
"Universally Unique Identifier">UUID</abbr> [[!RFC4122]]. | ||
</li> | ||
</ol> | ||
</li> | ||
<li>Process payment methods: | ||
<ol data-link-for="PaymentMethodData"> | ||
<li>If the length of the <var>methodData</var> sequence is zero, | ||
|
@@ -581,6 +593,16 @@ <h2> | |
</li> | ||
</ol> | ||
</section> | ||
<section data-dfn-for="PaymentRequest" data-link-for="PaymentRequest"> | ||
<h2> | ||
<dfn>id</dfn> attribute | ||
</h2> | ||
<p> | ||
When getting, the <a>id</a> attribute returns this | ||
<a>PaymentRequest</a>'s <a>[[\details]]</a>.<a data-lt= | ||
"PaymentDetailsInit.id">id</a>. | ||
</p> | ||
</section> | ||
<section data-dfn-for="PaymentRequest" data-link-for="PaymentRequest"> | ||
<h2> | ||
<dfn>show()</dfn> method | ||
|
@@ -1117,6 +1139,7 @@ <h2> | |
</h2> | ||
<pre class="idl"> | ||
dictionary PaymentDetailsInit : PaymentDetailsBase { | ||
DOMString id; | ||
required PaymentItem total; | ||
}; | ||
</pre> | ||
|
@@ -1130,6 +1153,17 @@ <h2> | |
<a>PaymentDetailsInit</a> dictionary: | ||
</p> | ||
<dl> | ||
<dt> | ||
<dfn>id</dfn> | ||
</dt> | ||
<dd> | ||
<a>id</a> is a free-form identifier for this payment request. | ||
<p class="note"> | ||
If an <a>id</a> member is not present, then the <a>user agent</a> | ||
will generate a unique identifier for the payment request during | ||
<a data-lt="PaymentRequest.PaymentRequest()">construction</a>. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ReSpec will link this soon... |
||
</p> | ||
</dd> | ||
<dt> | ||
<dfn>total</dfn> | ||
</dt> | ||
|
@@ -1615,7 +1649,7 @@ <h2> | |
interface PaymentResponse { | ||
serializer = { attribute }; | ||
|
||
readonly attribute DOMString paymentRequestId; | ||
readonly attribute DOMString requestId; | ||
readonly attribute DOMString methodName; | ||
readonly attribute object details; | ||
readonly attribute PaymentAddress? shippingAddress; | ||
|
@@ -1640,13 +1674,6 @@ <h2> | |
"WEBIDL-LS#dfn-convert-to-serialized-value">converted to serialized | ||
values</a> as per [[!WEBIDL-LS]]. | ||
</dd> | ||
<dt> | ||
<dfn>paymentRequestId</dfn> | ||
</dt> | ||
<dd> | ||
The same <a>paymentRequestId</a> present in the original | ||
<a>PaymentRequest</a>. | ||
</dd> | ||
<dt> | ||
<dfn>methodName</dfn> | ||
</dt> | ||
|
@@ -1718,6 +1745,13 @@ <h2> | |
"PaymentResponse.payerPhone">payerPhone</a> will be the phone number | ||
chosen by the user. | ||
</dd> | ||
<dt> | ||
<dfn>requestId</dfn> | ||
</dt> | ||
<dd> | ||
The corresponding payment request <a data-lt= | ||
"PaymentRequest.id">id</a> that spawned this payment response. | ||
</dd> | ||
</dl> | ||
<section data-dfn-for="PaymentResponse" data-link-for="PaymentResponse"> | ||
<h2> | ||
|
@@ -2327,6 +2361,11 @@ <h2> | |
</li> | ||
<li>Let <var>response</var> be a new <a>PaymentResponse</a>. | ||
</li> | ||
<li>Set the <a data-lt="PaymentResponse.requestId">requestId</a> | ||
attribute value of <var>response</var> to the value of | ||
<var>request</var>.<a>[[\details]]</a>.<a data-lt= | ||
"PaymentDetailsInit.id">id</a>. | ||
</li> | ||
<li>Set the <a data-lt="PaymentResponse.methodName">methodName</a> | ||
attribute value of <var>response</var> to the <a>payment method | ||
identifier</a> for the <a>payment method</a> that the user selected | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you delete this by accident from the interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the constructor algorithm doesn't ever set this to null - so maybe it should not be nullable. It's also unclear what happens if paymentRequestId is set to just "" (the empty string).
Should setting it to
null
be treated as a special case (i.e., treat null as undefined instead of being converted to the word "null")?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not treat
null
as a special case. It is no more special thanfalse
or0
(all of them are "things that are vaguely negative, but are not the default-triggering special value ofundefined
").