Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Hydra::Presenter#to_model delegation with self #145

Merged
merged 2 commits into from
Mar 21, 2018

Conversation

no-reply
Copy link
Member

@no-reply no-reply commented Mar 5, 2018

Beginning in v3.5.1, SimpleForm::FormBuilder calls #to_model (via
ActionView's #convert_to_model) in its initializer. This prevents
Hydra::Presenter inheritors from being used correctly when sent to the
form. Replacing the delegated method with one that returns self restores the
correct behavior to SimpleForm::FormBuilder and related view helpers.

Client code can still access the model by calling the #model accessor.

Fixes #139.

Tom Johnson added 2 commits March 5, 2018 13:50
Beginning in `v3.5.1`, `SimpleForm::FormBuilder` calls `#to_model` (via
ActionView's `#convert_to_model`) in its initializer. This prevents
`Hydra::Presenter` inheritors from being used correctly when sent to the
form. Replacing the delegated method with one that returns `self` restores the
correct behavior to `SimpleForm::FormBuilder` and related view helpers.

Client code can still access the model by calling the `#model` accessor.

Fixes #139.
@no-reply
Copy link
Member Author

no-reply commented Mar 5, 2018

A hyrax build running this code is at https://travis-ci.org/samvera/hyrax/jobs/349501904

@jrochkind
Copy link

jrochkind commented Mar 5, 2018

This seems to be in line with what ActiveModel#to_model expects

http://api.rubyonrails.org/classes/ActiveModel/Conversion.html#method-i-to_model

If your object is already designed to implement all of the Active Model you can use the default :to_model implementation, which simply returns self.… If your model does not act like an Active Model object, then you should define :to_model yourself returning a proxy object that wraps your object with Active Model compliant methods.

That is, if it's already ActiveModel-compliant, just return self, only return something else if you want something wanting an ActiveModel to use that, which is why simple_form was using that.

Copy link
Contributor

@jrgriffiniii jrgriffiniii left a comment

Choose a reason for hiding this comment

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

Thank you @no-reply for linking to the related Hyrax test

@jrgriffiniii jrgriffiniii merged commit c264bb8 into master Mar 21, 2018
@jrgriffiniii jrgriffiniii deleted the simple_form-update branch March 21, 2018 15:27
@aeschylus aeschylus removed the review label Mar 21, 2018
no-reply pushed a commit to samvera/hyrax that referenced this pull request May 4, 2018
The pin was initally introduced in #2663. `hydra-editor` now handles pinning of
`simple_form` versions (see: samvera/hydra-editor#143)
on its 3.x series. We support newer versions of `simple_form` via `hydra-editor`
4.x (see: samvera/hydra-editor#145).

Fixes #2665.
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.

None yet

4 participants