A few months ago I attempted to solve ticket 608 and as then I was too
new rails resulting patches where ''less promising''. You will
see too
many attachments and unclear comments, and this why 608 ended up being
one of the oldest not applied patches from queue. Now I want to do it
right and as such I would much appreciate your input.
Root of the problem is that ActiveRecord casts attributes from request
params to ruby objects matching the respective DB column type. That is
string "34" will be casted to number 34 if corresponding table column
is INTEGER. This coercion is performed
using the to_i method. For strings that cannot be converted to_i will
returns zero:
"not a number".to_i
=> 0
After this "not a number" string will be hidden behind
column_before_typecast making validation against column pass.
I suggest using a special value for conversion failed case. This cant
be zero (as it is currently) nor nil as both values may already have a
different meaning. Simplest would be to leave "not a number" string
unconverted and forget attr_before_typecast facility.
Detecting numbers can be a bit difficult, using a conversion to
Integer would be an option:
Integer("not a number")
=> ArgumentError: invalid value for Integer: "not a number"
But this is not quite perfect, as Integer is more liberal than most
web users are used to:
Integer("1_1_2_4")
=> 1124
Integer("0xffffffff")
=> 4294967295
Integer("011")
=> 9
Integer("-0b101")
=> -5
This ugly function should handle all cases:
def cast_to_integer(value) #nodoc
return 1 if value == true
return 0 if value == false
return value if value.class <= Integer
return value.to_i if /^\s*[+-]?\d+\s*$/ =~ value
return nil if /^\s*$/ =~ value
return value if /\w/ =~ value
int = value.to_i
(int == 0) ? (Integer(value) rescue value) : int
end
Once conversion is performed only if possible code bellow will do the
expected thing.
Topic.validates_numericality_of( :approved, :only_integer => true)
And that should end ticket 608, only that I (unfortunately) mixed in
autovalidation. That is adding validates_numericality_of for all
numerical columns, validates_presence_of for all database columns not
allowing NULL and validates_length_of for all string columns (see
patch with_autovalidation.diff). I''ve made it completely automatic,
while giving the chance to optionally turn off feature with
autovalidation_off for some columns. Or turn off completely by:
ActiveRecord::Base.do_autovalidation = false
Now autovalidation is a bit controversial, do we enable it by default
or do we not? Do we handle check for validates_uniqueness_of or do we
not? Also it has higher impact and it is not implemented for other
adapters than Mysql. And lastly there is another ticket #1112 for
with a very different approach
I think that autovalidation is more about sweetening, I would like to
see numerical columns handled correctly sooner. Therefore I''m very
curious about opinions on:
- can we ditch attr_before_typecast?
- is there another simpler way while keeping attr_before_typecast?
- could we not solve this ticket without autovalidation
(smaller impact)?
- How (if it) should be performed by autovalidation?
Zsombor
--
http://deezsombor.blogspot.com