Added acts_as_ordered_taggable to preserve the order in which tags ar…

…e created

The new methods set the attribute preserve_tag_order? to true and
consequently (1) when saving tags, the taggings are created in the order
in which the tags appear in the tag list; (2) when fetching tags by
context for the tag lists they are ordered by tagging created_at; (3) an
order option is added to the tag context associations (so that for a
'tags' context the associations tag_taggings & tags are always returned
in tagging created_at order)

lib/acts_as_taggable_on/acts_as_taggable_on.rb

@@ -14,6 +14,17 @@ def taggable?
     def acts_as_taggable
       acts_as_taggable_on :tags
     end
+
+    ##
+    # This is an alias for calling <tt>acts_as_ordered_taggable_on :tags</tt>.
+    #
+    # Example:
+    #   class Book < ActiveRecord::Base
+    #     acts_as_ordered_taggable
+    #   end
+    def acts_as_ordered_taggable
+      acts_as_taggable_on :tags
+    end
 
     ##
     # Make a model taggable on specified contexts.
@@ -25,29 +36,61 @@ def acts_as_taggable
     #     acts_as_taggable_on :languages, :skills
     #   end
     def acts_as_taggable_on(*tag_types)
-      tag_types = tag_types.to_a.flatten.compact.map(&:to_sym)
+      taggable_on(false, tag_types)
+    end
+
+    ##
+    # Make a model taggable on specified contexts
+    # and preserves the order in which tags are created
+    #
+    # @param [Array] tag_types An array of taggable contexts
+    #
+    # Example:
+    #   class User < ActiveRecord::Base
+    #     acts_as_ordered_taggable_on :languages, :skills
+    #   end
+    def acts_as_ordered_taggable_on(*tag_types)
+      taggable_on(true, tag_types)
+    end
+
+    private
+
+      # Make a model taggable on specified contexts
+      # and optionally preserves the order in which tags are created
+      #
+      # Seperate methods used above for backwards compatibility
+      # so that the original acts_as_taggable_on is unaffected
+      # as it's not possible to add another arguement to the method
+      # without the tag_types being enclosed in square brackets
+      #
+      def taggable_on(preserve_tag_order, *tag_types)
+        tag_types = tag_types.to_a.flatten.compact.map(&:to_sym)
+
+        if taggable?
+          write_inheritable_attribute(:tag_types, (self.tag_types + tag_types).uniq)
+          write_inheritable_attribute(:preserve_tag_order?, preserve_tag_order)
+        else
+          write_inheritable_attribute(:tag_types, tag_types)
+          write_inheritable_attribute(:preserve_tag_order?, preserve_tag_order)
+          class_inheritable_reader(:tag_types)
+          class_inheritable_reader(:preserve_tag_order?)
 
-      if taggable?
-        write_inheritable_attribute(:tag_types, (self.tag_types + tag_types).uniq)
-      else
-        write_inheritable_attribute(:tag_types, tag_types)
-        class_inheritable_reader(:tag_types)
-
-        class_eval do
-          has_many :taggings, :as => :taggable, :dependent => :destroy, :include => :tag, :class_name => "ActsAsTaggableOn::Tagging"
-          has_many :base_tags, :through => :taggings, :source => :tag, :class_name => "ActsAsTaggableOn::Tag"
+          class_eval do
+            has_many :taggings, :as => :taggable, :dependent => :destroy, :include => :tag, :class_name => "ActsAsTaggableOn::Tagging"
+            has_many :base_tags, :through => :taggings, :source => :tag, :class_name => "ActsAsTaggableOn::Tag"
 
-          def self.taggable?
-            true
+            def self.taggable?
+              true
+            end
+
+            include ActsAsTaggableOn::Taggable::Core
+            include ActsAsTaggableOn::Taggable::Collection
+            include ActsAsTaggableOn::Taggable::Cache
+            include ActsAsTaggableOn::Taggable::Ownership
+            include ActsAsTaggableOn::Taggable::Related
           end
-
-          include ActsAsTaggableOn::Taggable::Core
-          include ActsAsTaggableOn::Taggable::Collection
-          include ActsAsTaggableOn::Taggable::Cache
-          include ActsAsTaggableOn::Taggable::Ownership
-          include ActsAsTaggableOn::Taggable::Related
         end
       end
-    end
+       
   end
 end

lib/acts_as_taggable_on/acts_as_taggable_on/core.rb

@@ -18,11 +18,21 @@ def initialize_acts_as_taggable_on_core
           tag_type         = tags_type.to_s.singularize
           context_taggings = "#{tag_type}_taggings".to_sym
           context_tags     = tags_type.to_sym
+          taggings_order   = (preserve_tag_order? ? "#{ActsAsTaggableOn::Tagging.table_name}.created_at" : nil)
 
           class_eval do
-            has_many context_taggings, :as => :taggable, :dependent => :destroy, :include => :tag, :class_name => "ActsAsTaggableOn::Tagging",
-            :conditions => ["#{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.id AND #{ActsAsTaggableOn::Tagging.table_name}.context = ?", tags_type]
-            has_many context_tags, :through => context_taggings, :source => :tag, :class_name => "ActsAsTaggableOn::Tag"
+            # when preserving tag order, include order option so that for a 'tags' context
+            # the associations tag_taggings & tags are always returned in created order
+            has_many context_taggings, 
+              :as => :taggable, :dependent => :destroy, 
+              :include => :tag, :class_name => "ActsAsTaggableOn::Tagging",
+              :conditions => ["#{ActsAsTaggableOn::Tagging.table_name}.tag_id = #{ActsAsTaggableOn::Tag.table_name}.id AND #{ActsAsTaggableOn::Tagging.table_name}.context = ?", tags_type],
+              :order => taggings_order
+            has_many context_tags,
+              :through => context_taggings,
+              :source => :tag,
+              :class_name => "ActsAsTaggableOn::Tag",
+              :order => taggings_order
           end
 
           class_eval %(
@@ -204,7 +214,11 @@ def all_tags_on(context)
       ##
       # Returns all tags that are not owned of a given context
       def tags_on(context)
-        base_tags.where(["#{ActsAsTaggableOn::Tagging.table_name}.context = ? AND #{ActsAsTaggableOn::Tagging.table_name}.tagger_id IS NULL", context.to_s]).all
+        scope = base_tags.where(["#{ActsAsTaggableOn::Tagging.table_name}.context = ? AND #{ActsAsTaggableOn::Tagging.table_name}.tagger_id IS NULL", context.to_s])
+        # when preserving tag order, return tags in created order
+        # if we added the order to the association this would always apply
+        scope = scope.order("#{ActsAsTaggableOn::Tagging.table_name}.created_at") if self.class.preserve_tag_order?
+        scope.all
       end
 
       def set_tag_list_on(context, new_list)
@@ -231,21 +245,38 @@ def save_tags
         tagging_contexts.each do |context|
           next unless tag_list_cache_set_on(context)
 
+          # List of currently assigned tag names
           tag_list = tag_list_cache_on(context).uniq
 
           # Find existing tags or create non-existing tags:
-          tag_list = ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name(tag_list)
+          tags = ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name(tag_list)
 
+          # Tag objects for currently assigned tags
           current_tags = tags_on(context)
-          old_tags     = current_tags - tag_list
-          new_tags     = tag_list     - current_tags
+
+          # Tag maintenance based on whether preserving the created order of tags
+          if self.class.preserve_tag_order?
+            # First off order the array of tag objects to match the tag list
+            # rather than existing tags followed by new tags
+            tags = tag_list.map{|l| tags.detect{|t| t.name.downcase == l.downcase}}
+            # To preserve tags in the order in which they were added
+            # delete all current tags and create new tags if the content or order has changed
+            old_tags = (tags == current_tags ? [] : current_tags)
+            new_tags = (tags == current_tags ? [] : tags)
+          else
+            # Delete discarded tags and create new tags
+            old_tags = current_tags - tags
+            new_tags = tags - current_tags
+          end
 
           # Find taggings to remove:
-          old_taggings = taggings.where(:tagger_type => nil, :tagger_id => nil,
-                                        :context => context.to_s, :tag_id => old_tags).all
-
+          if old_tags.present?
+            old_taggings = taggings.where(:tagger_type => nil, :tagger_id => nil,
+                                          :context => context.to_s, :tag_id => old_tags).all
+          end
+
+          # Destroy old taggings:
           if old_taggings.present?
-            # Destroy old taggings:
             ActsAsTaggableOn::Tagging.destroy_all :id => old_taggings.map(&:id)
           end
 

lib/acts_as_taggable_on/acts_as_taggable_on/ownership.rb

@@ -31,12 +31,16 @@ def #{tag_type}_from(owner)
     module InstanceMethods
       def owner_tags_on(owner, context)
         if owner.nil?
-          base_tags.where([%(#{ActsAsTaggableOn::Tagging.table_name}.context = ?), context.to_s]).all                    
+          scope = base_tags.where([%(#{ActsAsTaggableOn::Tagging.table_name}.context = ?), context.to_s])
         else
-          base_tags.where([%(#{ActsAsTaggableOn::Tagging.table_name}.context = ? AND
-                             #{ActsAsTaggableOn::Tagging.table_name}.tagger_id = ? AND
-                             #{ActsAsTaggableOn::Tagging.table_name}.tagger_type = ?), context.to_s, owner.id, owner.class.to_s]).all          
+          scope = base_tags.where([%(#{ActsAsTaggableOn::Tagging.table_name}.context = ? AND
+                                     #{ActsAsTaggableOn::Tagging.table_name}.tagger_id = ? AND
+                                     #{ActsAsTaggableOn::Tagging.table_name}.tagger_type = ?), context.to_s, owner.id, owner.class.to_s])
         end
+        # when preserving tag order, return tags in created order
+        # if we added the order to the association this would always apply
+        scope = scope.order("#{ActsAsTaggableOn::Tagging.table_name}.created_at") if self.class.preserve_tag_order?
+        scope.all
       end
 
       def cached_owned_tag_list_on(context)
@@ -73,21 +77,38 @@ def reload(*args)
       def save_owned_tags
         tagging_contexts.each do |context|
           cached_owned_tag_list_on(context).each do |owner, tag_list|
+
             # Find existing tags or create non-existing tags:
-            tag_list = ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name(tag_list.uniq)            
+            tags = ActsAsTaggableOn::Tag.find_or_create_all_with_like_by_name(tag_list.uniq)            
 
-            owned_tags = owner_tags_on(owner, context)              
-            old_tags   = owned_tags - tag_list
-            new_tags   = tag_list   - owned_tags
+            # Tag objects for owned tags
+            owned_tags = owner_tags_on(owner, context)
+
+            # Tag maintenance based on whether preserving the created order of tags
+            if self.class.preserve_tag_order?
+              # First off order the array of tag objects to match the tag list
+              # rather than existing tags followed by new tags
+              tags = tag_list.uniq.map{|s| tags.detect{|t| t.name.downcase == s.downcase}}
+              # To preserve tags in the order in which they were added
+              # delete all owned tags and create new tags if the content or order has changed
+              old_tags = (tags == owned_tags ? [] : owned_tags)
+              new_tags = (tags == owned_tags ? [] : tags)
+            else
+              # Delete discarded tags and create new tags
+              old_tags = owned_tags - tags
+              new_tags = tags - owned_tags
+            end
 
             # Find all taggings that belong to the taggable (self), are owned by the owner, 
             # have the correct context, and are removed from the list.
-            old_taggings = ActsAsTaggableOn::Tagging.where(:taggable_id => id, :taggable_type => self.class.base_class.to_s,
-                                                           :tagger_type => owner.class.to_s, :tagger_id => owner.id,
-                                                           :tag_id => old_tags, :context => context).all
+            if old_tags.present?
+              old_taggings = ActsAsTaggableOn::Tagging.where(:taggable_id => id, :taggable_type => self.class.base_class.to_s,
+                                                             :tagger_type => owner.class.to_s, :tagger_id => owner.id,
+                                                             :tag_id => old_tags, :context => context).all
+            end
 
+            # Destroy old taggings:
             if old_taggings.present?
-              # Destroy old taggings:
               ActsAsTaggableOn::Tagging.destroy_all(:id => old_taggings.map(&:id))
             end
 

spec/acts_as_taggable_on/acts_as_taggable_on_spec.rb

@@ -8,6 +8,19 @@
   it "should provide a class method 'taggable?' that is false for untaggable models" do
     UntaggableModel.should_not be_taggable
   end
+
+  describe "Taggable Method Generation To Preserve Order" do
+    before(:each) do
+      clean_database!
+      TaggableModel.write_inheritable_attribute(:tag_types, [])
+      TaggableModel.acts_as_ordered_taggable_on(:ordered_tags)
+      @taggable = TaggableModel.new(:name => "Bob Jones")
+    end
+
+    it "should respond 'true' to preserve_tag_order?" do
+      @taggable.class.preserve_tag_order?.should be_true
+    end
+  end
 
   describe "Taggable Method Generation" do
     before(:each) do
@@ -32,6 +45,18 @@
     it "should have all tag types" do
       @taggable.tag_types.should == [:tags, :languages, :skills, :needs, :offerings]
     end
+
+    it "should create a class attribute for preserve tag order" do
+      @taggable.class.should respond_to(:preserve_tag_order?)
+    end
+
+    it "should create an instance attribute for preserve tag order" do
+      @taggable.should respond_to(:preserve_tag_order?)
+    end
+
+    it "should respond 'false' to preserve_tag_order?" do
+      @taggable.class.preserve_tag_order?.should be_false
+    end
 
     it "should generate an association for each tag type" do
       @taggable.should respond_to(:tags, :skills, :languages)
@@ -50,7 +75,7 @@
       @taggable.should respond_to(:all_tags_list, :all_skills_list, :all_languages_list)
     end
   end
-
+  
   describe "Single Table Inheritance" do
     before do
       @taggable = TaggableModel.new(:name => "taggable")

spec/acts_as_taggable_on/taggable_spec.rb

@@ -1,8 +1,74 @@
 require File.expand_path('../../spec_helper', __FILE__)
 
+describe "Taggable To Preserve Order" do
+  before(:each) do
+    clean_database!
+    TaggableModel.write_inheritable_attribute(:tag_types, [])
+    TaggableModel.acts_as_ordered_taggable_on(:tags)
+    @taggable = TaggableModel.new(:name => "Bob Jones")
+    @taggables = [@taggable, TaggableModel.new(:name => "John Doe")]
+  end
+
+  it "should return tag list in the order the tags were created" do
+    # create
+    @taggable.tag_list = "rails, ruby, css"
+    @taggable.instance_variable_get("@tag_list").instance_of?(ActsAsTaggableOn::TagList).should be_true
+
+    lambda {
+      @taggable.save
+    }.should change(ActsAsTaggableOn::Tag, :count).by(3)
+
+    @taggable.reload
+    @taggable.tag_list.should == %w(rails ruby css)
+
+    # update
+    @taggable.tag_list = "pow, ruby, rails"
+    @taggable.save
+
+    @taggable.reload
+    @taggable.tag_list.should == %w(pow ruby rails)
+
+    # update with no change
+    @taggable.tag_list = "pow, ruby, rails"
+    @taggable.save
+
+    @taggable.reload
+    @taggable.tag_list.should == %w(pow ruby rails)
+
+    # update to clear tags
+    @taggable.tag_list = ""
+    @taggable.save
+
+    @taggable.reload
+    @taggable.tag_list.should == []
+  end
+
+  it "should return tag objects in the order the tags were created" do
+    # create
+    @taggable.tag_list = "pow, ruby, rails"
+    @taggable.instance_variable_get("@tag_list").instance_of?(ActsAsTaggableOn::TagList).should be_true
+
+    lambda {
+      @taggable.save
+    }.should change(ActsAsTaggableOn::Tag, :count).by(3)
+
+    @taggable.reload
+    @taggable.tags.map{|t| t.name}.should == %w(pow ruby rails)
+
+    # update
+    @taggable.tag_list = "rails, ruby, css, pow"
+    @taggable.save
+
+    @taggable.reload
+    @taggable.tags.map{|t| t.name}.should == %w(rails ruby css pow)
+  end
+end
+
 describe "Taggable" do
   before(:each) do
     clean_database!
+    TaggableModel.write_inheritable_attribute(:tag_types, [])
+    TaggableModel.acts_as_taggable_on(:tags, :languages, :skills, :needs, :offerings)
     @taggable = TaggableModel.new(:name => "Bob Jones")
     @taggables = [@taggable, TaggableModel.new(:name => "John Doe")]
   end