Merge pull request #701 from owenr/fix-cleaner

Default order by timestamp to ensure early versions are cleaned

CHANGELOG.md

@@ -19,7 +19,10 @@ None
 
 ### Fixed
 
-None
+- [#701](https://github.com/airblade/paper_trail/pull/701) /
+  [#699](https://github.com/airblade/paper_trail/issues/699) -
+  Cleaning old versions explicitly preserves the most recent
+  versions instead of relying on database result ordering.
 
 ## 4.1.0 (Unreleased)
 

lib/paper_trail/cleaner.rb

@@ -6,7 +6,8 @@ module Cleaner
     # Options:
     #
     # - :keeping - An `integer` indicating the number of versions to be kept for
-    #   each item per date. Defaults to `1`.
+    #   each item per date. Defaults to `1`. The most recent matching versions
+    #   are kept.
     # - :date - Should either be a `Date` object specifying which date to
     #   destroy versions for or `:all`, which will specify that all dates
     #   should be cleaned. Defaults to `:all`.
@@ -30,12 +31,14 @@ def clean_versions!(options = {})
     # Returns a hash of versions grouped by the `item_id` attribute formatted
     # like this: {:item_id => PaperTrail::Version}. If `item_id` or `date` is
     # set, versions will be narrowed to those pointing at items with those ids
-    # that were created on specified date.
+    # that were created on specified date. Versions are returned in
+    # chronological order.
     def gather_versions(item_id = nil, date = :all)
       unless date == :all || date.respond_to?(:to_date)
         raise ArgumentError.new("`date` argument must receive a Timestamp or `:all`")
       end
       versions = item_id ? PaperTrail::Version.where(:item_id => item_id) : PaperTrail::Version
+      versions = versions.order(PaperTrail.timestamp_field, :id)
       versions = versions.between(date.to_date, date.to_date + 1.day) unless date == :all
 
       # If `versions` has not been converted to an ActiveRecord::Relation yet,