Hello,
I found a weird (though slightly inconsequential) bug in rails''
validation.
Here''s the model:
class Photo < ActiveRecord::Base
validates_presence_of :name, :caption
validates_length_of :name, :maximum => 20
validates_length_of :caption, :maximum => 255
validates_uniqueness_of :name
end
Now, with this, if I go to the "new" action and submit a blank form, I
get the expected "name can''t be empty" and "caption
can''t be empty"
errors. But if I visit the "create" action directly (as opposed to
visiting it by submitting the form from the "new" action), I get four
errors. two are the same two I already mentioned, but the new two are
"name is too long (max is 20 characters)", and "caption is too
long
(max is 255 characters)"
So I think that''s kind of silly, saying the field is too long when
nothing was given. Perhaps some sort of "allow_nil" option should be
given to work around this, though I think making the "maximum"
restriction simply not trip for a nil value by default would be more
sensible.
Of course another workaround is to take an idea from the login
generator and combine the new/create actions into one action, perhaps
like this:
def new
case @request.method
when :get
# Do stuff that would normally be in ''new'' action
when :post
# Do stuff that would normally be in ''create'' action
end
end
Thoughts?
--
One Guy With A Camera
http://rbpark.ath.cx
Rob Park wrote:> I found a weird (though slightly inconsequential) bug in rails'' validation. > [...] > Perhaps some sort of "allow_nil" option should be > given to work around this, though I think making the "maximum" > restriction simply not trip for a nil value by default would be more > sensible.There is an :allow_nil option, but I agree that it poorly captures the intent of this validation. When you say validates_size_of :field, :maximum => 255 you are declaring that field.size <= 255, not that !field.nil? I think :allow_nil should be dropped entirely for this and every other declared validation. Whether field.nil? is the subject of another validation. validates_presence_of wraps errors.add_on_empty which checks whether !field.nil? and !field.empty?. I think more precise language helps: validates_not_blank :field # !field.blank? validates_not_nil :field # !field.nil? validates_not_zero :field # !field.zero? This usage makes the validation declarations start to look a lot like assertions. That feels natural. On an unrelated note, since we added _why''s happy #blank? it''d be great to include an assert_blank and assert_not_blank to Test::Unit::Assertions.> Of course another workaround is to take an idea from the login > generator and combine the new/create actions into one action, perhaps > like this: > > def new > case @request.method > when :get > # Do stuff that would normally be in ''new'' action > when :post > # Do stuff that would normally be in ''create'' action > end > end > > Thoughts?I really like this style of using HTTP verbs to act on a resource. I use it extensively. I''m not sure how it''s a solution to the problem above, though-- you still have to validate your record on POST. Best, jeremy