Merge pull request #549 from seuros/dirtyorder

Fix dirty marking when tags are not ordered. Fixes #548

Appraisals

@@ -1,20 +1,16 @@
 appraise "activerecord-3.2" do
   gem "activerecord", "~> 3.2"
-  gem "actionpack", "~> 3.2"
 end
 
 appraise "activerecord-4.0" do
   gem "activerecord", "~> 4.0"
-  gem "actionpack", "~> 4.0"
 end
 
 appraise "activerecord-4.1" do
   gem "activerecord", "~> 4.1"
-  gem "actionpack", "~> 4.1"
 end
 
 appraise "activerecord-edge" do
   gem "activerecord", github: "rails/rails"
-  gem "actionpack", github: "rails/rails"
   gem 'arel', github: 'rails/arel'
 end

acts-as-taggable-on.gemspec

@@ -23,7 +23,6 @@ Gem::Specification.new do |gem|
   end
 
   gem.add_runtime_dependency 'activerecord',  ['>= 3', '< 5']
-  gem.add_runtime_dependency 'actionpack',    ['>= 3', '< 5']
 
   gem.add_development_dependency 'sqlite3'
   gem.add_development_dependency 'mysql2', '~> 0.3.7'

gemfiles/activerecord_3.2.gemfile

@@ -3,7 +3,6 @@
 source "https://rubygems.org"
 
 gem "activerecord", "~> 3.2"
-gem "actionpack", "~> 3.2"
 
 group :local_development do
   gem "guard"

gemfiles/activerecord_4.0.gemfile

@@ -3,7 +3,6 @@
 source "https://rubygems.org"
 
 gem "activerecord", "~> 4.0"
-gem "actionpack", "~> 4.0"
 
 group :local_development do
   gem "guard"

gemfiles/activerecord_4.1.gemfile

@@ -3,7 +3,6 @@
 source "https://rubygems.org"
 
 gem "activerecord", "~> 4.1"
-gem "actionpack", "~> 4.1"
 
 group :local_development do
   gem "guard"

gemfiles/activerecord_edge.gemfile

@@ -3,7 +3,6 @@
 source "https://rubygems.org"
 
 gem "activerecord", :github => "rails/rails"
-gem "actionpack", :github => "rails/rails"
 gem "arel", :github => "rails/arel"
 
 group :local_development do

lib/acts_as_taggable_on/taggable/core.rb

@@ -153,7 +153,7 @@ def tagged_with(tags, options = {})
           end
 
           joins << tagging_join
-          unless any == 'distinct'    # Fix issue #544
+          unless any == 'distinct' # Fix issue #544
             group = "#{table_name}.#{primary_key}"
             select_clause << group
           end
@@ -329,8 +329,12 @@ def process_dirty_object(context, new_list)
         old = changed_attributes[attrib]
         changed_attributes.delete(attrib) if old.to_s == value.to_s
       else
-        old = tag_list_on(context).to_s
-        changed_attributes[attrib] = old if old.to_s != value.to_s
+        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::TagListParser.parse(value).sort
+        end
       end
     end
 

lib/acts_as_taggable_on/version.rb

@@ -1,4 +1,4 @@
 module ActsAsTaggableOn
-  VERSION = '3.2.5'
+  VERSION = '3.2.6'
 end
 

spec/acts_as_taggable_on/taggable/dirty_spec.rb

@@ -0,0 +1,127 @@
+# -*- encoding : utf-8 -*-
+require 'spec_helper'
+
+describe ActsAsTaggableOn::Taggable::Dirty do
+  context 'with un-contexted tags' do
+    before(:each) do
+      @taggable = TaggableModel.create(tag_list: 'awesome, epic')
+    end
+
+    context 'when tag_list changed' do
+      before(:each) do
+        expect(@taggable.changes).to be_empty
+        @taggable.tag_list = 'one'
+      end
+
+      it 'should show changes of dirty object' do
+        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
+      end
+
+      it 'preserves original value' do
+        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']])
+      end
+
+      context 'without order' do
+        it 'should not mark attribute if order change ' do
+          taggable = TaggableModel.create(name: 'Dirty Harry', tag_list: %w(d c b a))
+          taggable.tag_list =  %w(a b c d)
+          expect(taggable.tag_list_changed?).to be_falsey
+        end
+      end
+
+      context 'with order' do
+        it 'should mark attribute if order change' do
+          taggable = OrderedTaggableModel.create(name: 'Clean Harry', tag_list: 'd,c,b,a')
+          taggable.save
+          taggable.tag_list =  %w(a b c d)
+          expect(taggable.tag_list_changed?).to be_truthy
+        end
+      end
+    end
+
+    context 'when tag_list is the same' do
+      before(:each) do
+        @taggable.tag_list = 'awesome, epic'
+      end
+
+      it 'is not flagged as changed' do
+        expect(@taggable.tag_list_changed?).to be_falsy
+      end
+
+      it 'does not show any changes to the taggable item' do
+        expect(@taggable.changes).to be_empty
+      end
+
+      context "and using a delimiter different from a ','" do
+        before do
+          @old_delimiter = ActsAsTaggableOn.delimiter
+          ActsAsTaggableOn.delimiter = ';'
+        end
+
+        after do
+          ActsAsTaggableOn.delimiter = @old_delimiter
+        end
+
+        it 'does not show any changes to the taggable item when using array assignments' do
+          @taggable.tag_list = %w(awesome epic)
+          expect(@taggable.changes).to be_empty
+        end
+      end
+    end
+  end
+
+  context 'with context tags' do
+    before(:each) do
+      @taggable = TaggableModel.create('language_list' => 'awesome, epic')
+    end
+
+    context 'when language_list changed' do
+      before(:each) do
+        expect(@taggable.changes).to be_empty
+        @taggable.language_list = 'one'
+      end
+
+      it 'should show changes of dirty object' do
+        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')
+      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']])
+      end
+    end
+
+    context 'when language_list is the same' do
+      before(:each) do
+        @taggable.language_list = 'awesome, epic'
+      end
+
+      it 'is not flagged as changed' do
+        expect(@taggable.language_list_changed?).to be_falsy
+      end
+
+      it 'does not show any changes to the taggable item' do
+        expect(@taggable.changes).to be_empty
+      end
+    end
+  end
+end

spec/acts_as_taggable_on/taggable_spec.rb

@@ -710,114 +710,6 @@
     end
   end
 
-  describe 'Dirty Objects' do
-    context 'with un-contexted tags' do
-      before(:each) do
-        @taggable = TaggableModel.create(tag_list: 'awesome, epic')
-      end
-
-      context 'when tag_list changed' do
-        before(:each) do
-          expect(@taggable.changes).to be_empty
-          @taggable.tag_list = 'one'
-        end
-
-        it 'should show changes of dirty object' do
-          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
-        end
-
-        it 'preserves original value' do
-          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']])
-        end
-      end
-
-      context 'when tag_list is the same' do
-        before(:each) do
-          @taggable.tag_list = 'awesome, epic'
-        end
-
-        it 'is not flagged as changed' do
-          expect(@taggable.tag_list_changed?).to be_falsy
-        end
-
-        it 'does not show any changes to the taggable item' do
-          expect(@taggable.changes).to be_empty
-        end
-
-        context "and using a delimiter different from a ','" do
-          before do
-            @old_delimiter = ActsAsTaggableOn.delimiter
-            ActsAsTaggableOn.delimiter = ';'
-          end
-
-          after do
-            ActsAsTaggableOn.delimiter = @old_delimiter
-          end
-
-          it 'does not show any changes to the taggable item when using array assignments' do
-            @taggable.tag_list = %w(awesome epic)
-            expect(@taggable.changes).to be_empty
-          end
-        end
-      end
-    end
-
-    context 'with context tags' do
-      before(:each) do
-        @taggable = TaggableModel.create('language_list' => 'awesome, epic')
-      end
-
-      context 'when language_list changed' do
-        before(:each) do
-          expect(@taggable.changes).to be_empty
-          @taggable.language_list = 'one'
-        end
-
-        it 'should show changes of dirty object' do
-          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')
-        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']])
-        end
-      end
-
-      context 'when language_list is the same' do
-        before(:each) do
-          @taggable.language_list = 'awesome, epic'
-        end
-
-        it 'is not flagged as changed' do
-          expect(@taggable.language_list_changed?).to be_falsy
-        end
-
-        it 'does not show any changes to the taggable item' do
-          expect(@taggable.changes).to be_empty
-        end
-      end
-    end
-  end
-
   describe 'Autogenerated methods' do
     it 'should be overridable' do
       expect(TaggableModel.create(tag_list: 'woo').tag_list_submethod_called).to be_truthy