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

offsite_payments 3.0 request for comments #250

Open
bdewater opened this issue Jun 12, 2017 · 1 comment
Open

offsite_payments 3.0 request for comments #250

bdewater opened this issue Jun 12, 2017 · 1 comment

Comments

@bdewater
Copy link
Contributor

bdewater commented Jun 12, 2017

👋 hello friends! Offsite Payments releases have been few and far between ever since it has become a separate project from Active Merchant. Here at Shopify we want to introduce some (breaking) changes and would like the input from the community using this gem before moving forward.

Changes

Remove money gem support, use BigDecimal instead

This follows ActiveMerchant's deprecation, but we believe using integers is not the best way and want to propose BigDecimal instead. BigDecimal does not suffer from floating point rounding errors (that's why money gems use it internally) and is intuitively more clear to use than juggling the smallest fraction of currency (e.g. cents) in integers. Not to mention the amount of decimal places for a currency could change in the future.

Compare representing 1 unit of currency in the two different data types:

Type Dollar/Euro Yen/ISK Bahraini/Jordanian Dinar
Integer 100 1 1000
BigDecimal 1.0 1.0 1.0

Consuming apps are free to use the whatever implementation they see fit so represent money value objects and do calculations/conversions with. In offsite payments we only do some string formatting if the gateway requires cents for example, most currencies have 2 decimal places so for this a small list of currencies with 0 or 3 decimals would be maintained.

WIP PR: #248

Added Jul 14: some gateways diverge from ISO4217, an example that keeps popping up is TWD. See #255 for an example. We need to make clear that the interface conforms to ISO and provide a facility for gateways to override this if they are handling it differently. ActiveMerchant's list of currencies_without_fractions and currencies_with_three_decimal_places is a bit of a crutch, a list of currencies with exponents other than 2 solves this more elegantly.

Introduce better error hierarchy

This should make it easier for consuming apps to deal with stuff going wrong. The structure is still to be decided. Issue: #207

Switch language/country options to locale

Some gateways accept language or country in the options but this does not tell the whole story for i18n. See the differences between fr_FR, fr_CA and en_CA for an example, although just 'fr' is valid too.

Deprecate existing language parameters and standardize on passing in locales which can contain both, the gateway code can decide what data to pass on to the API.

Remove support for old Ruby / Active(Support|View) versions

This would clean up our Travis test matrix quite a bit. Dropping support for older Rubies allows us to take advantage of for example keyword arguments, making it more clear what standard keys (see locale above) can be passed in instead of relying on conventions for an options hash.

Implementation plan

We create a 2.x branch with the current state of master for fixing bugs while all new features go to master, which will eventually become a released 3.0.0 version. With it, we want to become more consistent in releasing versions and maintaining a proper changelog. 2.x will receive 6 months of bug fixes, but no new features.

@bdewater bdewater changed the title offsite_payment 3.0 request for comments offsite_payments 3.0 request for comments Jun 12, 2017
@3ynm
Copy link

3ynm commented Mar 15, 2019

Just out of curiosity, given that you haven't received any comments and this lib is not as popular as active merchant, you still want to create a new version/introduce breaking changes? (would you update all of your currently supported gateways?)

About using integers instead of decimals, I think it's OK, the point is not losing precision, the mechanism behind it is really not that important, as always with Rails: it's better convention than configuration.

Internally in Shopify you still use this gem?

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

No branches or pull requests

2 participants