diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index 052d37dee..296c2434f 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -177,7 +177,7 @@ def originator def version_at(timestamp, reify_options={}) # Because a version stores how its object looked *before* the change, # we need to look for the first version created *after* the timestamp. - v = send(self.class.versions_association_name).subsequent(timestamp).first + v = send(self.class.versions_association_name).subsequent(timestamp, true).first v ? v.reify(reify_options) : self end diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index d5a62e05e..f45bb1e8a 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -33,22 +33,43 @@ def not_creates where 'event <> ?', 'create' end - # These methods accept a timestamp or a version and returns other versions that come before or after - def subsequent(obj) + # Expects `obj` to be an instance of `PaperTrail::Version` by default, but can accept a timestamp if + # `timestamp_arg` receives `true` + def subsequent(obj, timestamp_arg = false) + if timestamp_arg != true && self.primary_key_is_int? + return where("#{table_name}.#{self.primary_key} > ?", obj).order("#{table_name}.#{self.primary_key} ASC") + end + obj = obj.send(PaperTrail.timestamp_field) if obj.is_a?(self) where("#{table_name}.#{PaperTrail.timestamp_field} > ?", obj). - order("#{table_name}.#{PaperTrail.timestamp_field} ASC") + order(self.timestamp_sort_order) end - def preceding(obj) + def preceding(obj, timestamp_arg = false) + if timestamp_arg != true && self.primary_key_is_int? + return where("#{table_name}.#{self.primary_key} < ?", obj).order("#{table_name}.#{self.primary_key} DESC") + end + obj = obj.send(PaperTrail.timestamp_field) if obj.is_a?(self) where("#{table_name}.#{PaperTrail.timestamp_field} < ?", obj). - order("#{table_name}.#{PaperTrail.timestamp_field} DESC") + order(self.timestamp_sort_order('DESC')) end + def between(start_time, end_time) where("#{table_name}.#{PaperTrail.timestamp_field} > ? AND #{table_name}.#{PaperTrail.timestamp_field} < ?", - start_time, end_time).order("#{table_name}.#{PaperTrail.timestamp_field} ASC") + start_time, end_time).order(self.timestamp_sort_order) + end + + # defaults to using the primary key as the secondary sort order if possible + def timestamp_sort_order(order = 'ASC') + self.primary_key_is_int? ? + "#{table_name}.#{PaperTrail.timestamp_field} #{order}, #{table_name}.#{self.primary_key} #{order}" : + "#{table_name}.#{PaperTrail.timestamp_field} #{order}" + end + + def primary_key_is_int? + @primary_key_is_int ||= columns_hash[primary_key].type == :integer end # Returns whether the `object` column is using the `json` type supported by PostgreSQL diff --git a/test/unit/version_test.rb b/test/unit/version_test.rb index b92c6a190..8022d9d3f 100644 --- a/test/unit/version_test.rb +++ b/test/unit/version_test.rb @@ -66,9 +66,9 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase context "receiving a TimeStamp" do should "return all versions that were created before the Timestamp; descendingly by order of the `PaperTrail.timestamp_field`" do - value = PaperTrail::Version.subsequent(1.hour.ago) + value = PaperTrail::Version.subsequent(1.hour.ago, true) assert_equal value, @animal.versions.to_a - assert_not_nil value.to_sql.match(/ORDER BY versions.created_at ASC\z/) + assert_not_nil value.to_sql.match(/ORDER BY versions.created_at ASC/) end end @@ -76,10 +76,6 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase should "grab the Timestamp from the version and use that as the value" do value = PaperTrail::Version.subsequent(@animal.versions.first) assert_equal value, @animal.versions.to_a.tap { |assoc| assoc.shift } - # This asssertion can't pass in Ruby18 because the `strftime` method doesn't accept the %6 (milliseconds) command - if RUBY_VERSION.to_f >= 1.9 and not defined?(JRUBY_VERSION) - assert_not_nil value.to_sql.match(/WHERE \(versions.created_at > '#{@animal.versions.first.send(PaperTrail.timestamp_field).strftime("%F %T.%6N")}'\)/) - end end end end @@ -89,9 +85,9 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase context "receiving a TimeStamp" do should "return all versions that were created before the Timestamp; descendingly by order of the `PaperTrail.timestamp_field`" do - value = PaperTrail::Version.preceding(Time.now) + value = PaperTrail::Version.preceding(Time.now, true) assert_equal value, @animal.versions.reverse - assert_not_nil value.to_sql.match(/ORDER BY versions.created_at DESC\z/) + assert_not_nil value.to_sql.match(/ORDER BY versions.created_at DESC/) end end @@ -99,10 +95,6 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase should "grab the Timestamp from the version and use that as the value" do value = PaperTrail::Version.preceding(@animal.versions.last) assert_equal value, @animal.versions.to_a.tap { |assoc| assoc.pop }.reverse - # This asssertion can't pass in Ruby18 because the `strftime` method doesn't accept the %6 (milliseconds) command - if RUBY_VERSION.to_f >= 1.9 and not defined?(JRUBY_VERSION) - assert_not_nil value.to_sql.match(/WHERE \(versions.created_at < '#{@animal.versions.last.send(PaperTrail.timestamp_field).strftime("%F %T.%6N")}'\)/) - end end end end