search

envato / double_entry

A double-entry accounting system for Ruby applications.
https://rubygems.org/gems/double_entry
MIT License
422 stars 68 forks source link

Add configurable support for using the shopify-money gem #213

Open jeremycw opened 2 years ago

jeremycw commented 2 years ago

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

ricobl commented 2 years ago

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.

jeremycw commented 2 years ago

@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.

daande commented 2 years ago

@ricobl Can we get this merged in and a new release put out?

https://github.com/envato/double_entry/issues/211

bdewater commented 1 year ago

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.

jeremycw commented 1 year ago

@orien Could you take a look at this? It seems like the original reviewer is no longer a maintainer.

daande commented 11 months ago

@orien I want to see if there is any reason why we cannot get this merged and a new 2.1 release cut?

orien commented 11 months ago

@daande Are you using this branch in production already? Does it work as expected?