Merge pull request #2306 from Pistos/issue-2298-following-dot-tags

Issue 2298 following dot tags

app/controllers/tag_followings_controller.rb

@@ -14,13 +14,14 @@ class TagFollowingsController < ApplicationController
   # POST /tag_followings
   # POST /tag_followings.xml
   def create
-    @tag = ActsAsTaggableOn::Tag.find_or_create_by_name(tag_name)
+    name_normalized = ActsAsTaggableOn::Tag.normalize( params['name'] )
+    @tag = ActsAsTaggableOn::Tag.find_or_create_by_name(name_normalized)
     @tag_following = current_user.tag_followings.new(:tag_id => @tag.id)
 
     if @tag_following.save
-      flash[:notice] = I18n.t('tag_followings.create.success', :name => tag_name)
+      flash[:notice] = I18n.t('tag_followings.create.success', :name => name_normalized)
     else
-      flash[:error] = I18n.t('tag_followings.create.failure', :name => tag_name)
+      flash[:error] = I18n.t('tag_followings.create.failure', :name => name_normalized)
     end
 
     redirect_to :back
@@ -53,19 +54,12 @@ class TagFollowingsController < ApplicationController
   end
 
   def create_multiple
-    tags = params[:tags].split(",")
-    tags.each do |tag|
-      tag_name = tag.gsub(/^#/,"")
-
-      @tag = ActsAsTaggableOn::Tag.find_or_create_by_name(tag_name)
+    params[:tags].split(",").each do |name|
+      name_normalized = ActsAsTaggableOn::Tag.normalize(name)
+      @tag = ActsAsTaggableOn::Tag.find_or_create_by_name(name_normalized)
       @tag_following = current_user.tag_followings.create(:tag_id => @tag.id)
     end
 
     redirect_to multi_path
   end
-
-  private
-  def tag_name
-    @tag_name ||= params[:name].gsub(/\s/,'') if params[:name]
-  end
 end

app/models/acts_as_taggable_on_tag.rb

@@ -1,4 +1,4 @@
-class ActsAsTaggableOn::Tag 
+class ActsAsTaggableOn::Tag
   def followed_count
    @followed_count ||= TagFollowing.where(:tag_id => self.id).count
   end
@@ -6,4 +6,13 @@ class ActsAsTaggableOn::Tag
   def self.autocomplete(name)
     where("name LIKE ?", "#{name.downcase}%")
   end
+
+  def self.normalize(name)
+    if name =~ /^#?<3/
+      # Special case for love, because the world needs more love.
+      '<3'
+    elsif name
+      name.gsub(/[^\w-]/, '')
+    end
+  end
 end

spec/controllers/tag_followings_controller_spec.rb

@@ -28,7 +28,7 @@ describe TagFollowingsController do
   end
 
   describe "create" do
-    describe "with valid params" do
+    describe "successfully" do
       it "creates a new TagFollowing" do
         expect {
           post :create, valid_attributes
@@ -48,19 +48,10 @@ describe TagFollowingsController do
       end
 
       it "creates the tag IFF it doesn't already exist" do
+        ActsAsTaggableOn::Tag.find_by_name('tomcruisecontrol').should be_nil
         expect {
           post :create, :name => "tomcruisecontrol"
         }.to change(ActsAsTaggableOn::Tag, :count).by(1)
-
-        expect {
-          post :create, :name => "tomcruisecontrol"
-        }.to change(ActsAsTaggableOn::Tag, :count).by(0)
-      end
-      
-      it "will only create a tag following for the currently-signed-in user" do
-        expect {
-          post :create, valid_attributes.merge(:user_id => alice.id)
-        }.to_not change(alice.tag_followings, :count).by(1)
       end
 
       it "flashes success to the tag page" do
@@ -83,6 +74,62 @@ describe TagFollowingsController do
         post :create, :name => "SOMESTUFF"
         assigns[:tag].name.should == "somestuff"
       end
+
+      it 'strips invalid characters from the tag name' do
+        {
+          'node.js'                        => 'nodejs',
+          '#unneeded-hash'                 => 'unneeded-hash',
+          'hash#inside'                    => 'hashinside',
+          '.dotatstart'                    => 'dotatstart',
+          'f!u@n#k$y%-c^h&a*r(a)c{t}e[r]s' => 'funky-characters',
+          'how about spaces'               => 'howaboutspaces',
+        }.each do |invalid, normalized|
+          ActsAsTaggableOn::Tag.find_by_name(invalid).should be_nil
+          ActsAsTaggableOn::Tag.find_by_name(normalized).should be_nil
+
+          post :create, :name => invalid
+
+          ActsAsTaggableOn::Tag.find_by_name(invalid).should be_nil
+          ActsAsTaggableOn::Tag.find_by_name(normalized).should_not be_nil, "Expected #{normalized.inspect} not to be nil"
+          bob.reload
+          bob.followed_tags.map(&:name).should include(normalized)
+          bob.followed_tags.map(&:name).should_not include(invalid)
+        end
+      end
+
+      it 'follows love' do
+        name = '<3'
+
+        ActsAsTaggableOn::Tag.find_by_name(name).should be_nil
+
+        post :create, :name => name
+
+        ActsAsTaggableOn::Tag.find_by_name(name).should_not be_nil
+        bob.reload
+        bob.followed_tags.map(&:name).should include(name)
+      end
+    end
+
+    describe 'fails to' do
+      it "create the tag IFF already exists" do
+        ActsAsTaggableOn::Tag.find_by_name('tomcruisecontrol').should be_nil
+        expect {
+          post :create, :name => "tomcruisecontrol"
+        }.to change(ActsAsTaggableOn::Tag, :count).by(1)
+
+        ActsAsTaggableOn::Tag.find_by_name('tomcruisecontrol').should_not be_nil
+        expect {
+          post :create, :name => "tomcruisecontrol"
+        }.to change(ActsAsTaggableOn::Tag, :count).by(0)
+      end
+
+      it "create a tag following for a user other than the currently signed in user" do
+        expect {
+          expect {
+            post :create, valid_attributes.merge(:user_id => alice.id)
+          }.not_to change(alice.tag_followings, :count).by(1)
+        }.to change(bob.tag_followings, :count).by(1)
+      end
     end
   end
 
@@ -134,6 +181,28 @@ describe TagFollowingsController do
 
       response.should be_redirect
     end
+
+    it 'strips invalid characters from the tag name' do
+      {
+        'node.js' => 'nodejs',
+        '#unneeded-hash' => 'unneeded-hash',
+        'hash#inside' => 'hashinside',
+        '.dotatstart' => 'dotatstart',
+        'f!u@n#k$y%-c^h&a*r(a)c{t}e[r]s' => 'funky-characters',
+        'how about spaces' => 'howaboutspaces',
+      }.each do |invalid, normalized|
+        ActsAsTaggableOn::Tag.find_by_name(invalid).should be_nil
+        ActsAsTaggableOn::Tag.find_by_name(normalized).should be_nil
+
+        post :create_multiple, :tags => invalid
+
+        ActsAsTaggableOn::Tag.find_by_name(invalid).should be_nil
+        ActsAsTaggableOn::Tag.find_by_name(normalized).should_not be_nil
+        bob.reload
+        bob.followed_tags.map(&:name).should include(normalized)
+        bob.followed_tags.map(&:name).should_not include(invalid)
+      end
+    end
   end
 
 end

spec/spec-doc.rb

@@ -8,7 +8,8 @@ class SpecDoc
   end
 
   def has_content?(string)
-    @html.xpath("//*[contains(text(), '#{string}')]").any?
+    escaped = string.gsub("'", "\\'")
+    @html.xpath("//*[contains(text(), '#{escaped}')]").any?
   end
   def has_no_content?(string)
     ! has_content?(string)