RubyMoney / money-rails

Integration of RubyMoney - Money with Rails
MIT License
1.8k stars 387 forks source link

Allow for null price and currency columns #605

Closed thebravoman closed 3 years ago

thebravoman commented 4 years ago

nul: true options seems not to have the effect I would expect. Not sure if it is a problem in my configuration or I am missing something else. I am adding price to an already existing model so all the previous models should have a null value for the price.

I am adding a migration

class AddPriceToGroup < ActiveRecord::Migration[6.0]
  def change
    add_column :groups, :some, :integer, null: true
    add_monetize :groups, :price, null: true
  end
end

As a result I would expect to have a column "some" that could be null and a column price_cents that could be null and a column price_currency that could be null

Instead this is generated.

== 20200918035411 AddPriceToGroup: migrating ==================================
-- add_column(:groups, :some, :integer, {:null=>true})
   -> 0.0016s
-- add_column(:groups, "price_cents", :integer, {:null=>false, :default=>0})
   -> 0.0487s
-- add_column(:groups, "price_currency", :string, {:null=>false, :default=>"USD"})
   -> 0.0412s
== 20200918035411 AddPriceToGroup: migrated (0.0921s) =========================

This is the db/schema

+    t.integer "some"
+    t.integer "price_cents", default: 0, null: false
+    t.string "price_currency", default: "USD", null: false

Update 1

I managed to get it with

    add_monetize :groups, :price, amount: { null: true }, currency: {null: true}

that would produce

-- add_column(:groups, "price_cents", :integer, {:null=>true, :default=>0})
   -> 0.0829s
-- add_column(:groups, "price_currency", :string, {:null=>true, :default=>"USD"})
   -> 1.9088s

I then saw it in the readme - https://github.com/RubyMoney/money-rails#allow-nil-values

I guess it is all there, I was just expecting it to work in another way. Probably not a bug, yet it is not clear if we set the amount to null: true should we also set the currency to null: true.

It would be great if it can follow the same syntax for other columns where null: true would just set both amount, currency and all the other internal columns that are needed by the gem to null: true

antstorm commented 4 years ago

@thebravoman yes, this makes sense to me. Would you be up for putting together a PR to allow null: true as a top level argument affecting both columns?

thebravoman commented 4 years ago

@antstorm I went on to browse and see what would be required and reached.

$ rake spec:all
The dependency activerecord-jdbc-adapter (>= 0) will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for java. To add those platforms to the bundle, run `bundle lock --add-platform java`.
The dependency activerecord-jdbcsqlite3-adapter (>= 0) will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for java. To add those platforms to the bundle, run `bundle lock --add-platform java`.
The dependency jruby-openssl (>= 0) will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for java. To add those platforms to the bundle, run `bundle lock --add-platform java`.
[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/kireto/axles/tmp/money-rails/Rakefile:38)
BUNDLE_GEMFILE='gemfiles/mongoid2.gemfile' bundle install --quiet
Bundler could not find compatible versions for gem "bundler":
  In mongoid2.gemfile:
    rails (~> 3.1) was resolved to 3.2.22.5, which depends on
      bundler (~> 1.0)

  Current Bundler version:
    bundler (2.1.4)
This Gemfile requires a different version of Bundler.
Perhaps you need to update Bundler by running `gem install bundler`?

Could not find gem 'bundler (~> 1.0)', which is required by gem 'rails (~> 3.1)', in any of the sources.
rake aborted!
Command failed with status (6): [BUNDLE_GEMFILE='gemfiles/mongoid2.gemfile'...]
/home/kireto/axles/tmp/money-rails/Rakefile:40:in `block in run_with_gemfile'
/home/kireto/.rvm/gems/ruby-2.6.5/gems/bundler-2.1.4/lib/bundler.rb:376:in `block in with_clean_env'
/home/kireto/.rvm/gems/ruby-2.6.5/gems/bundler-2.1.4/lib/bundler.rb:677:in `with_env'
/home/kireto/.rvm/gems/ruby-2.6.5/gems/bundler-2.1.4/lib/bundler.rb:376:in `with_clean_env'
/home/kireto/axles/tmp/money-rails/Rakefile:38:in `run_with_gemfile'
/home/kireto/axles/tmp/money-rails/Rakefile:62:in `block (3 levels) in <top (required)>'
/home/kireto/.rvm/gems/ruby-2.6.5/gems/rake-13.0.1/exe/rake:27:in `<top (required)>'
Tasks: TOP => spec:all => spec:mongoid => spec:mongoid2
(See full trace by running task with --trace)

I think I can get around it but was my first encounted.

thebravoman commented 4 years ago

I've prepared a pull request - https://github.com/RubyMoney/money-rails/pull/606