diff --git a/README.md b/README.md index c99e36f45..34ea6e951 100644 --- a/README.md +++ b/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 diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index ad9f0d7d4..d9954dd80 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -111,17 +111,6 @@ def next_version 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) diff --git a/lib/paper_trail/version.rb b/lib/paper_trail/version.rb index 288c9d847..507bf9860 100644 --- a/lib/paper_trail/version.rb +++ b/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 @@ def reify 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 @@ def index 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 diff --git a/test/paper_trail_model_test.rb b/test/paper_trail_model_test.rb index 73a7d0a2a..8675c34e2 100644 --- a/test/paper_trail_model_test.rb +++ b/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