From e885e8492f0cf6c7fc07b500b8beb10b38821ac6 Mon Sep 17 00:00:00 2001 From: Pistos Date: Wed, 2 Nov 2011 20:46:04 -0400 Subject: [PATCH 1/2] Refactor / shuffle code in TagFollowingsController spec. --- .../tag_followings_controller_spec.rb | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/spec/controllers/tag_followings_controller_spec.rb b/spec/controllers/tag_followings_controller_spec.rb index 6a4dbcc9a..372e5a31b 100644 --- a/spec/controllers/tag_followings_controller_spec.rb +++ b/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 @@ -84,6 +75,28 @@ describe TagFollowingsController do assigns[:tag].name.should == "somestuff" 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 describe "DELETE destroy" do From 78a96a18e407676c7e79fd8f76d91ea5987d61c3 Mon Sep 17 00:00:00 2001 From: Pistos Date: Wed, 2 Nov 2011 23:51:12 -0400 Subject: [PATCH 2/2] This fixes issue #2298. Following a hashtag with a dot now no longer breaks the user's stream page. All unacceptable hashtag chars are stripped out, and the given tag is normalized before being followed. --- app/controllers/tag_followings_controller.rb | 20 +++---- app/models/acts_as_taggable_on_tag.rb | 11 +++- .../tag_followings_controller_spec.rb | 56 +++++++++++++++++++ spec/spec-doc.rb | 3 +- 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/app/controllers/tag_followings_controller.rb b/app/controllers/tag_followings_controller.rb index 4a82166b4..095d8f4c8 100644 --- a/app/controllers/tag_followings_controller.rb +++ b/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 diff --git a/app/models/acts_as_taggable_on_tag.rb b/app/models/acts_as_taggable_on_tag.rb index 66d948e99..fdf80defa 100644 --- a/app/models/acts_as_taggable_on_tag.rb +++ b/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 diff --git a/spec/controllers/tag_followings_controller_spec.rb b/spec/controllers/tag_followings_controller_spec.rb index 372e5a31b..dfb17a2f2 100644 --- a/spec/controllers/tag_followings_controller_spec.rb +++ b/spec/controllers/tag_followings_controller_spec.rb @@ -74,6 +74,40 @@ 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 @@ -147,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 diff --git a/spec/spec-doc.rb b/spec/spec-doc.rb index 2ec9ee5df..d1e778b12 100644 --- a/spec/spec-doc.rb +++ b/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)