diff --git a/CHANGELOG.md b/CHANGELOG.md index 397f26f8..ca258bcd 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/lib/paper_trail/version.rb b/lib/paper_trail/version.rb index 7e5abeab..6f088e70 100644 --- a/lib/paper_trail/version.rb +++ b/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 diff --git a/spec/models/joined_version_spec.rb b/spec/models/joined_version_spec.rb new file mode 100644 index 00000000..79f18a38 --- /dev/null +++ b/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 diff --git a/test/dummy/app/versions/joined_version.rb b/test/dummy/app/versions/joined_version.rb new file mode 100644 index 00000000..c8a81a12 --- /dev/null +++ b/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 diff --git a/test/unit/version_test.rb b/test/unit/version_test.rb index 50eb3fab..5d0186e6 100644 --- a/test/unit/version_test.rb +++ b/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