Passing in a parameter value with thousands separator but without decimal separator results in wrong amount

[stex]

Hey 😃

I'm using Money with attributes backed by decimal columns as I needed fractions of cents and didn't think of defining an own currency.

I noticed a strange behaviour today: With the "EUR" currency, Money usually formats and parses strings using the correct thousands and decimal limiters, e.g. "1.000,00".

However, when being given values without any decimal value, it seems to interpret the string as a float and removes everything after the dot - "1.000" becomes 1.0.

Is this the expected behaviour and I misconfigured something?

I wrote a short test for this behaviour:

#!/usr/bin/env ruby

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails"
  gem "sqlite3"
  gem "money-rails"
end

require "active_record"
require "active_support/all"
require "minitest/autorun"
require "logger"
require "money-rails"

# Money Configuration

Money.default_infinite_precision = true

MoneyRails.configure do |config|
  config.default_currency = :eur

  config.amount_column = { prefix: "",
    postfix: "_cents",
    column_name: nil,
    type: :decimal,
    present: true,
    null: false,
    default: 0 }

  config.currency_column = { prefix: "",
    postfix: "_currency",
    column_name: nil,
    type: :string,
    present: true,
    null: false,
    default: "EUR" }
end

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

MoneyRails::Hooks.init

ActiveRecord::Schema.define do
  create_table :money_models, force: true do |t|
    t.monetize :amount
  end
end

class MoneyModel < ActiveRecord::Base
  monetize :amount_cents
end

class BugTest < Minitest::Test
  def test_money_parsing
    record = MoneyModel.new

    record.amount = "1000,00"
    assert_equal 1000.0, record.amount.to_f

    record.amount = "1.000,00"
    assert_equal 1000.0, record.amount.to_f

    record.amount = "1.000"
    assert_equal 1000.0, record.amount.to_f

    # Failure:
    # BugTest#test_money_parsing [money-test.rb:69]:
    # Expected: 1000.0
    #   Actual: 1.0
  end
end

[sunny]

Since . is also a decimal separator (in English but also in Ruby), it makes sense to me that "1.234" would be interpreted as the 1.234 float.

If your inputs never use . as a decimal separator, you could try to remove them beforehand. Or perhaps there could be an option to always do this if your app doesn’t use . as a separator?

[stex]

In this case, it definitely is treated as a float, but in other cases, the currency settings are respected (for EUR, it is set to treat . as thousands separator and , as decimal separator).

I found the following cases by now:

"1.000"    # => Treated as float, becomes 1.0
"1.000,00" # => Treated as set in the currency, becomes 1000.0
"1.00"     # => You'd think, it would be treated as a float again, but no. It triggers an "invalid currency" validation error.

It seems inconsistent to me what happens here.

For now, I convert the value client-side as I didn't want to override the Money attribute setters, but I'd like to find a better solution.

[sunny]

Oh, indeed that is inconsistent.

I guess it should be one of the following:

• Both "1.00" and "1.000" are treated as the 1.0 float.
• Both "1.00" and "1.000" trigger an “invalid currency” error.
• "1.00" triggers an error and "1.000" is treated as the 1000.0 float when the currency’s thousands separator is ..

[stex]

I'd say that the 3rd one makes the most sense:

• "1.00" should be invalid as . is set as thousands separator and doesn't belong in this number
• "1.000" may not be treated as 1.0 as , is set as decimal point. MoneyRails uses the currency settings otherwise as "1.000,00" is parsed correctly.

[semmons99]

please submit a PR to consider changing this

[stex]

@semmons99 shouldn't the issue stay open as long as this isn't fixed?

[semmons99]

a PR with a failing test demonstration this is preferred to an open issue for how we try and run this project. we do this to help bring community collaboration and accountability to the project—hard learned lessons from 15 years on this project.

[stex]

Interesting approach... well, ok. Good luck :)