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

Add support to has_many form building with draper #1774

Closed
ready4god2513 opened this issue Nov 18, 2012 · 9 comments
Closed

Add support to has_many form building with draper #1774

ready4god2513 opened this issue Nov 18, 2012 · 9 comments

Comments

@ready4god2513
Copy link

When using draper to decorate, building forms with has_many fields does not work correctly as the class is not a model, but a draper wrapped object. Draper provides a method called "model_class" which will return the wrapped object class.

@ready4god2513
Copy link
Author

I am working on a fix for this right now and should have a pull request once I go through all the steps in the contribution guide.

@amiel
Copy link
Contributor

amiel commented Mar 22, 2013

@ready4god2513 How's this coming? I was actually thinking we should not send the decorated model to the form anyway, but I'm curious what your angle on this is...

See #2013.

@seanlinsley
Copy link
Contributor

@amiel given what's happened since, thoughts on this?

@amiel
Copy link
Contributor

amiel commented Apr 30, 2013

@ready4god2513 Does #2085 solve this for you? Or do you still need to handle has_many form building with decorated form objects.

I don't have need for this myself, but I do think that using decorate: false is not an obvious fix for this problem, so it should at least be documented...

@daxter you at railsconf?

@seanlinsley
Copy link
Contributor

No I'm not unfortunately. I assume you are?

I'm assuming that the OP's problem is caused by this line in the has_many form builder.

Where we'd need this to deal with either situation:

obj      = has_many_form.object
resource = obj.respond_to?(:model_class) ? obj.model_class : obj
if resource.new_record?
  # ...

@ready4god2513
Copy link
Author

I actually haven't been using active_admin for most recent projects. Sorry I couldn't be of more help.

@amiel
Copy link
Contributor

amiel commented May 1, 2013

@daxter thoughts? Are you ok with letting this lie until it becomes an issue?

@seanlinsley
Copy link
Contributor

I'm hesitant to implement this since I haven't actually used decorators before. I'd just leave it be until someone else runs into the problem.

@amiel
Copy link
Contributor

amiel commented May 1, 2013

That sounds reasonable to me. I'll be happy to tackle it if it becomes an issue. Until then the decorate: false feature solves the problem for me.

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

No branches or pull requests

3 participants