moiristo / deep_cloneable

This gem gives every ActiveRecord::Base object the possibility to do a deep clone that includes user specified associations.
MIT License
786 stars 88 forks source link

Doesn't seem to work with "has_many ... :through " #22

Closed fokcep closed 11 years ago

fokcep commented 11 years ago

it's trying to load id of the association in the target object instead of association.

Looks like a bug

moiristo commented 11 years ago

There are tests for hasmany through relations that pass, so could you be more specific? Which version of rails are you using? A failing test would be helpful (or some sample code to exemplify the issue).

fokcep commented 11 years ago

Rails 3.2.11 Ruby 1.9.3-p374

Example of the problem

Student.rb

has_many :subjects, :through => :student_assignments
has_many :student_assignments, :dependent => :destroy

Subject

has_many :students, :through => :student_assignments
has_many :student_assignments, :dependent => :destroy

StudentAssignment

belongs_to :subject
belongs_to :student

Then

Student.first.subjects.size

2

(Student.first.dup :include => [:student_assignments]).subjects.size

0


Or am I missing something obvious?

Thanks

moiristo commented 11 years ago

Not sure yet. I'll look into it.

moiristo commented 11 years ago

Hold on, I think you should simply include the subjects instead of the join-table. Try

Student.first.dup(:include => :subjects).subjects.size

Does that work for you? I think it doesn't work in your case because you don't include the associations in student_assignments. This should work too:

Student.first.dup(:include => { :student_assignments => :subject }).subjects.size
fokcep commented 11 years ago

I've tried that too

i.e.

Student.first.dup(:include => :subjects).subjects.size

NoMethodError: undefined method `subject_id=' for #<Subject .....

and

Student.first.dup(:include => { :student_assignments => :subject }).subjects.size

still empty

moiristo commented 11 years ago

Ok, thanks for the bugreport. I'll try to reproduce and fix the issue shortly.

moiristo commented 11 years ago

I've added a test that reproduces the error. Unfortunately, it's trickier then I thought. It is easy to set the association itself, but it's hard to find and set the reverse association (students) on the newly created subjects. For now, I recommend that you stick to the method:

Student.first.dup(:include => :student_assignments) # will create assignments with existing subjects
Student.first.dup(:include => { :student_assignments => :subject }) # will create new subjects

This will not set the 'subjects' association directly, so in that case you'll have to do something like:

Student.first.dup(:include => { :student_assignments => :subject }).student_assignments.map(&:subject)

After saving the student dup the subjects association will be reloaded, therefore it is only a problem before save.

fokcep commented 11 years ago

Thanks for looking into this. I've used something like this as a workaround. Please let me know when/if you fix it in future.

tristanoneil commented 11 years ago

:+1: Also ran into this issue. Worked around it but would be awesome if you could specify either the join model or associated model to clone a has_many :through.

moiristo commented 11 years ago

Fixed it in master, the test described in this issue now passes. Could you please verify that it works properly now?

tristanoneil commented 11 years ago

Just tested it and it does now work! Just including the through association like:

Student.first.dup(:include => :subjects)
tristanoneil commented 11 years ago

:shipit: Maybe we can get a release for this?

moiristo commented 11 years ago

Sure. Done! (1.5.2)