Improve automatic reification of has_one associations.

This still isn't optimal but it should be good enough most of the time.
You can tweak it or turn it off if you like.

README.md

@@ -222,7 +222,7 @@ To find out who made a `version`'s object look that way, use `version.originator
 
 ## Has-One Associations
 
-PaperTrail automatically restores `:has_one` associations as they were at the time.
+PaperTrail automatically restores `:has_one` associations as they were at (actually, 3 seconds before) the time.
 
     class Treasure < ActiveRecord::Base
       has_one :location
@@ -238,9 +238,18 @@ PaperTrail automatically restores `:has_one` associations as they were at the ti
     >> t.amount                         # 100
     >> t.location.latitude              # 12.345
 
-Note if you update the parent and child record in one go (in the same database transaction), and later reify the parent as it was before that update, you may not get the child as it was before the update.  This is rather annoying.  The problem is that PaperTrail doesn't know which records in the `versions` table are "together", i.e. which were in the same database transaction.  So if the child's version record was created before the parent's, PaperTrail will think the parent was looking at the post-update version of the child when it (the parent) was updated.  The solution is to make PaperTrail aware of transactions, or perhaps requests/actions (e.g. [Efficiency's transaction ID middleware](http://github.com/efficiency20/ops_middleware/blob/master/lib/e20/ops/middleware/transaction_id_middleware.rb)).
-    
-Unfortunately PaperTrail doesn't auto-restore `:has_many` associations (I can't get it to work) or `:belongs_to` (I ran out of time looking at `:has_many`).
+The implementation is complicated by the edge case where the parent and child are updated in one go, e.g. in one web request or database transaction.  PaperTrail doesn't know about different models being updated "together", so you can't ask it definitively to get the child as it was before the joint parent-and-child update.
+
+The correct solution is to make PaperTrail aware of requests or transactions (c.f. [Efficiency's transaction ID middleware](http://github.com/efficiency20/ops_middleware/blob/master/lib/e20/ops/middleware/transaction_id_middleware.rb)).  In the meantime we work around the problem by finding the child as it was a few seconds before the parent was updated.  By default we go 3 seconds before but you can change this by passing the `:has_one` option to `reify`:
+
+    >> t = treasure.versions.last.reify(:has_one => 1)       # look back 1 second instead of 3
+
+If you are shuddering, take solace from knowing you can opt out of these shenanigans:
+
+    >> t = treasure.versions.last.reify(:has_one => false)   # I say no to "workarounds"!
+
+Opting out means your `:has_one` associated objects will be the live ones, not the ones the user saw at the time.  Since PaperTrail doesn't auto-restore `:has_many` associations (I can't get it to work) or `:belongs_to` (I ran out of time looking at `:has_many`), this at least makes your associations wrong consistently ;)
+
 
 
 ## Has-Many-Through Associations

lib/paper_trail/has_paper_trail.rb

@@ -111,17 +111,6 @@ module PaperTrail
         subsequent_version.reify if subsequent_version
       end
 
-      protected
-
-      # Returns the object (not a Version) as it was until the version record
-      # with the given id.
-      def version_until(id)
-        # Because a version stores how its object looked *before* the change,
-        # we need to look for the first version created *on or after* the id.
-        version = versions.first :conditions => ['id >= ?', id], :order => 'id ASC'
-        version ? version.reify : self
-      end
-
       private
 
       def merge_metadata(data)

lib/paper_trail/version.rb

@@ -2,7 +2,20 @@ class Version < ActiveRecord::Base
   belongs_to :item, :polymorphic => true
   validates_presence_of :event
 
-  def reify
+  # Restore the item from this version.
+  #
+  # This will automatically restore all :has_one associations as they were "at the time",
+  # if they are also being versioned by PaperTrail.  NOTE: this isn't always guaranteed
+  # to work so you can either change the lookback period (from the default 3 seconds) or
+  # opt out.
+  #
+  # Options:
+  # +:has_one+   set to `false` to opt out of has_one reification.
+  #              set to a float to change the lookback time (check whether your db supports
+  #              sub-second datetimes if you want them).
+  def reify(options = {})
+    options.reverse_merge! :has_one => 3
+
     unless object.nil?
       attrs = YAML::load object
 
@@ -36,10 +49,11 @@ class Version < ActiveRecord::Base
       end
 
       model.version = self
-      # Restore the model's has_one associations as they were when this version was
-      # superseded by the next (because that's what the user was looking at when they
-      # made the change).
-      reify_has_ones model
+
+      unless options[:has_one] == false
+        reify_has_ones model, options[:has_one]
+      end
+
       model
     end
   end
@@ -72,12 +86,22 @@ class Version < ActiveRecord::Base
 
   private
 
-  def reify_has_ones(model)
+  # Restore the `model`'s has_one associations as they were when this version was
+  # superseded by the next (because that's what the user was looking at when they
+  # made the change).
+  #
+  # The `lookback` sets how many seconds before the model's change we go.
+  def reify_has_ones(model, lookback)
     model.class.reflect_on_all_associations(:has_one).each do |assoc|
       child = model.send assoc.name
-      if child.respond_to? :version_until
-        if (version_until = child.version_until(id))
-          version_until.attributes.each do |k,v|
+      if child.respond_to? :version_at
+        # N.B. we use version of the child as it was `lookback` seconds before the parent was updated.
+        # Ideally we want the version of the child as it was just before the parent was updated...
+        # but until PaperTrail knows which updates are "together" (e.g. parent and child being
+        # updated on the same form), it's impossible to tell when the overall update started;
+        # and therefore impossible to know when "just before" was.
+        if (child_as_it_was = child.version_at(created_at - lookback.seconds))
+          child_as_it_was.attributes.each do |k,v|
             model.send(assoc.name).send "#{k}=", v rescue nil
           end
         else

test/paper_trail_model_test.rb

@@ -669,7 +669,7 @@ class HasPaperTrailModelTest < Test::Unit::TestCase
       end
 
       context 'when reified' do
-        setup { @widget_0 = @widget.versions.last.reify }
+        setup { @widget_0 = @widget.versions.last.reify(:has_one => 1) }
 
         should 'see the associated as it was at the time' do
           assert_nil @widget_0.wotsit
@@ -680,11 +680,13 @@ class HasPaperTrailModelTest < Test::Unit::TestCase
     context 'where the associated is created between model versions' do
       setup do
         @wotsit = @widget.create_wotsit :name => 'wotsit_0'
+        make_last_version_earlier @wotsit
+
         @widget.update_attributes :name => 'widget_1'
       end
 
       context 'when reified' do
-        setup { @widget_0 = @widget.versions.last.reify }
+        setup { @widget_0 = @widget.versions.last.reify(:has_one => 1) }
 
         should 'see the associated as it was at the time' do
           assert_equal 'wotsit_0', @widget_0.wotsit.name
@@ -694,27 +696,41 @@ class HasPaperTrailModelTest < Test::Unit::TestCase
       context 'and then the associated is updated between model versions' do
         setup do
           @wotsit.update_attributes :name => 'wotsit_1'
+          make_last_version_earlier @wotsit
           @wotsit.update_attributes :name => 'wotsit_2'
+          make_last_version_earlier @wotsit
+
           @widget.update_attributes :name => 'widget_2'
+          @wotsit.update_attributes :name => 'wotsit_3'
         end
 
         context 'when reified' do
-          setup { @widget_1 = @widget.versions.last.reify }
+          setup { @widget_1 = @widget.versions.last.reify(:has_one => 1) }
 
           should 'see the associated as it was at the time' do
             assert_equal 'wotsit_2', @widget_1.wotsit.name
           end
         end
+
+        context 'when reified opting out of has_one reification' do
+          setup { @widget_1 = @widget.versions.last.reify(:has_one => false) }
+
+          should 'see the associated as it is live' do
+            assert_equal 'wotsit_3', @widget_1.wotsit.name
+          end
+        end
       end
 
       context 'and then the associated is destroyed between model versions' do
         setup do
           @wotsit.destroy
+          make_last_version_earlier @wotsit
+
           @widget.update_attributes :name => 'widget_3'
         end
 
         context 'when reified' do
-          setup { @widget_2 = @widget.versions.last.reify }
+          setup { @widget_2 = @widget.versions.last.reify(:has_one => 1) }
 
           should 'see the associated as it was at the time' do
             assert_nil @widget_2.wotsit
@@ -724,4 +740,14 @@ class HasPaperTrailModelTest < Test::Unit::TestCase
     end
   end
 
+  private
+
+  # Updates `model`'s last version so it looks like the version was
+  # created 2 seconds ago.
+  def make_last_version_earlier(model)
+    Version.record_timestamps = false
+    model.versions.last.update_attributes :created_at => 2.seconds.ago
+    Version.record_timestamps = true
+  end
+
 end