Reject acceptedPromise if converting method data fails #536

marcoscaceres · 2017-05-25T04:24:12Z

Align with w3c/payment-method-basic-card#23

domenic · 2017-05-25T04:27:03Z

index.html

+                  <li>If required by the specification that defines the
+                  <var>handler</var>, <a data-cite=
+                  "!WEBIDL#dfn-convert-ecmascript-to-idl-value">convert</a>
+                  <var>data</var> to an IDL value. Rethrow any exceptions.


You can't throw an exception from in parallel.

It seems unlikely we want to error if the user installs a payment handler which doesn't accept the provided data. Maybe if none of their installed handlers do... but that's already covered by the next step.

You can't throw an exception from in parallel.

Ah yeah, will reject acceptPromise.

It seems unlikely we want to error if the user installs a payment handler which doesn't accept the provided data.

Sure... but...

Maybe if none of their installed handlers do... but that's already covered by the next step.

Ok, but something bad needs to happen with invalid data? I'm confused about where that should go.

Sure, if the data is invalid for all payment apps, then the next step (which already exists) will fail. But we need to give every payment app a chance. If one succeeds, but 3 fail, it's not invalid data.

Ok, got you. So we just ignore bad conversions as "unsupported". I guess as long as we notify the developer of bad conversions in the error console, that should be ok (or else it's gonna be hard to debug).

marcoscaceres · 2017-05-25T04:54:25Z

To be clear, trying to solve for:

const methodData = [{
  supportedMethods: ["basic-card", "https://foo.com/pay"],
  data: { supportedTypes: ['THIS IS NOT A VALID ENUM VALUE!!'] }
}];
const request = new PaymentRequest(methodData, details, options);
try {
 // When converting data, should `supportedTypes` cause `acceptPromise` to reject?
 const response = await request.show();
} catch(err) {

}

domenic · 2017-05-25T05:02:59Z

That is already rejected before this PR, since no payment handlers support that data, so the next step takes care of rejecting the promise.

What this PR starts rejecting is:

const methodData = [{
  supportedMethods: ["basic-card", "bitcoin"],
  data: { supportedTypes: "credit" }
};

because "bitcoin" doesn't support "credit".

Is that intentional? It seems wrong to me; instead it makes more sense to me to do what the spec currently says, and display UI for paying with basic-card/credit, and ignore the bitcoin supported method.

EDIT: switched from "visa" to "credit" (the latter is actually a valid supportedType for basic-card).

marcoscaceres · 2017-05-25T05:09:08Z

this still feels super messed up to me. You could potentially end up with completely incompatible data types and property name clashes.

const methodData = [{
  supportedMethods: ["basic-card", "bitcoin"],
  data: { supportedTypes: "bitcoin-specific" }
};

domenic · 2017-05-25T05:13:30Z

Yeah, definitely. I don't understand why supportedMethods is an array at all; it seems strange to think that the same data would make sense for multiple methods. Usually it doesn't, I assume, so that's why it doesn't make sense to me to error on mismatch, because it seems like any time you have 2+ supportedMethods, you're always going to get at least one mismatch.

In other words, given that we have supportedMethods being an array, the current spec seems better than requiring data to work for all supportedMethods.

marcoscaceres · 2017-05-25T05:24:29Z

Usually it doesn't, I assume, so that's why it doesn't make sense to me to error on mismatch, because it seems like any time you have 2+ supportedMethods, you're always going to get at least one mismatch.

I can only assume this was done with the assumption that .data would be the same type for the array of supportedMethods - even if potentially false.

Ideally, you then want:

const methodData = [{
  supportedMethods: ["basic-card", "https://bank.com/basic-card"],
  data: { supportedTypes: "visa" }
}, {
  supportedMethods: ["crypto-currency", "https://bit.com/coin"],
  data: { cryptoCurrencyThing: "thing" }
}];

So you don't mix types. If you mix data types, this is going to all come crashing down.

marcoscaceres · 2017-05-25T05:44:43Z

@domenic, the more I think I about it, the more I think we should reject acceptPromise if there is a .data type mismatch during conversion.

I can't think of any valid use case for mixing .data types.

marcoscaceres · 2017-05-25T06:06:19Z

For those watching at home, the following would be all fine:

const methodData = [{
  // Must match "basic-card" spec, undergoes conversion
  supportedMethods: ["https://bank.com/any-type", "basic-card"],
  // .data MUST BE `BasicCardRequest`, because "basic-card" present. 
  data: { supportedTypes: ["visa"] } 
}, {
  // Must match "tokenized-thing" spec, undergoes conversion
  supportedMethods: ["tokenized-thing", "https://bit.com/coin", "https://bank.com/any-type"],
  data: { cryptoCurrencyThing: "thing" }
}, {
  // .data treated as a normal object, no dictionary conversion. 
  supportedMethods: ["https://foo.com/bar", "https://bank.com/any-type"],
  data: { foo: "bar", "thumb": "drive", anything: "goes" }
}];

But:

const methodData = [
  // This would cause a rejection, iff "future-card" is supported:
  {
    supportedMethods: ["https://foo.com/future-card", "future-card"],
    data: { supportedTypes: ['THIS IS NOT A VALID VALUE!!'] }
  }, 
  // iff "future-card" is not supported, this one is still ok:
  {
    supportedMethods: ["https://foo.com/tokenized-thing", "tokenized-thing"],
    data: {   tokensAreFancy: true   }
  },
  {
   // This also throws
    supportedMethods: ["basic-card", "nonbasic-card"],
    data: { supportedTypes: "credit" }
  } 
]

adrianhopebailie · 2017-05-25T09:16:18Z

@marcoscaceres and @domenic this was discussed at some length on a WG call a few weeks ago and has been a niggling issue since pretty much the first payment method spec was written.

My understanding from @rsolomakhin and @zkoch was that the data is treated as an opaque object and no validation is done against the IDL in the payment method spec.

The browser passes the opaque data onto the payment handler which then parses it and uses it. If it is invalid then the response from the handler will indicate this.

The basic-card payment method is "special" because the handler is internal to the browser but the implementation of how the data is validated shouldn't be exposed in the Payment Request spec.

Before you say: "But we need to look at the filters like supportedTypes before the data is passed to the handler". I know.

In an ideal world each PaymentMethodData would have:

A set of identifiers it applies to
A blob of opaque data that the browser ignores and simply passes to the handler
A set of filters used to filter handlers based on their capabilities

The proposal to add a new member filters was rejected on the basis that Chrome have already deployed this and moving supportedTypes from data to filters is too disruptive.

Another way to solve this is that we are revisiting the idea of handlers defining a function which is presented with the opaque data and can respond true/false if they will be able to handle the payment.

marcoscaceres · 2017-05-25T10:08:23Z

On 25 May 2017, at 7:16 pm, Adrian Hope-Bailie ***@***.***> wrote: @marcoscaceres and @domenic this was discussed at some length on a WG call a few weeks ago and has been a niggling issue since pretty much the first payment method spec was written. My understanding from @rsolomakhin and @zkoch was that the data is treated as an opaque object and no validation is done against the IDL in the payment method spec.

This is not correct. Please see: w3c/payment-method-basic-card#23 If we don't want IDL conversion, then why would we use WebIDL? WebIDL is not a data-structure definition language: it's a system for type coercion and moving things between c++ and js. Thus, we can't say on one hand "oh! It's Web IDL..." but then "it's an opaque blob". If we really don't want WebIDL, then we should not use it as the basis for BasicCardRequest.

The browser passes the opaque data onto the payment handler which then parses it and uses it. If it is invalid then the response from the handler will indicate this.

Again, this doesn't match what basic card spec says.

The basic-card payment method is "special" because the handler is internal to the browser but the implementation of how the data is validated shouldn't be exposed in the Payment Request spec.

I'm sorry, but again you are mistaken. It's not special and it's not treated as special. Anything can act as a "basic-card" handler (including proprietary payment methods denoted by a URL) if registered to handle that type. Also, if it's expected to be a basicCardRequest, then it must be treated as WebIDL or we should stop using WebIDL to represent BasicCardRequest.

Before you say: "But we need to look at the filters like supportedTypes before the data is passed to the handler". I know.

I don't think this is where we were going.

In an ideal world each PaymentMethodData would have: A set of identifiers it applies to A blob of opaque data that the browser ignores and simply passes to the handler

This seems like a massive security risk. Passing arbitrary data to apps that are expecting well-structured payment requests seems extremely bad. I don't think I would want to do that.

A set of filters used to filter handlers based on their capabilities

This doesn't match the spec.

The proposal to add a new member filters was rejected on the basis that Chrome have already deployed this and moving supportedTypes from data to filters is too disruptive. Another way to solve this is that we are revisiting the idea of handlers defining a function which is presented with the opaque data and can respond true/false if they will be able to handle the payment.

No, we are fine to work with the current structure: we just need to define some simple rules to avoid the insanity of arbitrary .data types (at least, that's my opinion). That's why I'm proposing having .data be converted to the PMI's data type (when not null).

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

domenic · 2017-05-25T12:16:54Z

Neither of the two cases you state are the ones I am worried about. The one I am worried about is

const methodData = [{
  supportedMethods: ["basic-card", "nonbasic-card"],
  data: { supportedTypes: "credit" }
}];

where nonbasic-card does not support the "credit" type (instead e.g. only "debit" or "giftcard").

I'd like the WG to discuss and pick one of the following:

If data applies to any of the supportedMethods, then show() works, presenting a UI for the subset of supportedMethods that accept the data. (Current spec)
Only if data applies to all of the supportedMethods does show() work; otherwise it rejects. (Your proposal in this PR)

If a position on this was previously documented please link me to it :). I don't otherwise feel comfortable changing this in an "editorial" PR.

marcoscaceres · 2017-05-25T13:24:44Z

My position is 2.

I don't otherwise feel comfortable changing this in an "editorial" PR.

I thought this was "editorial" because I thought we were just not clear in the behaviour - and because the PR from the other spec had not been merged. I didn't realize there would be disagreement on something that's clearly broken.

marcoscaceres · 2017-06-01T08:40:38Z

@domenic it's basically impossible to reliably do #541 without the assurances given here.

Can we please merge this so we can move forward?

domenic · 2017-06-01T12:38:50Z

I'm not willing to merge a normative change of this magnitude without confirmation from the WG, sorry.

ianbjacobs · 2017-06-01T12:41:43Z

Let's put it on the 8 June agenda [1]
Ian

[1] https://github.com/w3c/webpayments/wiki/Agenda-20170608

marcoscaceres · 2017-06-01T12:50:37Z

Zach understands the problem, so I'm happy for him to represent my position on the call. Otherwise please move the call to a reasonable hour so I can participate.

…

On 1 Jun 2017, at 10:41 pm, ianbjacobs ***@***.***> wrote: Let's put it on the 8 June agenda [1] Ian [1] https://github.com/w3c/webpayments/wiki/Agenda-20170608 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

domenic · 2017-06-02T05:33:03Z

I'm completely supportive of this being a working group decision - but it's absolutely unfair for you to assume that members of the WG understand the implications or even what WebIDL is (just scroll up to see what I mean!).

I agree there is a lot of confusion here that is not really related to this issue. What I want is a simple working group decision on whether we're willing to change the semantics of my example from #536 (comment) from working to not working. More accurately, please see my "I'd like the WG to discuss and pick one of the following:"

I don't think this requires the working group to figure out how bitcoin works with payment request, or gain a better understanding of Web IDL. This is a fairly narrowly-scoped question, setting aside the the attempts to several people in this thread to blow it up into a larger discussion.

marcoscaceres · 2017-06-02T05:37:41Z

I don't think this requires the working group to figure out how bitcoin works with payment request, or gain a better understanding of Web IDL. This is a fairly narrowly-scoped question, setting aside the the attempts to several people in this thread to blow it up into a larger discussion.

Agree (even if I'm guilty of the blow up). @domenic, please also make a choice.

I'll delete any comments that are not choice related.

dlongley · 2017-06-02T20:45:14Z

In the future, let's not delete comments (you are welcome to delete your own, of course), but instead create a new issue and redirect over that way.

marcoscaceres · 2017-06-03T01:30:14Z

Sure, will do.

rsolomakhin · 2017-06-05T13:31:56Z

Hey there! Sorry, I'm late to the game. Our current implementation does not throw if you use non-BasicCardRequest for "basic-card" method name. We've had a lengthy discussion about this when we implemented that behavior. The resolution was documented in w3c/payment-method-basic-card#20 (comment).

marcoscaceres · 2017-06-05T14:36:23Z

Thanks @rsolomakhin

I also said there it should throw 😭 And Microsoft didn't respond. In light of new information, we should really get the Edge folks to let us know what they are doing here too.

@patrickkettner, I'm really struggling to get an Edge version to run Web Platform tests on (it doesn't want to install with virtual box, and I'm scared of bricking a work machine with a insider build). Could you please please please help us? We have access to Suace Labs - so maybe we can run against Edge 15 somehow?

rsolomakhin · 2017-06-05T14:37:44Z

I believe @adrianba gave a 👍 on my "may throw" proposal, which is hard to notice.

domenic · 2017-06-12T18:42:47Z

index.html

+                <ol>
+                  <li>If required by the specification that defines the
+                  <var>handler</var>, <a data-cite=
+                  "!WEBIDL#dfn-convert-ecmascript-to-idl-value">convert</a>


I guess if we go with this architecture you need to JSON-parse before you convert, otherwise you're just converting it to an IDL DOMString, which is no good.

bentian · 2017-06-14T02:30:18Z

index.html

+                  <li>If required by the specification that defines the
+                  <var>handler</var>, <a data-cite=
+                  "!WEBIDL#dfn-convert-ecmascript-to-idl-value">convert</a>
+                  <var>data</var> to an IDL value. If it results in a error,


adrianhopebailie · 2017-06-15T09:52:19Z

@marcoscaceres I may be misunderstanding what you have done here but I think you have replaced an algorithm for determining which handlers are suitable to process the payment request with an algorithm that validates the payment method specific data.

We've all agreed that the latter is required but I think it should be in addition to the former not replace it.

i.e. For all accepted methods in the request, validate the data, then for all known handlers establish if they can handle one of the accepted methods. The outcome is a list of potential handlers that determine what the browser can present to the user as options to process the payment request.

This does beg the question as to whether methods with invalid data should just be discarded with a warning?

marcoscaceres · 2017-06-15T10:11:40Z

This does beg the question as to whether methods with invalid data should just be discarded with a warning?

They could be. That's an implementation detail tho. Just interested in getting the .data checked at this point so I can write more tests.

adrianhopebailie · 2017-06-15T12:33:40Z

Just interested in getting the .data checked at this point so I can write more tests.

Sure but this PR does discard some aspects of the original algorithm that are still required so I don't think it can be merged as is.

ianbjacobs · 2017-06-15T12:34:41Z

index.html

-              capabilities supplied by the payment handler match those provided
-              by <var>data</var>, the payment handler matches.
+              <li>Let <var>methods</var> be a list of <a>payment methods</a>
+              that support <var>identifiers</var>.


I don't think "payment methods support identifiers". The identifiers identify the payment methods. The payment handlers support the payment methods.

I prefer the earlier text.

Ian

Will switch it back.

@adrianhopebailie, please provide code counter examples. If you can't show actual code examples then we can't have a discussion.

adrianhopebailie · 2017-06-15T13:42:49Z

@marcoscaceres are you saying that if you delete something then in order to earn the privilege of engaging in a discussion with you on why, I must first re-write what you removed?

marcoscaceres · 2017-06-19T08:51:25Z

Blocked on #551

ianbjacobs · 2017-06-19T12:04:15Z

Just a note that we discussed on 15 June at the WPWG call:
https://www.w3.org/2017/06/15-wpwg-minutes#item01

@zkoch is going to help coordinate with @marcoscaceres a fix that addresses both topics this text intends to cover: looping through payment methods and validation.

zkoch

LGTM. I think we've now addressed both use cases: validation of data AND finding matching payment handlers based on data and identifier

marcoscaceres · 2017-06-21T23:14:39Z

@zkoch and I reworked this text in light of #551. It's ready to merge once #551 goes in.

ianbjacobs · 2017-06-22T00:45:00Z

Thanks for adding back payment handler matching!

adrianhopebailie · 2017-06-22T07:16:12Z

index.html

+              "!WEBIDL#dfn-convert-ecmascript-to-idl-value">convert</a> to
+              <a data-cite="!WEBIDL#idl-object">object</a>.
+              </li>
+              <li>If conversion results in an error, reject


Isn't the intent here to discard this tuple and continue (possibly logging an error in the process)?

i.e. Rejecting the promise and terminating the algorithm results in no matched payment handlers, even if there are multiple (identifer, data) tuples and only one has invalid data.

Is it possible to throw and not reject the promise so that the browser can log the error and continue the next iteration of the loop?

If so, I propose the following, or something similar:

<li>If conversion results in an error, discard the <var>paymentMethod</var> tuple, throw the error and process the next tuple.</li>

What does "discard" mean? That's not a well-defined operation, on tuples or anything else...

I understand Adrian to be distinguishing two possible behaviors in the face of invalid data:

Abandon that particular payment method identifier/data pair or

Abandon the entire payment request

I favor the former behavior (and think AdrianHB does).

Ian

OK. In that case I don't understand the suggestion. Discarding the tuple is a meaningless operation, and throwing the error is in direct contradiction to processing the next tuple (throwing an error terminates the algorithm, like it does in all programming languages).

@domenic The algorithm is a loop through a list of tuples. I am proposing that instead of terminating the algorithm when bad data is found in one tuple, we just discard the offending tuple that contains the bad data and continue with the next.

Again, I don't understand what "discard" means. Would your sentence mean the same thing if you said "we just continue with the next"? And what happened to "throw the error"?

@domenic please, try and understand that the majority of this WG are not experts on browser internals. We're trying our best to contribute nonetheless. Have some patience and read the whole comment.

I only made a code suggestion because the last time I tried to contribute anything @marcoscaceres refused to read it unless it had code in it.

If you re-read my original comment I asked the following question:

Is it possible to throw and not reject the promise so that the browser can log the error and continue the next iteration of the loop?
If so, I propose...

I.e. There is a parsing step happening inside a loop. Can the parsing step throw and can the logic inside the loop catch and log the error and then continue?

This is the experience I'd expect a developer to get:

They call the API with a list of 3 payment methods

They have 3 different payment handlers registered that can each handle one of these payment methods

One payment method has bad data

They are presented with only 2 payment handlers to choose to handle the payment

An error is logged to the browser console

I used the phrase "discard" because in the process of looping through this list of payment methods we are also establishing a list of payment handlers that match them. If a payment method has bad data then it is "discarded" from the set used for matching.

Sorry if that terminology is confusing, I hope you understand my intent now and can suggest a better text replacement?

To close the loop on this (apologies for this post merge):

We discussed this on yesterday's editor call. While there is certainly merit in ignoring the offending payment method and its related data blob, we've been moving in the direction of explicitly throwing errors for the purpose of developer clarity across this spec. It seems best to follow suit here.

We realize there are other options (e.g. console logging a warning), but as this is core to how PR works, an explicit throw was voted as the best option by the editors.

Per @adrianba 's comment below, I understand the editors to have decided to take a fail fast approach to this API, therefor a payment request with bad data in one of the provided payment methods will fail entirely rather than simply logging an error.

For consistency, I recommend that the payment method identifier is also validated as part of this algorithm. It should either be a recognised short-string identifier or a valid URL form identifier (per the validity rules in the PMI spec).

Also, I wonder if this sentence in the PMI spec should be revised:
"User agents MUST NOT enforce validity or well-formedness of standardized payment method identifiers. "

adrianba · 2017-06-28T22:35:51Z

LGTM - we've been moving in the direction of increasingly helping developers find issues so rejecting seems correct.

marcoscaceres · 2017-06-28T22:48:01Z

Will start on tests this week.

editorial: clarify that payment handler data is converted

6376b37

marcoscaceres requested a review from domenic May 25, 2017 04:24

domenic requested changes May 25, 2017

View reviewed changes

Reject acceptPromise, don't rethrow

dae2502

marcoscaceres mentioned this pull request May 29, 2017

Use first matching PaymentDetailsModifier #541

Closed

marcoscaceres changed the title ~~editorial: clarify that payment handler data is converted~~ Reject accpetedPromise if converting method data fails May 29, 2017

marcoscaceres mentioned this pull request Jun 2, 2017

Cater for platform-specific limitations #540

Merged

sideshowbarker changed the title ~~Reject accpetedPromise if converting method data fails~~ Reject acceptedPromise if converting method data fails Jun 2, 2017

domenic reviewed Jun 12, 2017

View reviewed changes

zkoch mentioned this pull request Jun 12, 2017

Drop array support for supported methods? #549

Closed

adrianhopebailie mentioned this pull request Jun 12, 2017

Add CanMakePaymentEvent. w3c/payment-handler#170

Merged

bentian reviewed Jun 14, 2017

View reviewed changes

marcoscaceres added 2 commits June 15, 2017 16:09

fix typos

18bb040

Refactor based on feedback

0a3a579

ianbjacobs reviewed Jun 15, 2017

View reviewed changes

marcoscaceres added the Blocked label Jun 19, 2017

Marcos Cáceres and others added 2 commits June 22, 2017 08:51

Simplification with single identifier

927c5f0

add in old text

644c14c

zkoch self-requested a review June 21, 2017 23:02

zkoch approved these changes Jun 21, 2017

View reviewed changes

only one identifier

5896b12

marcoscaceres force-pushed the conversion branch from c42f088 to 5896b12 Compare June 22, 2017 00:52

adrianhopebailie reviewed Jun 22, 2017

View reviewed changes

adrianba approved these changes Jun 28, 2017

View reviewed changes

marcoscaceres merged commit a7e1bda into gh-pages Jun 28, 2017

marcoscaceres deleted the conversion branch June 28, 2017 22:47

adrianhopebailie mentioned this pull request Jun 29, 2017

Reject acceptedPromise if payment method identifer is invalid #557

Closed

adrianhopebailie mentioned this pull request Jul 28, 2017

Matching algo doesn't handle absence of filters #532

Closed

Reject acceptedPromise if converting method data fails #536

Reject acceptedPromise if converting method data fails #536

Conversation

marcoscaceres commented May 25, 2017 • edited by pr-preview bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoscaceres commented May 25, 2017 • edited Loading

domenic commented May 25, 2017 • edited Loading

marcoscaceres commented May 25, 2017

domenic commented May 25, 2017

marcoscaceres commented May 25, 2017

marcoscaceres commented May 25, 2017 • edited Loading

marcoscaceres commented May 25, 2017 • edited Loading

adrianhopebailie commented May 25, 2017

marcoscaceres commented May 25, 2017 via email

domenic commented May 25, 2017 • edited Loading

marcoscaceres commented May 25, 2017

marcoscaceres commented Jun 1, 2017

domenic commented Jun 1, 2017

ianbjacobs commented Jun 1, 2017

marcoscaceres commented Jun 1, 2017 via email

domenic commented Jun 2, 2017

marcoscaceres commented Jun 2, 2017

dlongley commented Jun 2, 2017

marcoscaceres commented Jun 3, 2017

rsolomakhin commented Jun 5, 2017

marcoscaceres commented Jun 5, 2017

rsolomakhin commented Jun 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianhopebailie commented Jun 15, 2017

marcoscaceres commented Jun 15, 2017

adrianhopebailie commented Jun 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianhopebailie commented Jun 15, 2017

marcoscaceres commented Jun 19, 2017

ianbjacobs commented Jun 19, 2017

zkoch left a comment

Choose a reason for hiding this comment

marcoscaceres commented Jun 21, 2017

ianbjacobs commented Jun 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianhopebailie Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

domenic Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrianba commented Jun 28, 2017

marcoscaceres commented Jun 28, 2017

marcoscaceres commented May 25, 2017 •

edited by pr-preview bot

Loading

marcoscaceres commented May 25, 2017 •

edited

Loading

domenic commented May 25, 2017 •

edited

Loading

marcoscaceres commented May 25, 2017 •

edited

Loading

marcoscaceres commented May 25, 2017 •

edited

Loading

domenic commented May 25, 2017 •

edited

Loading

adrianhopebailie Jun 22, 2017 •

edited

Loading

domenic Jun 22, 2017 •

edited

Loading