Rails 5.2 support, drop Rails 4.2 support. (#887)

.travis.yml

@@ -14,9 +14,9 @@ env:
   - DB=postgresql
 
 gemfile:
+  - gemfiles/activerecord_5.2.gemfile
   - gemfiles/activerecord_5.1.gemfile
   - gemfiles/activerecord_5.0.gemfile
-  - gemfiles/activerecord_4.2.gemfile
 
 sudo: false
 

Appraisals

@@ -1,11 +1,11 @@
+appraise 'activerecord-5.2' do
+  gem 'activerecord', '~> 5.2.0'
+end
+
 appraise 'activerecord-5.1' do
   gem 'activerecord', "~> 5.1.1"
 end
 
 appraise 'activerecord-5.0' do
   gem 'activerecord', "~> 5.0.3"
 end
-
-appraise "activerecord-4.2" do
-  gem "activerecord", "~> 4.2.8"
-end

acts-as-taggable-on.gemspec

@@ -22,7 +22,7 @@ Gem::Specification.new do |gem|
     gem.post_install_message = File.read('UPGRADING.md')
   end
 
-  gem.add_runtime_dependency 'activerecord', ['>= 4.2.8']
+  gem.add_runtime_dependency 'activerecord', ['~> 5.0']
 
   gem.add_development_dependency 'sqlite3'
   gem.add_development_dependency 'mysql2', '~> 0.3'

gemfiles/activerecord_4.2.gemfile → gemfiles/activerecord_5.2.gemfile

@@ -2,9 +2,7 @@
 
 source "https://rubygems.org"
 
-gem "activerecord", "~> 4.2.8"
-gem "pg", "~> 0.18"
-gem "mysql2", "< 0.5"
+gem "activerecord", "~> 5.2.0"
 
 group :local_development do
   gem "guard"

lib/acts_as_taggable_on/taggable.rb

@@ -96,7 +96,6 @@ def self.taggable?
       include Cache
       include Ownership
       include Related
-      include Dirty
     end
   end
 end

lib/acts_as_taggable_on/taggable/collection.rb

@@ -174,7 +174,7 @@ module CalculationMethods
       # See https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/calculations.rb#L38
       def count(column_name = :all, options = {})
         # https://github.com/rails/rails/commit/da9b5d4a8435b744fcf278fffd6d7f1e36d4a4f2
-        ActsAsTaggableOn::Utils.active_record5? ? super(column_name) : super(column_name, options)
+        super(column_name)
       end
     end
   end

lib/acts_as_taggable_on/taggable/core.rb

@@ -2,6 +2,7 @@
 
 module ActsAsTaggableOn::Taggable
   module Core
+
     def self.included(base)
       base.extend ActsAsTaggableOn::Taggable::Core::ClassMethods
 
@@ -28,12 +29,16 @@ def initialize_acts_as_taggable_on_core
             has_many context_taggings, -> { includes(:tag).order(taggings_order).where(context: tags_type) },
                      as: :taggable,
                      class_name: 'ActsAsTaggableOn::Tagging',
-                     dependent: :destroy
+                     dependent: :destroy,
+                     after_add: :dirtify_tag_list,
+                     after_remove: :dirtify_tag_list
 
             has_many context_tags, -> { order(taggings_order) },
                      class_name: 'ActsAsTaggableOn::Tag',
                      through: context_taggings,
                      source: :tag
+
+            attribute "#{tags_type.singularize}_list".to_sym, ActiveModel::Type::Value.new
           end
 
           taggable_mixin.class_eval <<-RUBY, __FILE__, __LINE__ + 1
@@ -42,12 +47,24 @@ def #{tag_type}_list
             end
 
             def #{tag_type}_list=(new_tags)
+              parsed_new_list = ActsAsTaggableOn.default_parser.new(new_tags).parse
+
+              if self.class.preserve_tag_order? || parsed_new_list.sort != #{tag_type}_list.sort
+                set_attribute_was('#{tag_type}_list', #{tag_type}_list)
+                write_attribute("#{tag_type}_list", parsed_new_list)
+              end
+
               set_tag_list_on('#{tags_type}', new_tags)
             end
 
             def all_#{tags_type}_list
               all_tags_list_on('#{tags_type}')
             end
+
+            private
+            def dirtify_tag_list(tagging)
+              attribute_will_change! tagging.context.singularize+"_list"
+            end
           RUBY
         end
       end
@@ -161,7 +178,7 @@ def all_tags_on(context)
 
       if ActsAsTaggableOn::Utils.using_postgresql?
         group_columns = grouped_column_names_for(ActsAsTaggableOn::Tag)
-        scope.order("max(#{tagging_table_name}.created_at)").group(group_columns)
+        scope.order(Arel.sql("max(#{tagging_table_name}.created_at)")).group(group_columns)
       else
         scope.group("#{ActsAsTaggableOn::Tag.table_name}.#{ActsAsTaggableOn::Tag.primary_key}")
       end.to_a
@@ -181,33 +198,16 @@ def set_tag_list_on(context, new_list)
       add_custom_context(context)
 
       variable_name = "@#{context.to_s.singularize}_list"
-      process_dirty_object(context, new_list) unless custom_contexts.include?(context.to_s)
 
-      instance_variable_set(variable_name, ActsAsTaggableOn.default_parser.new(new_list).parse)
+      parsed_new_list = ActsAsTaggableOn.default_parser.new(new_list).parse
+
+      instance_variable_set(variable_name, parsed_new_list)
     end
 
     def tagging_contexts
       self.class.tag_types.map(&:to_s) + custom_contexts
     end
 
-    def process_dirty_object(context, new_list)
-      value = new_list.is_a?(Array) ? ActsAsTaggableOn::TagList.new(new_list) : new_list
-      attrib = "#{context.to_s.singularize}_list"
-
-      if changed_attributes.include?(attrib)
-        # The attribute already has an unsaved change.
-        old = changed_attributes[attrib]
-        @changed_attributes.delete(attrib) if old.to_s == value.to_s
-      else
-        old = tag_list_on(context)
-        if self.class.preserve_tag_order
-          @changed_attributes[attrib] = old if old.to_s != value.to_s
-        else
-          @changed_attributes[attrib] = old.to_s if old.sort != ActsAsTaggableOn.default_parser.new(value).parse.sort
-        end
-      end
-    end
-
     def reload(*args)
       self.class.tag_types.each do |context|
         instance_variable_set("@#{context.to_s.singularize}_list", nil)
@@ -315,3 +315,4 @@ def find_or_create_tags_from_list_with_context(tag_list, _context)
     end
   end
 end
+

lib/acts_as_taggable_on/taggable/tagged_with_query/any_tags_query.rb

@@ -3,7 +3,7 @@ class AnyTagsQuery < QueryBase
     def build
       taggable_model.select(all_fields)
                     .where(model_has_at_least_one_tag)
-                    .order(order_conditions)
+                    .order(Arel.sql(order_conditions))
                     .readonly(false)
     end
 

lib/acts_as_taggable_on/taggable/tagged_with_query/query_base.rb

@@ -48,7 +48,7 @@ def tags_match_type
 
     def escaped_tag(tag)
       tag = tag.downcase unless ActsAsTaggableOn.strict_case_match
-      ActsAsTaggableOn::Utils.escape_like(tag)
+      tag.gsub(/[!%_]/) { |x| '!' + x }
     end
 
     def adjust_taggings_alias(taggings_alias)

lib/acts_as_taggable_on/tagging.rb

@@ -4,7 +4,7 @@ class Tagging < ::ActiveRecord::Base #:nodoc:
     belongs_to :tag, class_name: '::ActsAsTaggableOn::Tag', counter_cache: ActsAsTaggableOn.tags_counter
     belongs_to :taggable, polymorphic: true
 
-    belongs_to :tagger, {polymorphic: true}.tap {|o| o.merge!(optional: true) if ActsAsTaggableOn::Utils.active_record5? }
+    belongs_to :tagger, {polymorphic: true}.tap {|o| o.merge!(optional: true) }
 
     scope :owned_by, ->(owner) { where(tagger: owner) }
     scope :not_owned, -> { where(tagger_id: nil, tagger_type: nil) }

lib/acts_as_taggable_on/utils.rb

@@ -20,21 +20,13 @@ def sha_prefix(string)
         Digest::SHA1.hexdigest(string)[0..6]
       end
 
-      def active_record5?
-        ::ActiveRecord::VERSION::MAJOR == 5
-      end
-
       def like_operator
         using_postgresql? ? 'ILIKE' : 'LIKE'
       end
 
       # escape _ and % characters in strings, since these are wildcards in SQL.
       def escape_like(str)
-        str.gsub(/[!%_]/) { |x| escape_replacement + x }
-      end
-
-      def escape_replacement
-        using_postgresql? ? '\\' : '!'
+        str.gsub(/[!%_]/) { |x| '!' + x }
       end
     end
   end

spec/acts_as_taggable_on/taggable/dirty_spec.rb → spec/acts_as_taggable_on/dirty_spec.rb

@@ -1,7 +1,7 @@
 # -*- encoding : utf-8 -*-
 require 'spec_helper'
 
-describe ActsAsTaggableOn::Taggable::Dirty do
+describe 'Dirty behavior of taggable objects' do
   context 'with un-contexted tags' do
     before(:each) do
       @taggable = TaggableModel.create(tag_list: 'awesome, epic')
@@ -14,19 +14,27 @@
       end
 
       it 'should show changes of dirty object' do
-        expect(@taggable.changes).to eq({'tag_list' => ['awesome, epic', ['one']]})
+        expect(@taggable.changes).to eq({'tag_list' => [['awesome', 'epic'], ['one']]})
       end
 
-      it 'flags tag_list as changed' do
-        expect(@taggable.tag_list_changed?).to be_truthy
+      it 'should show changes of freshly initialized dirty object' do
+        taggable = TaggableModel.find(@taggable.id)
+        taggable.tag_list = 'one'
+        expect(taggable.changes).to eq({'tag_list' => [['awesome', 'epic'], ['one']]})
+      end
+
+      if Rails.version >= "5.1"
+        it 'flags tag_list as changed' do
+          expect(@taggable.will_save_change_to_tag_list?).to be_truthy
+        end
       end
 
       it 'preserves original value' do
-        expect(@taggable.tag_list_was).to eq('awesome, epic')
+        expect(@taggable.tag_list_was).to eq(['awesome', 'epic'])
       end
 
       it 'shows what the change was' do
-        expect(@taggable.tag_list_change).to eq(['awesome, epic', ['one']])
+        expect(@taggable.tag_list_change).to eq([['awesome', 'epic'], ['one']])
       end
 
       context 'without order' do
@@ -90,23 +98,19 @@
       end
 
       it 'should show changes of dirty object' do
-        expect(@taggable.changes).to eq({'language_list' => ['awesome, epic', ['one']]})
+        expect(@taggable.changes).to eq({'language_list' => [['awesome', 'epic'], ['one']]})
       end
 
       it 'flags language_list as changed' do
         expect(@taggable.language_list_changed?).to be_truthy
       end
 
       it 'preserves original value' do
-        expect(@taggable.language_list_was).to eq('awesome, epic')
+        expect(@taggable.language_list_was).to eq(['awesome', 'epic'])
       end
 
       it 'shows what the change was' do
-        expect(@taggable.language_list_change).to eq(['awesome, epic', ['one']])
-      end
-
-      it 'shows what the changes were' do
-        expect(@taggable.language_list_changes).to eq(['awesome, epic', ['one']])
+        expect(@taggable.language_list_change).to eq([['awesome', 'epic'], ['one']])
       end
     end
 
@@ -123,5 +127,16 @@
         expect(@taggable.changes).to be_empty
       end
     end
+
+    context 'when language_list changed by association' do
+      let(:tag) { ActsAsTaggableOn::Tag.new(name: 'one') }
+
+      it 'flags language_list as changed' do
+        expect(@taggable.changes).to be_empty
+        @taggable.languages << tag
+        expect(@taggable.language_list_changed?).to be_truthy
+      end
+    end
+
   end
 end

spec/acts_as_taggable_on/utils_spec.rb

@@ -20,16 +20,4 @@
       expect(ActsAsTaggableOn::Utils.sha_prefix('puppies')).not_to eq(ActsAsTaggableOn::Utils.sha_prefix('kittens'))
     end
   end
-
-  describe '#escape_replacement' do
-    it 'should return ! when the adapter is not PostgreSQL' do
-      allow(ActsAsTaggableOn::Utils.connection).to receive(:adapter_name) { 'MySQL' }
-      expect(ActsAsTaggableOn::Utils.escape_replacement).to eq('!')
-    end
-
-    it 'should return \\ when the adapter is PostgreSQL' do
-      allow(ActsAsTaggableOn::Utils.connection).to receive(:adapter_name) { 'PostgreSQL' }
-      expect(ActsAsTaggableOn::Utils.escape_replacement).to eq('\\')
-    end
-  end
 end