Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

form_for behaves inconsistently for symbol and instance arguments. #9166

Closed
jjoos opened this Issue Feb 3, 2013 · 3 comments

Comments

Projects
None yet
3 participants

jjoos commented Feb 3, 2013

I'm using rails 3.2.11 and noticed that the form_for method behaves differently for:

form_for @person

or

form_for :person

For the #new action the form action url is set to '/persons/new' instead of '/persons/'
In both cases the form is filled correctly for an #edit and has the correct url.

I made a really quick and dirty patch to fix this:

index a3409ee..f4b5cf4 100644
--- a/lib/action_view/helpers/form_helper.rb
+++ b/form_helper_patched.rb
@@ -360,12 +360,19 @@ module ActionView

         options[:html] ||= {}

-        case record
+        object = record.is_a?(Array) ? record.last : record
+        case object
         when String, Symbol
-          object_name = record
-          object      = nil
+          object_name = object
+          object      = instance_variable_get("@#{object_name}")
+
+          if object.nil?
+            # TODO: this should use the correct record array when record is a array.
+            options[:url] ||= polymorphic_path(object_name.to_s.camelize.constantize, :format => options.delete(:format))
+          else
+            # TODO: this should use the correct record array when record is a array.
+            apply_form_for_options!(object, options)
+          end
         else
-          object      = record.is_a?(Array) ? record.last : record
           object_name = options[:as] || ActiveModel::Naming.param_key(object)
           apply_form_for_options!(record, options)
         end

I need this functionality because I have active model compatible classes that I can't instantiate in a invalid state. So instantiating a dummy object for the #new action becomes quite awkward.

Is this something that would be considered? If so, maybe some people could give me some pointers and I'll make a proper pull request!

P.S.
I don't seem to be the only one confused by this issue.
http://stackoverflow.com/questions/2006329/ruby-on-rails-symbol-as-argument-in-form-for

Member

steveklabnik commented Feb 3, 2013

Is this something that would be considered? If so, maybe some people could give me some pointers and I'll make a proper pull request!

In general, if you have a patch, just submit one! It's much more likely to be 'accepted' if the code already exists.

We have a contributing guide here: http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html

Please, do re-submit this as a PR, and we can discuss the specifics then. :)

Using an object and a symbol are the available APIs for form_for, perhaps the confusion is better docs explaining and showing differences, I don't think that adding instance_variable_get to the mix would help, I think it might introduce more confusion. My 2 cents, of course please feel free to better clarify your intents and sum everything up in a pull request. Thanks!

jjoos commented Feb 6, 2013

But when you're using a symbol, the form is still filled from the instance variable (with matching name) only the form meta data (class, url, etc.) aren't set properly. So it's not only a documentation issue.

And why would you need to initialize a dummy object on the #new action. I know this produces no overhead for active record. For me, because I'm writing a different active model implementation, this causes friction.

I also see some unit test for the form_for symbol variation, where the expected results that are asserted seem strange and inconsistent. I.e. why wouldn't a default class or id that is set for the instance version, wouldn't be set for the symbol version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment