Permalink
Browse files

We don't need to check blank? here.

Also the blank? check introduced a bug.

$ rails generate model Foo blank:boolean

form_for(Foo.new(:blank => true)) => ArgumentError, "First argument in form cannot contain nil or be empty"
  • Loading branch information...
1 parent d76ef6f commit e1e4b1d1fde315c74ad983719ed03407ca324c4b @rafaelfranca rafaelfranca committed Oct 7, 2012
Showing with 1 addition and 1 deletion.
  1. +1 −1 actionpack/lib/action_view/helpers/form_helper.rb
@@ -423,7 +423,7 @@ def form_for(record, options = {}, &proc)
object = nil
else
object = record.is_a?(Array) ? record.last : record
- raise ArgumentError, "First argument in form cannot contain nil or be empty" if object.blank?
+ raise ArgumentError, "First argument in form cannot contain nil or be empty" unless object
@skojin
skojin Oct 7, 2012

then "or be empty" not need in error message too

@rafaelfranca
rafaelfranca Oct 7, 2012 Ruby on Rails member

It is still needed because the first argument of the form can be a []

@sobrinho
sobrinho Oct 7, 2012

@rafaelfranca in that case the exception will not be raised because the [] is evaluated as true.

@skojin
skojin Oct 7, 2012

@sobrinho no, here checked array.first

@sobrinho
sobrinho Oct 7, 2012

@rafaelfranca I'm not sure if should deal with that case of blank attribute.

It basically will break every ruby code that relies on Object#blank?.

@rafaelfranca
rafaelfranca Oct 7, 2012 Ruby on Rails member

I know but we don't need to be so defensive and check blank. Both nil and [] are being covered. I don't think we should cover "" and [""].

@sobrinho
sobrinho Oct 7, 2012

I had not seen the record.last, it makes more sense now :)

object_name = options[:as] || model_name_from_record_or_class(object).param_key
apply_form_for_options!(record, object, options)
end

0 comments on commit e1e4b1d

Please sign in to comment.