MONGOID-5842 validates_numericality_of is valid when allow_blank is true and string is assigned

[kyleburgess2025]

Prior to this change, the validation for numericality was skipped if allow_blank was set to true and a string was provided. This happened because typecasting the string resulted in nil, which caused the validation check to be bypassed here. We have updated read_attribute_for_validation to return the original, non-typecasted value if typecasting to a numeric type fails.

[Copilot]

Pull request overview

This PR addresses MONGOID-5842 by fixing validation behavior when validates_numericality_of is used with allow_blank: true and a non-numeric string is assigned to a numeric field. The fix ensures that non-numeric strings are properly validated as invalid, rather than being coerced to nil/0 before validation.

Key Changes

• Modified read_attribute_for_validation to return the pre-type-cast value for BigDecimal, Float, and Integer fields
• Added comprehensive test coverage for allow_blank: true scenarios with both Integer and Float field types

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
lib/mongoid/validatable.rb	Added logic to return pre-type-cast values for numeric field types during validation
spec/mongoid/validatable/numericality_spec.rb	Added test cases for allow_blank: true validation behavior with Integer and Float fields

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jamis]

Very nice. Thank you for your work on this!

I know I suggested this solution, but I was thinking more about it this morning and I worry a bit about changing read_attribute_for_validation like this. It's a Rails API, not a Mongoid one, which means we don't have any control over how Rails decides to use it. A new version of Rails could easily call that method somewhere unexpected, and expect a numeric value to be returned for a numeric field. The default implementation of prepare_value_for_validation, for example, simply returns the value it is given.

At the core of the issue, and this incompatibility, is the fact that ActiveRecord returns 0, not nil, when a non-numeric value is set and allow_blank is true. It's also how Ruby itself works (e.g. "hello".to_i # => 0). I wonder if we ought to roll that change back, so that Mongoid mirrors this? The alternative is probably to subclass NumericalValidator and write our own #validate method, to accommodate the fact that Mongoid returns nil instead of 0, here.

We should discuss this as a team next week, but this is a very good example of what we're trying to do with the Mongoid X initiative: this used to work, and now it doesn't. Do we revert the change? Do we allow the change and find a way to work around it? Or do we say that the new way is correct and the old way is wrong, and we describe how people ought to update their apps to accommodate that?

So. Let's hold off on further work here until we know what we want to do. Thank you for your investigation on this!

[johnnyshields]

At the core of the issue, and this incompatibility, is the fact that ActiveRecord returns 0, not nil, when a non-numeric value is set and allow_blank is true. It's also how Ruby itself works (e.g. "hello".to_i # => 0). I wonder if we ought to roll that change back, so that Mongoid mirrors this?

I strongly, strongly vote against changing this. nil is the correct behavior here; 0 is a valid number, the same as any other number. Coercing a string like "Foobar" to a number should not return any valid number, 0 or otherwise.

[jamis]

@johnnyshields, I agree that this conversion makes much more sense if it returns nil. Many programming languages even raise an error when such conversions are attempted, which is probably an even better solution (as it avoids any ambiguity).

However, we are kind of bound by two primary constraints in Mongoid. The first is that the project explicitly tries to mimic ActiveRecord in order to maximize compatibility with the wider Rails ecosystem. If you have an ActiveRecord model with an integer field, and you don’t have any validates_numericality_of validators on that field, then the following is true, even with the latest version of Rails:

class Person < ActiveRecord::Base
  # create_table :people do |t|
  #   t.string :name
  #   t.integer :age
  # end
end

user = User.create!(name: 'Alice', age: 'hello')
p user.age # => 0

The way to avoid this (as I'm sure you already know) is to validate the value first. If you use validates_numericality_of :age, then the numericality of the value will be enforced, and trying to set age to (e.g.) ’hello’ will cause the validation to fail.

However, even if we decide that parity with ActiveRecord is undesirable in this case, we have an even stronger precedent: Ruby itself. The default method for converting strings to integers will cast any non-numeric string to 0. (Yes, you can use Integer(‘hello’) instead, but the fact is that most Ruby programmers will reach for String#to_i first.)

If we decide that all of this is still insufficient and we insist that we know best and that casting ’hello’ to a Integer must return nil, we find ourselves in the situation we’re in right now: we have to rewrite pieces of Rails to accommodate our superior opinions.

Is it worth it? I don’t think so. This is Mongoid, which intentionally is trying to behave like ActiveRecord. Let’s embrace that constraint, and embrace Rails’ (and Ruby’s) opinions. If we push back, it only makes more work for us and more code to maintain.

For the sake of compatibility with Rails and its ecosystem, Mongoid’s numeric fields should be casting arbitrary strings to 0, instead of nil.

[johnnyshields]

@jamis Unlike ActiveRecord / SQL, MongoDB can in theory have any value in any field. So this is uncharted territory as far as AR is concerned--there is no "AR precedent" here.

What Mongoid should do is in this proposal here: https://jira.mongodb.org/browse/MONGOID-5609.

We should introduce a concept of Mongoid::RawValue which wraps uncastable values. Then we can handle them in a variety of ways. Coercing to "foo" to 0 is a terrible choice. Coercing "foo" to nil is a less bad choice. RawValue would be the best. I started implementing it in this PR here #5368 and asked for comments.

But before introducing new breaking behaviors, I would really like to see the team first reduce all the breaks/changes from Mongoid 8/9...

This is Mongoid, which intentionally is trying to behave like ActiveRecord. Let’s embrace that constraint, and embrace Rails’ (and Ruby’s) opinions. If we push back, it only makes more work for us and more code to maintain

This has never been a core philosophy of Mongoid, and if you go down this path, it will cause much heartbreak. Mongoid should not "embrace" ActiveRecord--we should keep it at arm's length. Use conventions from AR where it makes sense, but do not go out of our way to adopt AR conventions which deviate from Mongo-like norms, especially when it comes to flexible type handling (a core feature of MongoDB, not of SQL). Double-down on MongoDB, not on AR/SQL.

[jamis]

@johnnyshields You're right about MongoDB. But this isn't about MongoDB; it's about Mongoid, and Mongoid is constrained as I described. If someone were to create an entirely new ODM, with no baggage from ActiveRecord (oh, glorious thought), the story would very definitely be different, on many fronts.

But before introducing new breaking behaviors, I would really like to see the team first reduce all the breaks/changes from Mongoid 8/9...

Funny you should say that. This ticket is actually one of the (large) backlog of tickets we're looking at for our "Mongoid X" initiative. It relates to a change in Mongoid 8 that broke existing behavior from Mongoid 7.5, which is why it is being looked at right now.

[johnnyshields]

Mongoid is constrained as I described

No it is not, and it never has been. The worst mistakes of Mongoid, such as the and/or-query chaining debacle (before your time at MongoDB), all have arisen from trying to "embrace" ActiveRecord behavior. ("But this time is different" you will say...)

Anyway, in absence of a strong reason to change this, let's please keep the status quo of returning nil. This is a behavior that I discussed at length with Neil Shweky when we designed the current type casting logic--we reviewed the coercion of all types (not just String and Integer, but Date/Float/Boolean/yada-yada) and intentionally designed this spec, and yes, we were very well aware of both the AR and Ruby behaviors here. Chesterton's Fence applies--please read the old JIRA trail first.