double_entry
double_entry copied to clipboard
Add configurable support for using the shopify-money gem
Unfortunately shopify-money
and money
do not play well together. Both gems provide a Money
class and an entry point into the gem at lib/money.rb
. This makes it pretty difficult to include double_entry in a codebase that makes use of the shopify-money
gem.
To work around this I've added a configuration option shopify_money
that allows double_entry to be configured to internally use the shopify-money
gem instead of the money
gem. shopify-money
was added as a development dependency since it's expected that people using this option will be requiring shopify-money
themselves. The specs can be run against either gem by providing the MONEY_GEM
env variable. For example:
DB=mysql MONEY_GEM=shopify-money bundle exec rspec
DB=mysql MONEY_GEM=money bundle exec rspec
Omitting the MONEY_GEM
var will use the money
gem as usual.
This was done by adding a DoubleEntry::Money
class to abstract the money
and shopify-money
apis. When using money
it actually doesn't do anything and just returns the raw Money
object. When using shopify-money
it wraps the api to make it match the one provided by the money
gem.
Tested with shopify-money
0.14.2 & 1.0.2.pre
Hey @jeremycw! Thanks for your contribution.
That's really a tricky issue, those gems should ideally use different namespaces, it feels like shopify-money
should use the Shopify::
namespace to make it consistent with the gem name, maybe you can consider reaching out to its maintainers.
But I understand that we can't expect that to happen, so in the meantime I'll recommend a slightly different approach.
While your suggestion seems to work, it introduces a dependency on shopify-money
that we would have to maintain going forward and we don't have the throughput for that.
An alternative is introducing a configurable money adapter. It could be defined in DoubleEntry.config.money_adapter
having a default of Money
(or ::Money
). You can then override it with something like ::ShopifyMoneyAdapter
which would have an implementation compatible with the money
gem but based on shopify-money
, similar to the solution you provided here.
I noticed that your PR mostly changes references to Money
in specs but not in the gem code. Maybe the interface is somewhat equivalent but if we proceed with this I feel like all references to Money
methods should be changed to use the adapter.
Let me know what you think.
@ricobl Thanks for the suggestion, that makes sense. I've taken a first pass at your suggestion. I've left DoubleEntry::Money
in as a module that delegates new
and any singleton methods to the configured adapter. Let me know if you would prefer me to change the actual call sites to use DoubleEntry.config.money_adapter
instead.
@ricobl Can we get this merged in and a new release put out?
https://github.com/envato/double_entry/issues/211
This PR can be simplified by replacing Money.zero(currency)
with Money.new(0, currency)
. It's all it does anyway and then from this gem's point of view creating new Money objects works exactly the same regardless of implementation.
@orien Could you take a look at this? It seems like the original reviewer is no longer a maintainer.
@orien I want to see if there is any reason why we cannot get this merged and a new 2.1 release cut?
@daande Are you using this branch in production already? Does it work as expected?