merge with original master

README.md

@@ -135,17 +135,19 @@ When you declare `has_paper_trail` in your model, you get these methods:
 
 ```ruby
 class Widget < ActiveRecord::Base
-  has_paper_trail   # you can pass various options here
+  has_paper_trail
 end
 
-# Returns this widget's versions.  You can customise the name of the association.
+# Returns this widget's versions.  You can customise the name of the
+# association.
 widget.versions
 
 # Return the version this widget was reified from, or nil if it is live.
 # You can customise the name of the method.
 widget.version
 
-# Returns true if this widget is the current, live one; or false if it is from a previous version.
+# Returns true if this widget is the current, live one; or false if it is from
+# a previous version.
 widget.live?
 
 # Returns who put the widget into its current state.
@@ -160,7 +162,8 @@ widget.previous_version
 # Returns the widget (not a version) as it became next.
 widget.next_version
 
-# Generates a version for a `touch` event (`widget.touch` does NOT generate a version)
+# Generates a version for a `touch` event (`widget.touch` does NOT generate a
+# version)
 widget.touch_with_version
 
 # Turn PaperTrail off for all widgets.
@@ -169,9 +172,11 @@ Widget.paper_trail_off!
 # Turn PaperTrail on for all widgets.
 Widget.paper_trail_on!
 
-# Check whether PaperTrail is enabled for all widgets.
+# Is PaperTrail enabled for Widget, the class?
 Widget.paper_trail_enabled_for_model?
-widget.paper_trail_enabled_for_model? # only available on instances of versioned models
+
+# Is PaperTrail enabled for widget, the instance?
+widget.paper_trail_enabled_for_model?
 ```
 
 And a `PaperTrail::Version` instance has these methods:
@@ -282,32 +287,14 @@ usual.
 
 Here's a helpful table showing what PaperTrail stores:
 
-<table>
-  <tr>
-    <th>Event</th>
-    <th>Model Before</th>
-    <th>Model After</th>
-  </tr>
-  <tr>
-    <td>create</td>
-    <td>nil</td>
-    <td>widget</td>
-  </tr>
-  <tr>
-    <td>update</td>
-    <td>widget</td>
-    <td>widget</td>
-  <tr>
-    <td>destroy</td>
-    <td>widget</td>
-    <td>nil</td>
-  </tr>
-</table>
+| *Event*        | *create* | *update* | *destroy* |
+| -------------- | -------- | -------- | --------- |
+| *Model Before* | nil      | widget   | widget    |
+| *Model After*  | widget   | widget   | nil       |
 
 PaperTrail stores the values in the Model Before column.  Most other
 auditing/versioning plugins store the After column.
 
-
 ## Choosing Lifecycle Events To Monitor
 
 You can choose which events to track with the `on` option.  For example, to
@@ -319,6 +306,10 @@ class Article < ActiveRecord::Base
 end
 ```
 
+`has_paper_trail` installs callbacks for these lifecycle events. If there are
+other callbacks in your model, their order relative to those installed by
+PaperTrail may matter, so be aware of any potential interactions.
+
 You may also have the `PaperTrail::Version` model save a custom string in it's
 `event` field instead of the typical `create`, `update`, `destroy`. PaperTrail
 supplies a custom accessor method called `paper_trail_event`, which it will

lib/paper_trail/version_concern.rb

@@ -417,7 +417,14 @@ module PaperTrail
       associations.each do |assoc|
         next unless assoc.klass.paper_trail_enabled_for_model?
         through_collection = model.send(assoc.options[:through])
-        collection_keys = through_collection.map { |through_model| through_model.send(assoc.foreign_key) }
+
+        # if the association is a has_many association again, then call reify_has_manys for each through_collection
+        if !assoc.source_reflection.belongs_to? && through_collection.present?
+          through_collection.each { |through_model| reify_has_manys(through_model,options) }
+          next
+        end
+
+        collection_keys = through_collection.map { |through_model| through_model.send(assoc.association_foreign_key)}
 
         version_id_subquery = assoc.klass.paper_trail_version_class.
           select("MIN(id)").

paper_trail.gemspec

@@ -33,6 +33,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'rspec-rails', '~> 3.1.0'
   s.add_development_dependency 'generator_spec'
   s.add_development_dependency 'database_cleaner', '~> 1.2'
+  s.add_development_dependency 'pry-nav'
 
   # Allow time travel in testing. timecop is only supported after 1.9.2 but does a better cleanup at 'return'
   if RUBY_VERSION < "1.9.2"

test/dummy/app/models/chapter.rb

@@ -0,0 +1,9 @@
+class Chapter < ActiveRecord::Base
+  has_many :sections, :dependent => :destroy
+  has_many :paragraphs, :through => :sections
+
+  has_many :quotations, :dependent => :destroy
+  has_many :citations, :through => :quotations
+
+  has_paper_trail
+end

test/dummy/app/models/citation.rb

@@ -0,0 +1,5 @@
+class Citation < ActiveRecord::Base
+  belongs_to :quotation
+
+  has_paper_trail
+end

test/dummy/app/models/paragraph.rb

@@ -0,0 +1,5 @@
+class Paragraph < ActiveRecord::Base
+  belongs_to :section
+
+  has_paper_trail
+end

test/dummy/app/models/quotation.rb

@@ -0,0 +1,5 @@
+class Quotation < ActiveRecord::Base
+  belongs_to :chapter
+  has_many :citations, :dependent => :destroy
+  has_paper_trail
+end

test/dummy/app/models/section.rb

@@ -0,0 +1,6 @@
+class Section < ActiveRecord::Base
+  belongs_to :chapter
+  has_many :paragraphs, :dependent => :destroy
+
+  has_paper_trail
+end

test/dummy/db/migrate/20110208155312_set_up_test_tables.rb

@@ -216,9 +216,33 @@ class SetUpTestTables < ActiveRecord::Migration
       t.string  :some_content
       t.boolean :deleted, :default => false
     end
+
+    create_table :chapters, :force => true do |t|
+      t.string :name
+    end
+
+    create_table :sections, :force => true do |t|
+      t.integer :chapter_id
+      t.string :name
+    end
+
+    create_table :paragraphs, :force => true do |t|
+      t.integer :section_id
+      t.string :name
+    end
+
+    create_table :quotations, :force => true do |t|
+      t.integer :chapter_id
+    end
+
+    create_table :citations, :force => true do |t|
+      t.integer :quotation_id
+    end
   end
 
   def self.down
+    drop_table :citations
+    drop_table :quotations
     drop_table :animals
     drop_table :skippers
     drop_table :not_on_updates
@@ -252,6 +276,9 @@ class SetUpTestTables < ActiveRecord::Migration
     drop_table :line_items
     drop_table :fruits
     drop_table :boolits
+    drop_table :chapters
+    drop_table :sections
+    drop_table :paragraphs
     remove_index :version_associations, :column => [:version_id]
     remove_index :version_associations, :name => 'index_version_associations_on_foreign_key'
     drop_table :version_associations

test/test_helper.rb

@@ -1,3 +1,9 @@
+begin
+  require 'pry-nav'
+rescue LoadError
+  # It's OK, we don't include pry in e.g. gemfiles/3.0.gemfile
+end
+
 ENV["RAILS_ENV"] = "test"
 ENV["DB"] ||= "sqlite"
 

test/unit/model_test.rb

@@ -1902,4 +1902,169 @@ class HasPaperTrailModelTransactionalTest < ActiveSupport::TestCase
       end
     end
   end
+
+  context "Models with nested has_many through relationships" do
+    setup { @chapter = Chapter.create(:name => 'ch_1') }
+
+    context "before any associations are created" do
+      setup do
+        @chapter.update_attributes(:name => "ch_2")
+        @ch_1 = @chapter.versions.last.reify(:has_many => true)
+      end
+
+      should "reify the record when reify is called" do
+        assert_equal "ch_1", @ch_1.name
+      end
+    end
+
+    context "after the first has_many through relationship is created" do
+      setup do
+        @chapter.update_attributes :name => "ch_2"
+        Timecop.travel 1.second.since
+        @chapter.sections.create :name => "section 1"
+        Timecop.travel 1.second.since
+        @chapter.sections.first.update_attributes :name => "section 2"
+        Timecop.travel 1.second.since
+        @chapter.update_attributes :name => "ch_3"
+        Timecop.travel 1.second.since
+        @chapter.sections.first.update_attributes :name => "section 3"
+      end
+
+      context "when reify is called" do
+        setup do
+          @chapter_2 = @chapter.versions.last.reify(:has_many => true)
+        end
+
+        should "show the value of the base record as it was before" do
+          assert_equal "ch_2", @chapter_2.name
+        end
+
+        should "show the value of the associated record as it was before the base record was updated" do
+          assert_equal ['section 2'], @chapter_2.sections.map(&:name)
+        end
+
+        context "to the version before the relationship was created" do
+          setup { @chapter_1 = @chapter.versions.second.reify(:has_many => true) }
+
+          should "not return any associated records" do
+            assert_equal 0, @chapter_1.sections.size
+          end
+        end
+
+        context "to the version before the associated record has been destroyed" do
+          setup do
+            @chapter.update_attributes :name => 'ch_3'
+            Timecop.travel 1.second.since
+            @chapter.sections.destroy_all
+            Timecop.travel 1.second.since
+            @chapter_3 = @chapter.versions.last.reify(:has_many => true)
+          end
+
+          should "return the associated record" do
+            assert_equal ['section 2'], @chapter_3.sections.map(&:name)
+          end
+        end
+
+        context "to the version after the associated record has been destroyed" do
+          setup do
+            @chapter.sections.destroy_all
+            Timecop.travel 1.second.since
+            @chapter.update_attributes :name => 'ch_4'
+            Timecop.travel 1.second.since
+            @chapter_4 = @chapter.versions.last.reify(:has_many => true)
+          end
+
+          should "return the associated record" do
+            assert_equal 0, @chapter_4.sections.size
+          end
+        end
+      end
+
+      context "after the nested has_many through relationship is created" do
+        setup do
+          @section = @chapter.sections.first
+          @paragraph = @section.paragraphs.create :name => 'para1'
+        end
+
+        context "reify the associations" do
+          setup do
+            Timecop.travel 1.second.since
+            @initial_section_name = @section.name
+            @initial_paragraph_name = @paragraph.name
+            @chapter.update_attributes :name => 'ch_5'
+            Timecop.travel 1.second.since
+            @paragraph.update_attributes :name => 'para3'
+            Timecop.travel 1.second.since
+            @chapter_4 = @chapter.versions.last.reify(:has_many => true)
+          end
+
+          should "to the version before the change was made" do
+            assert_equal [@initial_section_name], @chapter_4.sections.map(&:name)
+            assert_equal [@initial_paragraph_name], @chapter_4.sections.first.paragraphs.map(&:name)
+          end
+        end
+
+        context "and the first has_many through relationship is destroyed" do
+          setup do
+            @section.destroy
+            Timecop.travel 1.second.since
+            @chapter.update_attributes(:name => 'chapter 6')
+            Timecop.travel 1.second.since
+            @chapter_before = @chapter.versions.last.reify(:has_many => true)
+          end
+
+          should "reify should not return any associated models" do
+            assert_equal 0, @chapter_before.sections.size
+            assert_equal 0, @chapter_before.paragraphs.size
+          end
+        end
+
+        context "reified to the version before the nested has_many through relationship is destroyed" do
+          setup do
+            Timecop.travel 1.second.since
+            @initial_paragraph_name = @section.paragraphs.first.name
+            @chapter.update_attributes(:name => 'chapter 6')
+            Timecop.travel 1.second.since
+            @paragraph.destroy
+            @chapter_before = @chapter.versions.last.reify(:has_many => true)
+          end
+
+          should "restore the associated has_many relationship" do
+            assert_equal [@initial_paragraph_name], @chapter_before.sections.first.paragraphs.map(&:name)
+          end
+        end
+
+        context "reified to the version after the nested has_many through relationship is destroyed" do
+          setup do
+            @paragraph.destroy
+            Timecop.travel 1.second.since
+            @chapter.update_attributes(:name => 'chapter 6')
+            @chapter_before = @chapter.versions.last.reify(:has_many => true)
+          end
+
+          should "restore the associated has_many relationship" do
+            assert_equal [], @chapter_before.sections.first.paragraphs
+          end
+        end
+      end
+    end
+
+    context "a chapter with one paragraph and one citation" do
+      should "reify paragraphs and citations" do
+        chapter = Chapter.create(:name => 'Chapter One')
+        section = Section.create(:name => 'Section One', :chapter => chapter)
+        paragraph = Paragraph.create(:name => 'Paragraph One', :section => section)
+        quotation = Quotation.create(:chapter => chapter)
+        citation = Citation.create(:quotation => quotation)
+        Timecop.travel 1.second.since
+        chapter.update_attributes(:name => 'Chapter One, Vers. Two')
+        assert_equal 2, chapter.versions.count
+        paragraph.destroy
+        citation.destroy
+        reified = chapter.versions[1].reify(:has_many => true)
+        assert_equal [paragraph], reified.sections.first.paragraphs
+        assert_equal [citation], reified.quotations.first.citations
+      end
+    end
+  end
 end