Explicitly specify table name for scope methods on VersonConcern. close #295

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 3.0.0 (Unreleased)
 
+  - [#295](https://github.com/airblade/paper_trail/issues/295) - Explicitly specify table name for version class when
+    querying attributes. Prevents `AmbiguousColumn` errors on certain `JOIN` statements.
   - [#289](https://github.com/airblade/paper_trail/pull/289) - Use `ActiveSupport::Concern` for implementation of base functionality on
     `PaperTrail::Version` class. Increases flexibility and makes it easier to use custom version classes with multiple `ActiveRecord` connections.
   - [#288](https://github.com/airblade/paper_trail/issues/288) - Change all scope declarations to class methods on the `PaperTrail::Version`

lib/paper_trail/version.rb

@@ -35,17 +35,19 @@ module PaperTrail
       # These methods accept a timestamp or a version and returns other versions that come before or after
       def subsequent(obj)
         obj = obj.send(PaperTrail.timestamp_field) if obj.is_a?(self)
-        where("#{PaperTrail.timestamp_field} > ?", obj).order("#{PaperTrail.timestamp_field} ASC")
+        where("#{table_name}.#{PaperTrail.timestamp_field} > ?", obj).
+          order("#{table_name}.#{PaperTrail.timestamp_field} ASC")
       end
 
       def preceding(obj)
         obj = obj.send(PaperTrail.timestamp_field) if obj.is_a?(self)
-        where("#{PaperTrail.timestamp_field} < ?", obj).order("#{PaperTrail.timestamp_field} DESC")
+        where("#{table_name}.#{PaperTrail.timestamp_field} < ?", obj).
+          order("#{table_name}.#{PaperTrail.timestamp_field} DESC")
       end
 
       def between(start_time, end_time)
-        where("#{PaperTrail.timestamp_field} > ? AND #{PaperTrail.timestamp_field} < ?", start_time, end_time).
-          order("#{PaperTrail.timestamp_field} ASC")
+        where("#{table_name}.#{PaperTrail.timestamp_field} > ? AND #{table_name}.#{PaperTrail.timestamp_field} < ?",
+          start_time, end_time).order("#{table_name}.#{PaperTrail.timestamp_field} ASC")
       end
 
       # Returns whether the `object` column is using the `json` type supported by PostgreSQL
@@ -163,8 +165,10 @@ module PaperTrail
     end
 
     def index
-      @index ||= sibling_versions.select([PaperTrail.timestamp_field, self.class.primary_key.to_sym]).
-        order("#{PaperTrail.timestamp_field} ASC").index(self)
+      table_name = self.class.table_name
+      @index ||= sibling_versions.
+        select(["#{table_name}.#{PaperTrail.timestamp_field}", "#{table_name}.#{self.class.primary_key}"]).
+        order("#{table_name}.#{PaperTrail.timestamp_field} ASC").index(self)
     end
 
     private

spec/models/joined_version_spec.rb

@@ -0,0 +1,47 @@
+require 'spec_helper'
+
+describe JoinedVersion, :versioning => true do
+  it { JoinedVersion.superclass.should == PaperTrail::Version }
+
+  let(:widget) { Widget.create!(:name => Faker::Name.name) }
+  let(:version) { JoinedVersion.first }
+
+  describe "Scopes" do
+    describe "default_scope" do
+      it { JoinedVersion.default_scopes.should_not be_empty }
+    end
+
+    describe "VersionConcern::ClassMethods" do
+      before { widget } # persist a widget
+
+      describe :subsequent do
+        it "shouldn't error out when there is a default_scope that joins" do
+          JoinedVersion.subsequent(version).first
+        end
+      end
+
+      describe :preceding do
+        it "shouldn't error out when there is a default scope that joins" do
+          JoinedVersion.preceding(version).first
+        end
+      end
+
+      describe :between do
+        it "shouldn't error out when there is a default scope that joins" do
+          JoinedVersion.between(Time.now, 1.minute.from_now).first
+        end
+      end
+    end
+  end
+
+  describe "Methods" do
+    describe :index do
+      it { should respond_to(:index) }
+
+      it "shouldn't error out when there is a default scope that joins" do
+        widget # persist a widget
+        version.index
+      end
+    end
+  end
+end

test/dummy/app/versions/joined_version.rb

@@ -0,0 +1,5 @@
+# The purpose of this custom version class is to test the scope methods on the VersionConcern::ClassMethods
+# module. See https://github.com/airblade/paper_trail/issues/295 for more details.
+class JoinedVersion < PaperTrail::Version
+  default_scope { joins('INNER JOIN widgets ON widgets.id = versions.item_id') }
+end

test/unit/version_test.rb

@@ -62,7 +62,7 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase
       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)
         assert_equal value, @animal.versions.to_a
-        assert_not_nil value.to_sql.match(/ORDER BY created_at ASC\z/)
+        assert_not_nil value.to_sql.match(/ORDER BY versions.created_at ASC\z/)
       end
     end
 
@@ -72,7 +72,7 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase
         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 \(created_at > '#{@animal.versions.first.send(PaperTrail.timestamp_field).strftime("%F %T.%6N")}'\)/)
+          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
@@ -85,7 +85,7 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase
       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)
         assert_equal value, @animal.versions.reverse
-        assert_not_nil value.to_sql.match(/ORDER BY created_at DESC\z/)
+        assert_not_nil value.to_sql.match(/ORDER BY versions.created_at DESC\z/)
       end
     end
 
@@ -95,7 +95,7 @@ class PaperTrail::VersionTest < ActiveSupport::TestCase
         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 \(created_at < '#{@animal.versions.last.send(PaperTrail.timestamp_field).strftime("%F %T.%6N")}'\)/)
+          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