RubyMoney/money-rails

`monetize`d attribute autoconverts Hash to Money but validation rejects

wwahammy opened this issue · 6 comments

Today, I noticed inconsistent validation behavior when validating the numericality of a provided value. In particular, if you set an attribute to a Hash, it will be type cast to a Money object but the validation will reject it as invalid. This can be illustrated by adding the following tests to spec/active_record/monetizable_spec.rb at line 403

         it "passes validation when Money is given" do
        product = Product.create(price_in_a_range: Money.new(5000, 'usd'))
        expect(product.errors[:price_in_a_range].size).to eq(0)
      end

      it "passes validation when Hash is given" do
        product = Product.create(price_in_a_range: { cents: 5000, currency:'usd'})
        expect(product.price_in_a_range).to eq(Money.new(5000, 'usd'))
        expect(product.errors[:price_in_a_range].size).to eq(0)
      end

The first test illustrates that assigning a Money object which is larger than 0 and less than $100 (the requirement set for price_in_a_range) to price_in_a_range validates properly.

However, the second test illustrates though that the price_in_a_range will properly type cast the Hash to a Money object (the first expectation passes) but price_in_a_range is still treated as invalid.

I think the cause of this is in #normalize at line 115. If details.raw_value is Money object, Numeric or String, the #to_s on line 115 will lead to a text representation of a number. However, if you provide a Hash, you'll end up with something like this: "{:amount=>4440, :currency=>\"usd\"}".

can you show the code for price_in_a_range?

It's from the spec code:

monetize :price_in_a_range_cents, allow_nil: true,
subunit_numericality: {
greater_than: 0,
less_than_or_equal_to: 10000
},
numericality: {
greater_than: 0,
less_than_or_equal_to: 100,
message: "must be greater than zero and less than $100"
}

I have a possible fix actually but I'm not sure the behavior is totally correct. The following values for price_in_a_range pass:

Product.create(price_in_a_range:{amount: 4, currency: 'usd'})
Product.create(price_in_a_range:{cents: 4})
Product.create(price_in_a_range:{}) # treats it as if `nil` was passed

However the following will fail validation because price_in_a_range must be greater than 0:

Product.create(price_in_a_range:{currency: 'usd'}) # treat it as if `0` was passed to cents
Product.create(price_in_range:{some_other_key: 289}) # treat it as if `0` was passed to cents

Looking at the code, it seems like what needs to happen is to cast the hash to a money object right away and use that for validation instead of the hash.

is it standard behavior for folks using money-rails to not cast to an explicit money object and just pass hashes around and assume the magic happens?

@wwahammy I'm getting bit by this too; did you find a workaround? Is there any PR to fix this?

@machty I just submitted the PR with my change: #649.