This project is archived and is in readonly mode.
numericality validation gives misleading error message when only_integer is used
Reported by Jsmith45 | April 15th, 2010 @ 07:02 PM | in 3.0.2
If one uses the only_integer option of the numericality
validator the result is an error message that may look like the
following:
"2.5 is not a number". That is a very misleading error message.
Unless I am mistaken, 2.5 is very much a number. Most of the other
options for the numericality validator result in customized error
messages by default. So while I could pass in a custom error
message every time I use ':only_integer => true', that seems
less than ideal.
Comments and changes to this ticket
-
Jsmith45 April 23rd, 2010 @ 05:59 AM
- Title changed from numericality validation gives misleading error message only_integer is uses to numericality validation gives misleading error message when only_integer is used
-
Jason Langenauer April 23rd, 2010 @ 02:01 PM
- Tag set to patch
Here's a patch with tests to resolve this issue - it's a patch against 2.3.5
The only minor issue is that it hard-codes the use of "." as the numeric separator, which can be obviously be localized. The behavior here is consistent with the rest of ActiveRecord, however.
-
Rodrigo Navarro April 24th, 2010 @ 02:35 AM
- Tag changed from patch to rails3 validations, patch
Rails 3 ActiveModel has the same problem. I don't know if I should open another ticket or post the patch here. Anyways, here is the patch for rails 3.
-
Jsmith45 April 24th, 2010 @ 06:09 AM
Jason, instead of testing for a decimal separator, why not just change the error message that occurs When the regex does not match? That regex is only run when only_integer is specified, and if said regex does not match then whatever was passed in is indisputably not an integer. That would also match Rodrigo's rails 3 patch.
Otherwise I see no obvious issues with either patch, but I'm not an expert in rails internals.
It probably does not need to be said, but if we don't open a second issue for Rails 3, let's make sure to land both patches at the same time, or to reopen after one lands so the second one closes the issue.
-
Rodrigo Navarro April 24th, 2010 @ 04:29 PM
Jsmith
Exactly, at least the rails 3 version of the patch, it first tries to convert the string using Kernel.Float and only if that test pass we check if it is an integer (using the regex /\A[+-]?\d+\Z/).
So Jason, as Jsmith said, try just to change the order of your checks, then you won't need to check for the ".", because the regex will do that for you.
-
Scott Messinger April 24th, 2010 @ 05:54 PM
The patch doesn't apply to the master.
Error:
error: patch failed: activemodel/lib/active_model/validations/numericality.rb:44
error: activemodel/lib/active_model/validations/numericality.rb: patch does not apply -
Rodrigo Navarro April 24th, 2010 @ 06:22 PM
Sorry Scott, my bad. Probably some weird formating issues. This one applies successfully.
-
Rodrigo Navarro April 24th, 2010 @ 07:28 PM
This one is for 2.3.x. Checking for the integer with the regex and not with the hardcoded dot and with refactored tests.
-
Neeraj Singh April 24th, 2010 @ 08:16 PM
Attached is a patch.
I have no additional if options[:only_integer] condition. One place where all the validations happen returns the error code.
-
Rodrigo Navarro April 24th, 2010 @ 10:05 PM
Well that was almost like my first approach too, but I decided to keep the checks separated, I feel that it is easily to read and understand, but that is just my opinion.
-
Neeraj Singh April 24th, 2010 @ 10:25 PM
Hi Rodrigo,
Sorry. I did not checkout all the patches. You are right it's pretty close.
-
José Valim April 24th, 2010 @ 10:29 PM
- Milestone cleared.
- Assigned user set to José Valim
-
Repository April 25th, 2010 @ 09:15 AM
- State changed from new to resolved
(from [77c099c231f2efb36a2a77a32138ed5c6761ec19]) Fix validates_numericaly_of only integer error message [#4406 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/77c099c231f2efb36a2a77a32138ed... -
José Valim April 25th, 2010 @ 09:19 AM
Excellent patch!
I applied it just on the 3.0 branch. Not on the 2.3 branch because it includes some non-minor changes in the way the validator works.
-
Neeraj Singh April 25th, 2010 @ 05:49 PM
- Tag changed from rails3 validations, patch to rails3 validations, 2.3.6, patch
Attached is a patch which is backport of the commit for 2-3-stable branch.
-
Jsmith45 April 25th, 2010 @ 06:49 PM
Please not that the patch just committed for Rails 3 gives a "is not a number" error if the input is completely non-numeric such as an input of "xyzzy", and an error of "must be an integer" if it is numeric but non-integral.
On the other hand I believe Neeraj's patch results in "must be an integer" in both cases. On the other hand Rodrigo's patch for 2.3 should give the same results as Rails 3.
-
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to 3.0.2
- Importance changed from to Low
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Tags
Referenced by
- 4406 numericality validation gives misleading error message when only_integer is used (from [77c099c231f2efb36a2a77a32138ed5c6761ec19]) Fix val...