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

Has Many Through Association via has_many: and belongs_to: on the join table #590

Closed
allavena opened this issue Aug 18, 2015 · 12 comments
Closed

Comments

@allavena
Copy link

I'm reopening #530 (#530) as I'm facing the same issue and don't believe the reference #466) addresses it (Or if it does, I've completely failed to understand)

class Document < ActiveRecord::Base
  has_many :sections
  has_many :paragraphs, through: :sections
end

class Section < ActiveRecord::Base
  belongs_to :document
  has_many :paragraphs
end

class Paragraph < ActiveRecord::Base
  belongs_to :section
end

Paper_Trail 4.0.0 Beta doesn't cater this situation. It looks for foreign key of association Paragraph i.e, section_id in the through model i.e, Section which will not be available in the above mentioned case.

#466 mentions adding an additional column to 'versions' table. However, the data is already there. With the existing code, a query on section can already return its paragraphs and revisions.

[If anything, may I suggest a note in the Readme that this case is not supported?]

Thanks
André

@jaredbeck
Copy link
Member

Paper_Trail 4.0.0 Beta doesn't cater this situation.

Currently, paper_trail 4.0.0 is the latest. If you are still using a beta, please upgrade.

It looks for foreign key of association Paragraph i.e, section_id in the through model i.e, Section which will not be available in the above mentioned case.

What PaperTrail methods are you using? reify? You've given some ActiveRecord models, but I don't see anything from PaperTrail in your example.

@allavena
Copy link
Author

Bad copy/paste, sorry. I'm using paper trail 4.0.0, although same result with using master from git.

document = FactoryGirl.create(:document_with_sections_and_paragraphs)
puts 'Updating some paragraph'
document.sections[0].paragraphs[0].update_attributes(title: 'New paragraph 1 title')
puts 'Updating a section name'
document.sections[0].update_attributes(title: 'My new Section')
puts 'Updating document itself'
document.update_attributes(title: 'New document Title')
document.sections[0].paragraphs[0].update_attributes(title: 'Very New paragraph 1 title')
puts 'Previous version of document'
pp document.versions.last.reify(:has_many => true)

This last line fails, in the rails console in development mode, with a
NoMethodError: undefined method `section_id' for #Section:0x00000006c6ad58

Note, if I stick that wrap that code in a test, it runs, however, the previous version of the document returns an empty list of sections.

@jaredbeck
Copy link
Member

I can't reproduce this issue. Please review the test app I created (https://github.com/jaredbeck/pt_issue_590) and propose (via a PR to the test app) a change which will reproduce the issue. Thanks.

@allavena
Copy link
Author

For me, there are two distinct issues. In test mode, behaviour is different than from the console. In test, the old version no longer has the associations - test app shows this. But the crash, I only get in console. I don't know how to reproduce in test mode (I think it would crash there too, if the associations were not lost). Here is the code reproducing the issue in console:

From the console:

ActiveRecord::Migrator.migrate "db/migrate"
d = Document.create_sample_document
d_v2 = d.versions.last.reify(:has_many => true)

last line fails with the below. Happy to provide more info/test, let me know what is needed.

NoMethodError: undefined method section_id' for #<Section:0x00000001566318> from /home/andre/.rvm/gems/ruby-2.1.2/gems/activemodel-4.2.3/lib/active_model/attribute_methods.rb:433:inmethod_missing'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:394:in block (2 levels) in reify_has_many_through' from /home/andre/.rvm/gems/ruby-2.1.2/gems/activerecord-4.2.3/lib/active_record/relation/delegation.rb:46:inmap'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/activerecord-4.2.3/lib/active_record/relation/delegation.rb:46:in map' from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:394:inblock in reify_has_many_through'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:391:in each' from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:391:inreify_has_many_through'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:347:in reify_has_manys' from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:232:inblock in reify'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:303:in call' from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:303:inwithout_identity_map'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/paper_trail-4.0.0/lib/paper_trail/version_concern.rb:168:in reify' from (irb):3 from /home/andre/.rvm/gems/ruby-2.1.2/gems/railties-4.2.3/lib/rails/commands/console.rb:110:instart'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/railties-4.2.3/lib/rails/commands/console.rb:9:in start' from /home/andre/.rvm/gems/ruby-2.1.2/gems/railties-4.2.3/lib/rails/commands/commands_tasks.rb:68:inconsole'
from /home/andre/.rvm/gems/ruby-2.1.2/gems/railties-4.2.3/lib/rails/commands/commands_tasks.rb:39:in run_command!' from /home/andre/.rvm/gems/ruby-2.1.2/gems/railties-4.2.3/lib/rails/commands.rb:17:in<top (required)>'

@allavena
Copy link
Author

If I merely remove the has_many :paragraphs, through: :sections from document, then it no longer crashes in console - this is something I'm happy to leave for my app.

Difference between test and dev environment is still there. I've sent you another pull request for a simpler case (no paragraphs) which still shows the children missing on a previous version in test environment.

@jaredbeck
Copy link
Member

Difference between test and dev environment is still there.

Yes, it's not currently possible to test reification of associations when use_transactional_fixtures = true. This is a known issue: #542

Regarding the NoMethodError ..

NoMethodError: undefined method `section_id' for #Section:0x00000006c6ad58

By setting use_transactional_fixtures = false, I've reproduced this error in the test app we've been working on (https://github.com/jaredbeck/pt_issue_590) with a failing test.

If I merely remove the has_many :paragraphs, through: :sections from document, then it no longer crashes in console - this is something I'm happy to leave for my app.

I'm glad that workaround is OK for you, but I believe it is our goal to support has_many through, and you shouldn't have to do that. I'll mark this as a confirmed bug. Thanks for reporting it!

@jaredbeck
Copy link
Member

I've added a warning to the readme: ce37034

@theRealNG
Copy link
Contributor

The issue is at this line: https://github.com/airblade/paper_trail/blob/master/lib/paper_trail/version_concern.rb#L420

collection_keys should collect the list of all the records that are associated with has_many through relationship. ( The present code only works for belongs_to association).

Fixed the issue in theRealNG@5bcfb86

Will test it throughly and place a pull request

@jaredbeck
Copy link
Member

Fixed by #596, closing.

@bdelmas
Copy link

bdelmas commented Apr 18, 2016

Is it really fixed? I came across this. Maybe we can delete it from the known issue if it is?

@jaredbeck
Copy link
Member

Is it really fixed? I came across this. Maybe we can delete it from the known issue if it is?

Hi Benjamin. This issue (#590) was fixed by #596 and released in PT 4.1.0. You are looking at the docs for 4.0. The docs for 4.1 (https://github.com/airblade/paper_trail/blob/4.1-stable/README.md) do not mention #590.

@bdelmas
Copy link

bdelmas commented Apr 20, 2016

Hi Jared. Ah ok, thanks for your answer! I still have problems with associations with the 4.1.0 and the master version. If I can pin down the problem I will post an issue.

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

4 participants