Skip to content

Commit

Permalink
Makes form_helper use overriden model accessors backport
Browse files Browse the repository at this point in the history
  • Loading branch information
spastorino committed Aug 1, 2010
1 parent 27651c1 commit 8141f08
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
6 changes: 3 additions & 3 deletions actionpack/lib/action_view/helpers/form_helper.rb
Expand Up @@ -877,9 +877,9 @@ def value(object, method_name)

def value_before_type_cast(object, method_name)
unless object.nil?
object.respond_to?(method_name + "_before_type_cast") ?
object.send(method_name + "_before_type_cast") :
object.send(method_name)
object.respond_to?(method_name) ?
object.send(method_name) :
object.send(method_name + "_before_type_cast")
end
end

Expand Down
17 changes: 17 additions & 0 deletions actionpack/test/template/form_helper_test.rb
Expand Up @@ -91,6 +91,16 @@ def post_attributes=(attributes); end
class FormHelperTest < ActionView::TestCase
tests ActionView::Helpers::FormHelper

class Developer
def name_before_type_cast
"David"
end

def name
"Santiago"
end
end

def setup
super

Expand Down Expand Up @@ -256,6 +266,13 @@ def test_text_field_doesnt_change_param_values
assert_equal object_name, "post[]"
end

def test_text_field_from_a_user_defined_method
@developer = Developer.new
assert_dom_equal(
'<input id="developer_name" name="developer[name]" size="30" type="text" value="Santiago" />', text_field("developer", "name")
)
end

def test_hidden_field
assert_dom_equal '<input id="post_title" name="post[title]" type="hidden" value="Hello World" />',
hidden_field("post", "title")
Expand Down

8 comments on commit 8141f08

@feldpost
Copy link

Choose a reason for hiding this comment

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

It's fine to add this to Rails 3, but I don't think it's a good idea to backport this to Rails 2.3. It's a major change of behavior. In the past I have taken full advantage of the fact that facade methods are not used to generate form values. This change causes breakages all over, and I have a feeling it may affect a lot of people. After all, this was not a bug -- this particular behavior has been documented in books, blog posts etc. Can you please re-consider this change for 2.3?

@ralph
Copy link

@ralph ralph commented on 8141f08 Sep 14, 2010

Choose a reason for hiding this comment

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

I agree with feldpost on this one. In addition to this, the method is called "value_BEFORE_type_cast", but it does deliver the value AFTER the type cast. This is misleading at best.

@sjain
Copy link

@sjain sjain commented on 8141f08 Sep 22, 2010

Choose a reason for hiding this comment

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

+1.
When validation errors happen on form, the form needs to display the invalid value. This code change prevents us from showing user entered invalid value. A case in point is any invalid numeric value gets turned into 0.0 (after the type cast).

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on 8141f08 Sep 26, 2010

Choose a reason for hiding this comment

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

This has now been reverted in 2-3-stable

@feldpost
Copy link

Choose a reason for hiding this comment

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

Thanks, Michael.

@ralph
Copy link

@ralph ralph commented on 8141f08 Sep 27, 2010

Choose a reason for hiding this comment

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

Cool, thank you!

@tomstuart
Copy link
Contributor

Choose a reason for hiding this comment

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

Why hasn't this been reverted in Rails 3 too? Is the failed-validation behaviour not a problem there?

@spastorino
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussing this we agreed to revert this behavior everywhere. So i'm reverting this on Rails 3 too

Please sign in to comment.