Skip to content

Commit

Permalink
Ensure :index works with fields_for select methods. [#518 state:resol…
Browse files Browse the repository at this point in the history 
…ved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
  • Loading branch information
@lifo
rsl authored and lifo committed Jul 13, 2008
1 parent 0d241f4 commit 95812d5
Show file tree
Hide file tree
Showing 2 changed files with 911 additions and 26 deletions.
8 changes: 4 additions & 4 deletions actionpack/lib/action_view/helpers/form_options_helper.rb
View file
Expand Up @@ -445,19 +445,19 @@ def add_options(option_tags, options, value = nil)

class FormBuilder
def select(method, choices, options = {}, html_options = {})
@template.select(@object_name, method, choices, options.merge(:object => @object), html_options)
@template.select(@object_name, method, choices, objectify_options(options), @default_options.merge(html_options))
end

def collection_select(method, collection, value_method, text_method, options = {}, html_options = {})
@template.collection_select(@object_name, method, collection, value_method, text_method, options.merge(:object => @object), html_options)
@template.collection_select(@object_name, method, collection, value_method, text_method, objectify_options(options), @default_options.merge(html_options))
end

def country_select(method, priority_countries = nil, options = {}, html_options = {})
@template.country_select(@object_name, method, priority_countries, options.merge(:object => @object), html_options)
@template.country_select(@object_name, method, priority_countries, objectify_options(options), @default_options.merge(html_options))
end

def time_zone_select(method, priority_zones = nil, options = {}, html_options = {})
@template.time_zone_select(@object_name, method, priority_zones, options.merge(:object => @object), html_options)
@template.time_zone_select(@object_name, method, priority_zones, objectify_options(options), @default_options.merge(html_options))
end
end
end
Expand Down

6 comments on commit 95812d5

@rsl
Copy link
Contributor

@rsl rsl commented on 95812d5 Jul 14, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a strange looking commit. :) Is it my browser? A lot of empty lines.

@ncr
Copy link
Contributor

@ncr ncr commented on 95812d5 Jul 14, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like some kind of funny injection testing :)

@yaroslav
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selects are really nice

@augustl
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope they escape coments.

@ncr
Copy link
Contributor

@ncr ncr commented on 95812d5 Jul 14, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do now :)

@glennpow
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am attempting to utilize mass-assign for my models, but can’t get the update_attributes to work correctly when I have association_collections as child models.
My create methods work fine, since the hash passes in an Array value for these collections. I.E.:
emails => [ {…}, {…} ] )
But, when I try to edit/update my model, the field names in the form naturally have the :id of the email included in them, so that the resulting value passed to the mass-assign is not an Array, but a Hash of this form:
emails => {“5” => {…}, “6” => {…}}
Where the 5 and 6 are the :id values of the respective emails. This gets passed into the update_attributes, which eventually gets to:
AssociationCollection.replace(other_array)
which assumes “emails” to be an Array. Shouldn’t this replace method be “smarter” so that if a Hash is passed in, it will ascertain the :id values from it, and then reassign the model attributes accordingly?
Furthermore, what would happen if there were a combination of updated models (emails) and perhaps one new email (that didn’t have an :id). Does the mass-assign functionality handle this situation?
Any help would be appreciated.
-Glenn

Please sign in to comment.