money-rails icon indicating copy to clipboard operation
money-rails copied to clipboard

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

Open wwahammy opened this issue 4 years ago • 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\"}".

wwahammy avatar Mar 04 '21 00:03 wwahammy

can you show the code for price_in_a_range?

semmons99 avatar Mar 04 '21 00:03 semmons99

It's from the spec code: https://github.com/RubyMoney/money-rails/blob/71b4d8fca9d1a3a9dd23caf63d1c83871801b898/spec/dummy/app/models/product.rb#L24-L33

wwahammy avatar Mar 04 '21 00:03 wwahammy

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

wwahammy avatar Mar 04 '21 01:03 wwahammy

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?

semmons99 avatar Mar 04 '21 11:03 semmons99

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

machty avatar Jan 04 '22 20:01 machty

@machty I just submitted the PR with my change: https://github.com/RubyMoney/money-rails/pull/649.

wwahammy avatar Jan 04 '22 23:01 wwahammy