Merge pull request #993 from airblade/where_object_changes_on_text

where_object_changes no longer supports reading json from a text column

.github/CONTRIBUTING.md

@@ -105,9 +105,9 @@ DB=postgres bundle exec rake prepare
 # See spec/dummy_app/config/boot.rb for a complete explanation.
 cd spec/dummy_app
 export BUNDLE_GEMFILE=../../gemfiles/ar_5.0.gemfile
-DB=postgres RAILS_ENV=test bundle exec rake db:drop db:create db:migrate
-DB=postgres RAILS_ENV=foo bundle exec rake db:drop db:create db:migrate
-DB=postgres RAILS_ENV=bar bundle exec rake db:drop db:create db:migrate
+DB=postgres RAILS_ENV=test bundle exec rake db:environment:set db:drop db:create db:migrate
+DB=postgres RAILS_ENV=foo bundle exec rake db:environment:set db:drop db:create db:migrate
+DB=postgres RAILS_ENV=bar bundle exec rake db:environment:set db:drop db:create db:migrate
 unset BUNDLE_GEMFILE
 cd ../..
 

CHANGELOG.md

@@ -7,7 +7,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Breaking Changes
 
-- None
+- [#803](https://github.com/airblade/paper_trail/issues/803) -
+  where_object_changes no longer supports reading json from a text column
 
 ### Added
 

lib/paper_trail/serializers/json.rb

@@ -4,13 +4,6 @@ module PaperTrail
   module Serializers
     # An alternate serializer for, e.g. `versions.object`.
     module JSON
-      E_WHERE_OBJ_CHANGES = <<-STR.squish.freeze
-        where_object_changes has a known issue. When reading json from a text
-        column, it may return more records than expected. Instead of a warning,
-        this method may raise an error in the future. Please join the discussion
-        at https://github.com/airblade/paper_trail/issues/803
-      STR
-
       extend self # makes all instance methods become module methods as well
 
       def load(string)
@@ -39,17 +32,14 @@ module PaperTrail
         end
       end
 
-      # Returns a SQL LIKE condition to be used to match the given field and
-      # value in the serialized `object_changes`.
-      def where_object_changes_condition(arel_field, field, value)
-        ::ActiveSupport::Deprecation.warn(E_WHERE_OBJ_CHANGES)
-
-        # Convert to JSON to handle strings and nulls correctly.
-        json_value = value.to_json
-
-        # Need to check first (before) and secondary (after) fields
-        arel_field.matches("%\"#{field}\":[#{json_value},%").
-          or(arel_field.matches("%\"#{field}\":[%,#{json_value}]%"))
+      def where_object_changes_condition(*)
+        raise <<-STR.squish.freeze
+          where_object_changes no longer supports reading json from a text
+          column. The old implementation was inaccurate, returning more records
+          than you wanted. This feature was deprecated in 7.1.0 and removed in
+          8.0.0. The json and jsonb datatypes are still supported. See the
+          discussion at https://github.com/airblade/paper_trail/issues/803
+        STR
       end
     end
   end

spec/models/version_spec.rb

@@ -105,10 +105,10 @@ module PaperTrail
         column_overrides << "jsonb" if ::ActiveRecord::VERSION::STRING >= "4.2"
       end
 
-      column_overrides.shuffle.each do |override|
-        context "with a #{override || 'text'} column" do
+      column_overrides.shuffle.each do |column_datatype_override|
+        context "with a #{column_datatype_override || 'text'} column" do
           before do
-            if override
+            if column_datatype_override
               # In rails < 5, we use truncation, ie. there is no transaction
               # around the tests, so we can't use a savepoint.
               if active_record_gem_version >= ::Gem::Version.new("5")
@@ -119,7 +119,7 @@ module PaperTrail
                   "ALTER TABLE versions DROP COLUMN #{column};"
                 )
                 ActiveRecord::Base.connection.execute(
-                  "ALTER TABLE versions ADD COLUMN #{column} #{override};"
+                  "ALTER TABLE versions ADD COLUMN #{column} #{column_datatype_override};"
                 )
               end
               PaperTrail::Version.reset_column_information
@@ -127,7 +127,7 @@ module PaperTrail
           end
 
           after do
-            if override
+            if column_datatype_override
               # In rails < 5, we use truncation, ie. there is no transaction
               # around the tests, so we can't use a savepoint.
               if active_record_gem_version >= ::Gem::Version.new("5")
@@ -245,35 +245,33 @@ module PaperTrail
               end
             end
 
-            context "JSON serializer" do
-              before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON }
-              before do
-                unless override
-                  expect(::ActiveSupport::Deprecation).to(
-                    receive(:warn).at_least(:once).with(/^where_object_changes/)
-                  )
+            # Only test the JSON serializer against where_object_changes
+            # for json and jsonb columns, not for text columns, which are
+            # no longer supported.
+            if column_datatype_override
+              context "JSON serializer" do
+                before(:all) { PaperTrail.serializer = PaperTrail::Serializers::JSON }
+
+                it "locates versions according to their `object_changes` contents" do
+                  expect(
+                    widget.versions.where_object_changes(name: name)
+                  ).to eq(widget.versions[0..1])
+                  expect(
+                    widget.versions.where_object_changes(an_integer: 77)
+                  ).to eq(widget.versions[1..2])
+                  expect(
+                    widget.versions.where_object_changes(an_integer: int)
+                  ).to eq([widget.versions.last])
                 end
-              end
 
-              it "locates versions according to their `object_changes` contents" do
-                expect(
-                  widget.versions.where_object_changes(name: name)
-                ).to eq(widget.versions[0..1])
-                expect(
-                  widget.versions.where_object_changes(an_integer: 77)
-                ).to eq(widget.versions[1..2])
-                expect(
-                  widget.versions.where_object_changes(an_integer: int)
-                ).to eq([widget.versions.last])
-              end
+                it "handles queries for multiple attributes" do
+                  expect(
+                    widget.versions.where_object_changes(an_integer: 77, name: "foobar")
+                  ).to eq(widget.versions[1..2])
+                end
 
-              it "handles queries for multiple attributes" do
-                expect(
-                  widget.versions.where_object_changes(an_integer: 77, name: "foobar")
-                ).to eq(widget.versions[1..2])
+                after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML }
               end
-
-              after(:all) { PaperTrail.serializer = PaperTrail::Serializers::YAML }
             end
           end
         end

spec/paper_trail/serializers/json_spec.rb

@@ -52,6 +52,14 @@ module PaperTrail
           end
         end
       end
+
+      describe ".where_object_changes_condition" do
+        it "raises error" do
+          expect {
+            described_class.where_object_changes_condition
+          }.to raise_error(/no longer supports/)
+        end
+      end
     end
   end
 end