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

Currency amount #101

Closed

Conversation

adrianhopebailie
Copy link
Collaborator

Updated definition to allow for flexible currency identifiers and not
allow negative amounts

@halindrome
Copy link
Contributor

Wait.... if the item list is truly just a holder for various items, then it should permit negative amounts to allow for discounts/credits/exchanges as line items, shouldn't it?

Updated definition to allow for flexible currency identifiers and not
allow negative amounts
@adrianhopebailie
Copy link
Collaborator Author

@halindrome the group's consensus was to follow the lead of ISO20022 an not have negative amounts. The only allowed characters for the amount field are digits and a decimal marker - w3c/webpayments#57

@adrianhopebailie
Copy link
Collaborator Author

The way this is done in most payment protocol is that the "settlement effect" of the amount is determined by the context of where the amount is encountered.

One way to do this is to have separate places to put debits and credits another way is to use transaction type to imply if this is a debit or credit.

I'm inclined to support the latter as I think this would also solve #56

@halindrome
Copy link
Contributor

I understand. I think the whole item list structure is sort of broken, in that it is basically a list of the things that would appear on a receipt as opposed to information that defines a transaction. If we need a receipt itemization, then we should provide a holder for that. If we need a total for an offer, then we should have that. They should not be intermingled. But I digress.

@mattsaxon
Copy link
Contributor

@adrianhopebailie whilst we want to follow ISO20022, I think where we find exceptions, we agreed to feed this into ISO20022, I agree that for line items, we should have the ability to show -ve amounts.

@adrianhopebailie
Copy link
Collaborator Author

@mattsaxon, @halindrome this is not directly related to line items. It's the definition of how we represent currencies and amounts (effectively the CurrencyAmount datatype that we may use in various places in our data).

These are currently used in the definition of line item BUT if there is a requirement for line items to have negative amounts a flag or similar should be added to the line item interface to accommodate this.

I want to reiterate, that we should have a way for the browser to understand some semantic about the line item and therefor know if it should be displayed as negative or positive. We already have this to some extent with the specialized line items for shipping costs.

This would help us address #56

@adrianhopebailie
Copy link
Collaborator Author

Submitted a PR to deal with negative numbers (amongst other things):
#111

@zkoch
Copy link
Contributor

zkoch commented Mar 31, 2016

I'm not sure what value these changes to the names provide, but I'm not opposed, though I don't like doing: amount.amount. Why not leave value?

@kirkalx
Copy link

kirkalx commented Mar 31, 2016

Forgive me if this issue has been addressed elsewhere, but if the currency code is to be flexible enough viz. 'any string is considered valid and user agents MUST not attempt to validate this string' then what good is the currency code field? e.g. I could insert the code BTC, intending it to mean bitcoin but someone could interpret it as some currency from the country of Bhutan.

@kirkalx
Copy link

kirkalx commented Mar 31, 2016

Further to my comment above, perhaps the URL approach could be used for the currency code as well, e.g. 3 letter iso currency code or URL identifying currency. Apologies if this has already been considered...

@adrianhopebailie
Copy link
Collaborator Author

@zkoch the PR addresses the inconsistency between the spec and the resolution made by the group not just in the naming but also the fact that we resolved to use unsigned numbers and allow any string as a currency identifier. See w3c/webpayments#57

On that basis the PR is non-controversial and should be merged. However, if there is a desire by the group to revisit that resolution then let's do that once the spec is at least consistent with the WG consensus as previously recorded.

@kirkalx The browser API is not the place to enforce validation against a) a list we don't control and b) a list that does not include all of the values we know we want to support. I'd be very concerned if we expected user agents to do data validation on this beyond very basic format conformance.

The set of supported currencies SHOULD be defined per payment method so we should leave it for the payment method specifications to restrict the currencies they support if they wish to do so.

This will then be validated by the payment app that processes the payment request.

@adrianhopebailie
Copy link
Collaborator Author

PROPOSAL: Use the label "value" instead of "amount" for the compound CurrencyAmount type.

This would imply the following rewording of the resolution previously made by the WG (w3c/webpayments#57):

The amount object MUST have at least two named properties: "currency" and "amount".
becomes
The amount object MUST have at least two named properties: "currency" and "value".

Please indicate your support with a +1 or -1 and I'll update the PR based on feedback.
(Note that GitHub now supports a +1 or -1 directly on this comment using the +:smile: icon at the top-right)

@kirkalx
Copy link

kirkalx commented Apr 1, 2016

The set of supported currencies SHOULD be defined per payment method so we should leave it for the payment method specifications to restrict the currencies they support if they wish to do so.

Thanks again Adrian, I had thought that should probably be the case after posting my suggestion. Cheers!

@dlongley
Copy link

dlongley commented Apr 1, 2016

PROPOSAL: Use the label "value" instead of "amount" for the compound CurrencyAmount type.

I think "value" is too generic and also seems potentially misleading to me (does a simple reading cause someone to think it represents the value of the currency, which fluctuates daily?). If we need to change something to avoid amount.amount, I'd prefer that we change the former, not the latter.

I'm a general -1 to any modeling of data that uses extremely generic properties that change their meaning based upon the position of a particular object -- when that data is intended to be serialized to a JSON representation and transmitted as such. I believe that's the case here.

Meaning should be better carried in the data itself. Instead this meaning is only present when the data has been parsed into a strongly-typed system that is running code that required reading human-only-consumable documentation. Using more specific terms makes it easier to apply machine-readable context after the fact. As "value" is probably the most generic term that could possibly be chosen here, I'm a big -1. "Amount" is still generic, but at least considerably less so. I'd also be ok with "monetaryValue" (or some other prefix) as a better alternative.

Let's remember the Rule of Least Power. I think we often forget how much context or extra power is required to understand the data when we're making modeling decisions. We may not even consider it all -- which is not a good approach for the Web.

@dlongley
Copy link

dlongley commented Apr 1, 2016

A good article on meaningful data APIs:

http://blog.codeship.com/json-ld-building-meaningful-data-apis/

@adrianba
Copy link
Contributor

adrianba commented Apr 1, 2016

  • I don't support having the spec say amount.amount - I don't believe that was what the group intended when this discussion happened in January.
  • I believe the current API shape, context, and @dlongley's comments are new information.
  • I'm not sure that I support the idea that currency amounts can't be negative numbers. Every finance application I've worked with supports the notion of negative amounts.

Per discussion this has been changed to suit the context for FPWD. It
is expected that a new proposal will be made to revise all of these
attribute names
@adrianhopebailie
Copy link
Collaborator Author

@dlongley

I'm a general -1 to any modeling of data that uses extremely generic properties that change their meaning based upon the position of a particular object -- when that data is intended to be serialized to a JSON representation and transmitted as such.

Can you provide a proposal for this interface that meets the criteria you have defined? Yours was the only objection to reverting amount back to value but I suspect you would like to propose an entirely different set of less generic names.

@adrianba

I don't support having the spec say amount.amount - I don't believe that was what the group intended when this discussion happened in January

I agree, but this could also be fixed by changing the PaymentItem interface to use a different attribute name. My proposal above to revert back to amount.value has not got consensus.

I believe the current API shape, context, and @dlongley's comments are new information.

Again I agree, and I think this is sufficient grounds to stick with the original resolution of the group until the new information has been discussed and we have a new proposal that is accepted by the WG.

I'm not sure that I support the idea that currency amounts can't be negative numbers. Every finance application I've worked with supports the notion of negative amounts.

This is the norm in financial messaging (see ISO20022 and ISO8583). Using a signed amount infers "settlement impact" which gets very confusing when combined with a transaction type. Example, what does a -$50 credit mean? Is it actually a debit. The UI is expected to abstract this complexity away and can use a whole host of heuristics to make this easier for the user to understand such as context, transaction type, locale etc.

There are three issues addressed in this PR:

  1. naming the amount/value attribute of this compound type
  2. allowing negative numbers
  3. allowing more free form currency identifiers

I believe we only have strong objections to the way the first is handled (although no consensus and one supporter). I hear reservations (but mostly questions) about the second and no further issues with the last.

The first feels a little like bike-shedding to me and while I hear @dlongley 's concerns it seems to me he is proposing a far more sweeping change to all attribute names which could be addressed in it's own subsequent PR.

On that basis I have edited the PR to revert to "value" instead of "amount" and request that the editors merge the PR as is (which reflects the previous consensus of the group and a minor terminology change which I expect @dlongley to propose major changes to anyway).

@adrianba
Copy link
Contributor

adrianba commented Apr 2, 2016

Merged as 563ecd5 per instructions from chairs.

@adrianba adrianba closed this Apr 2, 2016
@dlongley
Copy link

dlongley commented Apr 2, 2016

@adrianhopebailie,

On that basis I have edited the PR to revert to "value" instead of "amount" and request that the editors merge the PR as is (which reflects the previous consensus of the group and a minor terminology change...

I don't consider it a minor terminology change (for the reasons stated in this thread); the consensus of the group and the resolution was to use "amount". If it had said "value", it wouldn't have gotten my vote. See the use of "amount" in: w3c/webpayments#57

In any case, I'm not going to raise an objection that gets in the way of FPWD here, but we should accurately reflect the consensus of the group. We can then debate changing it further. As I said, I'm "ok" with "amount" (it got my vote), because I think it's "sufficiently specific" in this case. But if "amount" is problematic here, I would propose "monetaryValue" rather than the ultra-generic "value".

It's true that I have a general concern that we're being too cavalier in how we go about naming terms in data that is intended to be transported as JSON and shared between various actors (user agents, Payment Apps, etc). Often we end up picking pretty good terms incidentally anyway -- but we should be more deliberate in respecting best practices like the Rule of Least Power.

I don't have any other specific terms that I believe need further bikeshedding right now, but I do hope people will be more mindful of this and if there are other terms that need addressing I (and hopefully others) will bring them up in separate issues in the future.

@adrianba
Copy link
Contributor

adrianba commented Apr 2, 2016

Removing the ability to have negative amounts from the spec is a significant change and not one that I support. I don't think we will support the FPWD with this change. Applying discount codes is an important aspect that we think should be in the FPWD scope.

@dlongley
Copy link

dlongley commented Apr 2, 2016

@adrianba,

Applying discount codes is an important aspect that we think should be in the FPWD scope.

Can't these just be modeled using a "type" that indicates "Credit" or "Debit"? This seems like it's a "how should we model it" question, not whether or not it should be possible.

@adrianba
Copy link
Contributor

adrianba commented Apr 2, 2016

Can't these just be modeled using a "type" that indicates "Credit" or "Debit"? This seems like it's a "how should we model it" question, not whether or not it should be possible.

Perhaps, but until this change the only way to support it was with a negative amount. Now there is no way to do this. I don't object to proposing a different way but I do object to removing the capability completely.

@dlongley
Copy link

dlongley commented Apr 2, 2016

@adrianba,

Now there is no way to do this. I don't object to proposing a different way but I do object to removing the capability completely.

That's reasonable. I think everyone wants to support the functionality and some perhaps didn't really consider it supported previously because they weren't thinking you would/should ever use a negative number. So from their perspective there was a gap and continues to be a gap for this feature -- that will eventually be solved with a different solution.

@dlongley
Copy link

dlongley commented Apr 2, 2016

@adrianba,

Perhaps the best thing to do this late in the game would be to add an issue marker that indicates we want to support discounts, etc., and that would be sufficient for FPWD. I doubt there would be an objection to that.

@adrianba
Copy link
Contributor

adrianba commented Apr 2, 2016

I think the design issues you raised are important. I think we should be thoughtful about how we deal with negative values (removing the possibility without a replacement isn't that). I think the working group couldn't make an informed choice in the absence of context in January. I think #101 is a bad change that we didn't need to make before FPWD and I think we should revert it.

@kirkalx
Copy link

kirkalx commented Apr 2, 2016

If this PR is reverted, I believe the currency code liberalization part, or something similar, is necessary.

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.

7 participants