Attempt to sort based on primary key in scope methods on VersionConce…

…rn if primary key is an integer.

This makes some of the changes to these scope methods that came from 6a4aba2 more flexible,
in that the user can choose to compare timestamps if desired, but it defaults to comparing and sorting,
via the primary key (if it is an integer).  If the primary key is not an integer, it still defaults
to using the PaperTrail.timestamp_field.

This is my proposed fix for #314, and I also believe it should fix #317. It seems that that this issue
is usually encountered when testing PaperTrail with MySQL (presumably due to lack of microsecond timestamp support).

lib/paper_trail/has_paper_trail.rb

@@ -181,7 +181,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
 

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

test/unit/version_test.rb

@@ -66,20 +66,16 @@ 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
 
     context "receiving a `PaperTrail::Version`" do
       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,20 +85,16 @@ 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
 
     context "receiving a `PaperTrail::Version`" do
       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