diff --git a/.travis.yml b/.travis.yml index e2472b335..bd88b2b00 100644 --- a/.travis.yml +++ b/.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 diff --git a/Appraisals b/Appraisals index a206c726b..20a505a4a 100644 --- a/Appraisals +++ b/Appraisals @@ -1,3 +1,7 @@ +appraise 'activerecord-5.2' do + gem 'activerecord', '~> 5.2.0' +end + appraise 'activerecord-5.1' do gem 'activerecord', "~> 5.1.1" end @@ -5,7 +9,3 @@ end appraise 'activerecord-5.0' do gem 'activerecord', "~> 5.0.3" end - -appraise "activerecord-4.2" do - gem "activerecord", "~> 4.2.8" -end \ No newline at end of file diff --git a/acts-as-taggable-on.gemspec b/acts-as-taggable-on.gemspec index f652550de..534ab499c 100644 --- a/acts-as-taggable-on.gemspec +++ b/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' diff --git a/gemfiles/activerecord_4.2.gemfile b/gemfiles/activerecord_5.2.gemfile similarity index 77% rename from gemfiles/activerecord_4.2.gemfile rename to gemfiles/activerecord_5.2.gemfile index 54a829b0e..6b566da46 100644 --- a/gemfiles/activerecord_4.2.gemfile +++ b/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" diff --git a/lib/acts_as_taggable_on/taggable.rb b/lib/acts_as_taggable_on/taggable.rb index 62cc958e8..eb6328c03 100644 --- a/lib/acts_as_taggable_on/taggable.rb +++ b/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 diff --git a/lib/acts_as_taggable_on/taggable/collection.rb b/lib/acts_as_taggable_on/taggable/collection.rb index 786623fdc..26e277f24 100644 --- a/lib/acts_as_taggable_on/taggable/collection.rb +++ b/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 diff --git a/lib/acts_as_taggable_on/taggable/core.rb b/lib/acts_as_taggable_on/taggable/core.rb index 734d82aa1..c0d66b86e 100644 --- a/lib/acts_as_taggable_on/taggable/core.rb +++ b/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 + diff --git a/lib/acts_as_taggable_on/taggable/dirty.rb b/lib/acts_as_taggable_on/taggable/dirty.rb deleted file mode 100644 index d4f0dd57f..000000000 --- a/lib/acts_as_taggable_on/taggable/dirty.rb +++ /dev/null @@ -1,36 +0,0 @@ -module ActsAsTaggableOn::Taggable - module Dirty - def self.included(base) - base.extend ActsAsTaggableOn::Taggable::Dirty::ClassMethods - - base.initialize_acts_as_taggable_on_dirty - end - - module ClassMethods - def initialize_acts_as_taggable_on_dirty - tag_types.map(&:to_s).each do |tags_type| - tag_type = tags_type.to_s.singularize - - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{tag_type}_list_changed? - changed_attributes.include?("#{tag_type}_list") - end - - def #{tag_type}_list_was - changed_attributes.include?("#{tag_type}_list") ? changed_attributes["#{tag_type}_list"] : __send__("#{tag_type}_list") - end - - def #{tag_type}_list_change - [changed_attributes['#{tag_type}_list'], __send__('#{tag_type}_list')] if changed_attributes.include?("#{tag_type}_list") - end - - def #{tag_type}_list_changes - [changed_attributes['#{tag_type}_list'], __send__('#{tag_type}_list')] if changed_attributes.include?("#{tag_type}_list") - end - RUBY - - end - end - end - end -end diff --git a/lib/acts_as_taggable_on/taggable/tagged_with_query/any_tags_query.rb b/lib/acts_as_taggable_on/taggable/tagged_with_query/any_tags_query.rb index e66116c6c..b8c280b06 100644 --- a/lib/acts_as_taggable_on/taggable/tagged_with_query/any_tags_query.rb +++ b/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 diff --git a/lib/acts_as_taggable_on/taggable/tagged_with_query/query_base.rb b/lib/acts_as_taggable_on/taggable/tagged_with_query/query_base.rb index b556d70f0..56481e74e 100644 --- a/lib/acts_as_taggable_on/taggable/tagged_with_query/query_base.rb +++ b/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) diff --git a/lib/acts_as_taggable_on/tagging.rb b/lib/acts_as_taggable_on/tagging.rb index 783c45825..2188d99ee 100644 --- a/lib/acts_as_taggable_on/tagging.rb +++ b/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) } diff --git a/lib/acts_as_taggable_on/utils.rb b/lib/acts_as_taggable_on/utils.rb index df423cb74..3297c3a1e 100644 --- a/lib/acts_as_taggable_on/utils.rb +++ b/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 diff --git a/spec/acts_as_taggable_on/taggable/dirty_spec.rb b/spec/acts_as_taggable_on/dirty_spec.rb similarity index 70% rename from spec/acts_as_taggable_on/taggable/dirty_spec.rb rename to spec/acts_as_taggable_on/dirty_spec.rb index a07ad1b09..19c225448 100644 --- a/spec/acts_as_taggable_on/taggable/dirty_spec.rb +++ b/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,7 +98,7 @@ 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 @@ -98,15 +106,11 @@ 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 diff --git a/spec/acts_as_taggable_on/utils_spec.rb b/spec/acts_as_taggable_on/utils_spec.rb index ecd95b03c..fa23f897e 100644 --- a/spec/acts_as_taggable_on/utils_spec.rb +++ b/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