Skip to content

Conversation

mcls
Copy link
Contributor

@mcls mcls commented Jan 22, 2015

See #18646

Should the same be done for the following lines? E.g. model.respond_to?(:human_attribute_name)
https://github.com/rails/rails/blob/v4.2.0/actionview/lib/action_view/helpers/tags/placeholderable.rb#L22-L24

@rafaelfranca
Copy link
Member

Actually I think we should call set object = object.to_model and use it later.

@mcls
Copy link
Contributor Author

mcls commented Jan 22, 2015

Is there a reason it doesn't happen immediately when doing the retrieve_object?

https://github.com/rails/rails/blob/v4.2.0/actionview/lib/action_view/helpers/tags/base.rb#L41

@rafaelfranca
Copy link
Member

I don't think so. Try it and see how tests responds to the change.

@mcls
Copy link
Contributor Author

mcls commented Jan 22, 2015

Yeah, never mind, they blew up. I'll try to find that out for myself later.

I'll just add the object = object.to_model for now.

Thanks for the help :)

@mcls mcls force-pushed the placeholderable-to-model branch from b688687 to 505447f Compare January 22, 2015 21:13
@mcls
Copy link
Contributor Author

mcls commented Jan 22, 2015

Hmm... found very similar code here: https://github.com/rails/rails/blob/v4.2.0/actionview/lib/action_view/helpers/tags/label.rb#L20-L32

I'll try to clean that up

@mcls mcls force-pushed the placeholderable-to-model branch from 505447f to c946b2a Compare January 22, 2015 22:03
@mcls
Copy link
Contributor Author

mcls commented Jan 22, 2015

Not too sure about that last commit.

It reduces the duplication, but it could probably be done in a clearer way.

@rafaelfranca rafaelfranca merged commit c946b2a into rails:master Feb 5, 2015
rafaelfranca added a commit that referenced this pull request Feb 5, 2015
The `model_name` method should be called on `to_model`
mfazekas added a commit to mfazekas/rails that referenced this pull request Feb 6, 2015
…AR form_for

Refactoring at rails#18647 broke using non active record objects in form_for. This patch
restores the original behaviour where we only compute i18 key when object.respond_to?(:to_model)
@mfazekas
Copy link
Contributor

mfazekas commented Feb 6, 2015

There is a bug in the merged version:
79100b8#diff-75ef522cd1bc95094df58064c185a4efR11

@model = object.respond_to?(:to_model) ? object.to_model : object

we should not set @model if it's not responds to :to_model, this breaks

form_for(OpenStuct.new(....),)

see #18834

mfazekas added a commit to mfazekas/rails that referenced this pull request Feb 6, 2015
…AR form_for

Refactoring at rails#18647 broke using non active record objects in form_for. This patch
restores the original behaviour where we only compute i18 key when object.respond_to?(:to_model)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants