omnipay icon indicating copy to clipboard operation
omnipay copied to clipboard

Item price - what are the units?

Open judgej opened this issue 9 years ago • 8 comments
trafficstars

The price of an Item appears to be a string that is passed on to the remote gateway unaltered. It does not get validated, parsed or converted in any way. So, what are the units? There seem to be two main options:

  • It's whatever the remote gateway wants. That doesn't feel very "OmniPay".
  • It's the major units like the overall price. There are no tools built into the Item for validating, converting to minor units etc. so every driver would need to provide their own.

Any other thoughts? The documentation does not really state one way or the other, but since the main amount is a decimal, it makes sense for the Item price to be treated the same way. Hopefully it can be a proper money object of some sort in v3.0.

judgej avatar Jul 25 '16 10:07 judgej

I would say they need to be handled the same as the regular amount.

barryvdh avatar Jul 25 '16 10:07 barryvdh

That would be my assumption too. I think I'll put a check in my gateway: if it is an integer then it's minor units, and if a string with a decimal point or a float, then it's in major units. Not ideal, but just trying to be be pragmatic.

judgej avatar Jul 25 '16 16:07 judgej

Should V3 formalise it a little more? Perhaps accept a money object (with no ambiguities) as well as the current string?

judgej avatar Jul 25 '16 22:07 judgej

The ambiguity of the price attribute is stopping me from implementing this. If there is any plan to add a Money parameter, I would be more than happy to help.

devnix avatar Nov 13 '20 12:11 devnix

@devnix there is a setMoney() method on the abstract request now: https://github.com/thephpleague/omnipay-common/blob/master/src/Common/Message/AbstractRequest.php#L366

The setAmount() method accepts major units as a string. Make sure you include the decimal point and minor units in the amount that is passed in to be sure it is parsed correctly.

The setAmountInteger() method accepts minor units as an integer.

Hopefully one of the above will serve your needs. If the documentation is not clear, then maybe that's something that could be improved?

judgej avatar Nov 15 '20 23:11 judgej

What concerns me is that there is not any kind of typehint about passing an int or a float. For example, I would expect passing 100 to send €1 to the gateway, and this is technically possible: each gateway can implement it with or without decimals.

Hopefully one of the above will serve your needs. If the documentation is not clear, then maybe that's something that could be improved?

Yup. The docs are very unclear about a lot of things, and the items API is not even documented. If I want to support items on my own Omnipay driver, I could be doing it in the wrong way, and for people using two different gateways with two different behaviors, this is an important problem. That's why I was thinking about sending a Money object to setPrice and eliminate any ambiguity: https://github.com/thephpleague/omnipay-common/blob/master/src/Common/Item.php#L104

devnix avatar Nov 16 '20 07:11 devnix

Since setAmount() does not have any type-hints, then it does make sense to allow it to accept a Money object.

I agree about a lot of the documentation. A lot of it was written a long time ago, with quite a bit either in our heads, or requiring looking through the code. Feel free to submit a PR, even if it is for just one or two lines. Lots of baby steps is what it needs IMO.

judgej avatar Nov 16 '20 10:11 judgej

Yeah the Items aren't really formalized so I think most of them just use the value as 'as-is'. So if you put in '12.34', it will provide that to the gateway. It would be good to make it follow the same rules, but we must make sure we don't break BC.

barryvdh avatar Nov 16 '20 10:11 barryvdh