Refactoring the Cleaner module to have condensed and more readable code, and better tests

lib/paper_trail/cleaner.rb

@@ -1,55 +1,27 @@
 module PaperTrail
   module Cleaner
-    def gather_all_versions
-      Version.all.group_by(&:item_id)
-    end
 
-    def get_all_keys
-      @versions.keys
-    end
-
-    def grouping_for_key(key)
-      @versions[key].group_by(&:grouping_by_date)
-    end
-
-    def sanitize(group)
-      group = keep_versions(group)
-      if group.size > 0
-        group.each do |member| 
-          member.destroy
+    # Deletes all versions that were created on a given date except the last `keep_int` versions.
+    # `date` argument should receive either an object of type `Date` or `:all` (which will clean versions on all dates).
+    def clean_versions!(keep_int = 1, date = :all)
+      version_hash = gather_versions(date)
+      version_hash.each do |item_id, versions|
+        grouping_by_date = versions.group_by { |v| v.created_at.to_date } # now group the versions by date
+        grouping_by_date.each do |date, versions|
+          versions.pop(keep_int)
+          versions.map(&:destroy)
         end
       end
     end
 
-    def keep_versions(group)
-      @keeping_versions.times do
-        group.pop
-      end
-      group
+    private
+
+    # Returns a hash of versions in this format: {:item_id => PaperTrail::Version}
+    def gather_versions(date)
+      raise "`date` argument must receive a Timestamp or `:all`" unless date == :all || date.respond_to?(:to_time)
+      versions = date == :all ? Version.all : Version.between(date, date+1.day)
+      versions.group_by(&:item_id)
     end
 
-    def analyze_grouping(grouping)
-      grouping.each_value do |group|
-        sanitize(group)
-      end
-    end
-
-    def acquire_version_info
-      @versions = gather_all_versions
-      @keys = get_all_keys
-    end
-
-    def examine_and_clean_versions
-      @keys.each do |key|
-        grouping = grouping_for_key(key)
-        analyze_grouping(grouping)
-      end
-    end 
-
-    def clean_paper_trail_versions(keeping = 1)
-      @keeping_versions = keeping
-      acquire_version_info
-      examine_and_clean_versions
-    end
   end
 end

lib/paper_trail/version.rb

@@ -150,10 +150,6 @@ module PaperTrail
       sibling_versions.select(id_column).order("#{id_column} ASC").map(&id_column).index(self.send(id_column))
     end
 
-    def grouping_by_date
-      created_at.to_date.to_s(:db)
-    end
-
     private
 
     # In Rails 3.1+, calling reify on a previous version confuses the

test/unit/cleaner_test.rb

@@ -2,7 +2,7 @@ require 'test_helper'
 
 class PaperTrailCleanerTest < ActiveSupport::TestCase
 
-  test 'Baseline' do
+  setup do
     @animal = Animal.create :name => 'Animal'
     @animal.update_attributes :name => 'Animal from the Muppets'
     @animal.update_attributes :name => 'Animal Muppet'
@@ -14,48 +14,55 @@ class PaperTrailCleanerTest < ActiveSupport::TestCase
     @cat = Cat.create :name => 'Garfield'
     @cat.update_attributes :name => 'Garfield (I hate Mondays)'
     @cat.update_attributes :name => 'Garfield The Cat'
+    @animals = [@animal, @dog, @cat]
+  end
+
+  test 'Baseline' do
     assert_equal 9, PaperTrail::Version.count
   end
 
-  test 'cleaner removes extra versions' do
-    @animal = Animal.create :name => 'Animal'
-    @animal.update_attributes :name => 'Animal from the Muppets'
-    @animal.update_attributes :name => 'Animal Muppet'
-    PaperTrail.clean_paper_trail_versions
-    assert_equal 1, PaperTrail::Version.all.count
-  end
+  context 'Cleaner' do
+    context '`clean_versions!` method' do
 
-  test 'cleaner removes versions' do
-    @animal = Animal.create :name => 'Animal'
-    @animal.update_attributes :name => 'Animal from the Muppets'
-    @animal.update_attributes :name => 'Animal Muppet'
+      should 'removes extra versions for each item' do
+        PaperTrail.clean_versions!
+        assert_equal 3, PaperTrail::Version.count
+        @animals.each { |animal| assert_equal 1, animal.versions.size }
+      end
 
-    @dog = Dog.create :name => 'Snoopy'
-    @dog.update_attributes :name => 'Scooby'
-    @dog.update_attributes :name => 'Scooby Doo'
+      should 'removes the earliest version(s)' do
+        most_recent_version_names = @animals.map { |animal| animal.versions.last.reify.name }
+        PaperTrail.clean_versions!
+        assert_equal most_recent_version_names, @animals.map { |animal| animal.versions.last.reify.name }
+      end
 
-    @cat = Cat.create :name => 'Garfield'
-    @cat.update_attributes :name => 'Garfield (I hate Mondays)'
-    @cat.update_attributes :name => 'Garfield The Cat'
-    PaperTrail.clean_paper_trail_versions
-    assert_equal 3, PaperTrail::Version.all.count
-  end
+      context '`keep_int` argument' do
+        should 'modifies the number of versions ommitted from destruction' do
+          PaperTrail.clean_versions!(2)
+          assert_equal 6, PaperTrail::Version.all.count
+          @animals.each { |animal| assert_equal 2, animal.versions.size }
+        end
+      end
 
-  test 'cleaner keeps the correct (last) version' do
-    @animal = Animal.create :name => 'Animal'
-    @animal.update_attributes :name => 'Animal from the Muppets'
-    @animal.update_attributes :name => 'Animal Muppet'
-    PaperTrail.clean_paper_trail_versions
-    assert_equal 1, PaperTrail::Version.all.count
-    assert_equal "Animal Muppet", @animal.name
-  end
+      context '`date` argument' do
+        setup do
+          @animal.versions.each { |ver| ver.update_attribute(:created_at, ver.created_at - 1.day) }
+          @date = @animal.versions.first.created_at.to_date
+          @animal.update_attribute(:name, 'Muppet')
+        end
+
+        should 'restrict the version destroyed to those that were created on the date provided' do
+          assert_equal 10, PaperTrail::Version.count
+          assert_equal 4, @animal.versions.size
+          assert_equal 3, @animal.versions_between(@date, @date + 1.day).size
+          PaperTrail.clean_versions!(1, @date)
+          assert_equal 8, PaperTrail::Version.count
+          assert_equal 2, @animal.versions(true).size
+          assert_equal @date, @animal.versions.first.created_at.to_date
+          assert_equal @date + 1.day, @animal.versions.last.created_at.to_date
+        end
+      end
+    end
 
-  test 'cleaner accepts variable arguments' do
-    @animal = Animal.create :name => 'Animal'
-    @animal.update_attributes :name => 'Animal from the Muppets'
-    @animal.update_attributes :name => 'Animal Muppet'
-    PaperTrail.clean_paper_trail_versions(2)
-    assert_equal 2, PaperTrail::Version.all.count
-    assert_equal "Animal Muppet", @animal.name
   end
 end