Fixes #2713 - searching for punctuation now doesn't throw a 500. Refactored people#index specs.

app/controllers/people_controller.rb

@@ -46,6 +46,7 @@ class PeopleController < ApplicationController
         else
           people = Person.search(params[:q], current_user)
         end
+        @normalized_tag_for_query = ActsAsTaggableOn::Tag.normalize(params[:q])
         @people = people.paginate( :page => params[:page], :per_page => 15)
         @hashes = hashes_for_people(@people, @aspects)
       end

app/helpers/tags_helper.rb

@@ -1,10 +0,0 @@
-#   Copyright (c) 2010-2011, Diaspora Inc.  This file is
-#   licensed under the Affero General Public License version 3 or later.  See
-#   the COPYRIGHT file.
-
-module TagsHelper
-  def tag_page_link(tag)
-    tag_name = ActsAsTaggableOn::Tag.normalize(tag)
-    link_to("##{tag_name}", tag_path(:name => tag_name))
-  end
-end

app/views/people/index.html.haml

@@ -18,9 +18,10 @@
       =t('.results_for')
       %span.term
         = params[:q]
-    %h4
+    - if @normalized_tag_for_query.present?
-      %small
+      %h4
-        = t('.looking_for', :tag_link => tag_page_link(params[:q])).html_safe
+        %small
+          = t('.looking_for', :tag_link => link_to("##{@normalized_tag_for_query}", tag_path(:name => @normalized_tag_for_query))).html_safe
 .span-15
   %hr
 .clearfix

spec/controllers/people_controller_spec.rb

@@ -14,92 +14,114 @@ describe PeopleController do
   describe '#index (search)' do
     before do
       @eugene = Factory(:person,
-        :profile => Factory.build(:profile, :first_name => "Eugene", :last_name => "w"))
+                        :profile => Factory.build(:profile, :first_name => "Eugene", :last_name => "w"))
       @korth = Factory(:person,
-        :profile => Factory.build(:profile, :first_name => "Evan", :last_name => "Korth"))
+                       :profile => Factory.build(:profile, :first_name => "Evan", :last_name => "Korth"))
     end
 
-    it 'responds with json' do
+    describe 'via json' do
-      get :index, :q => "Korth", :format => 'json'
+      it 'succeeds' do
-      response.body.should == [@korth].to_json
+        get :index, :q => "Korth", :format => 'json'
+        response.should be_success
+      end
+
+      it 'responds with json' do
+        get :index, :q => "Korth", :format => 'json'
+        response.body.should == [@korth].to_json
+      end
+
+      it 'does not assign hashes' do
+        get :index, :q => "Korth", :format => 'json'
+        assigns[:hashes].should be_nil
+      end
     end
 
-    it 'does not set @hashes in a json request' do
+    describe 'via html' do
-      get :index, :q => "Korth", :format => 'json'
+      context 'query is a diaspora ID' do
-      assigns[:hashes].should be_nil
+        before do
-    end
+          @unsearchable_eugene = Factory(:person, :diaspora_handle => "eugene@example.org",
+                                         :profile => Factory.build(:profile, :first_name => "Eugene",
+                                                                   :last_name => "w", :searchable => false))
+        end
+        it 'finds people even if they have searchable off' do
+          get :index, :q => "eugene@example.org"
+          assigns[:people][0].id.should == @unsearchable_eugene.id
+        end
 
-    it 'sets @hashes in an html request' do
+        it 'downcases the query term' do
-      get :index, :q => "Korth"
+          get :index, :q => "Eugene@Example.ORG"
-      assigns[:hashes].should_not be_nil
+          assigns[:people][0].id.should == @unsearchable_eugene.id
-    end
+        end
+      end
 
-    it "assigns people" do
+      context 'query is a tag' do
-      eugene2 = Factory(:person,
+        it 'goes to a tag page' do
-                               :profile => Factory.build(:profile, :first_name => "Eugene",
+          get :index, :q => '#babies'
-                                                         :last_name => "w"))
+          response.should redirect_to(tag_path('babies', :q => '#babies'))
-      get :index, :q => "Eug"
+        end
-      assigns[:people].map{|x| x.id}.should =~ [@eugene.id, eugene2.id]
-    end
 
-    it "excludes people that are not searchable" do
+        it 'removes dots from the query' do
-      eugene2 = Factory(:person,
+          get :index, :q => '#babi.es'
-                               :profile => Factory.build(:profile, :first_name => "Eugene",
+          response.should redirect_to(tag_path('babies', :q => '#babi.es'))
-                                                         :last_name => "w", :searchable => false))
+        end
-      get :index, :q => "Eug"
-      assigns[:people].should_not =~ [eugene2]
-    end
 
-    it "allows unsearchable people to be found by handle" do
+        it 'stay on the page if you search for the empty hash' do
-      eugene2 = Factory(:person, :diaspora_handle => "eugene@example.org",
+          get :index, :q => '#'
-                               :profile => Factory.build(:profile, :first_name => "Eugene",
+          flash[:error].should be_present
-                                                         :last_name => "w", :searchable => false))
+        end
-      get :index, :q => "eugene@example.org"
+      end
-      assigns[:people][0].id.should == eugene2.id
-    end
 
-    it "allows unsearchable people to be found by handle" do
+      context 'query is not a tag or a diaspora ID' do
-      d_id = "eugene@example.org"
+        it 'assigns hashes' do
-      @controller.should_receive(:diaspora_id?).with(d_id)
+          get :index, :q => "Korth"
-      get :index, :q => d_id
+          assigns[:hashes].should_not be_nil
-    end
+        end
 
-     it "downcases the handle before trying to find someone by it" do
+        it "assigns people" do
-      eugene2 = Factory(:person, :diaspora_handle => "eugene@example.org",
+          eugene2 = Factory(:person,
-                               :profile => Factory.build(:profile, :first_name => "Eugene",
+                            :profile => Factory.build(:profile, :first_name => "Eugene",
-                                                         :last_name => "w", :searchable => false))
+                                                      :last_name => "w"))
-      get :index, :q => "Eugene@Example.ORG"
+          get :index, :q => "Eug"
-      assigns[:people][0].id.should == eugene2.id
+          assigns[:people].map { |x| x.id }.should =~ [@eugene.id, eugene2.id]
-    end
+        end
+        
+        it "assigns a normalized tag" do
+          get :index, :q => "foo"
+          assigns[:normalized_tag_for_query].should == "foo"
+        end
 
-    it "does not redirect to person page if there is exactly one match" do
+        it "succeeds if there is exactly one match" do
-      get :index, :q => "Korth"
+          get :index, :q => "Korth"
-      response.should_not redirect_to @korth
+          assigns[:people].length.should == 1
-    end
+          response.should be_success
+        end
 
-    it "does not redirect if there are no matches" do
+        it "succeeds if there are no matches" do
-      get :index, :q => "Korthsauce"
+          get :index, :q => "Korthsauce"
-      response.should_not be_redirect
+          assigns[:people].length.should == 0
-    end
+          response.should be_success
+        end
 
-    it 'goes to a tag page if you search for a hash' do
+        it 'succeeds if you search for the empty term' do
-      get :index, :q => '#babies'
+          get :index, :q => ''
-      response.should redirect_to(tag_path('babies', :q => '#babies'))
+          assigns[:normalized_tag_for_query].should be_empty
-    end
+          response.should be_success
+        end
 
-    it 'goes to a tag page if you search for a hash with dots' do
+        it 'succeeds if you search for punctuation' do
-      get :index, :q => '#babi.es'
+          get :index, :q => '+'
-      response.should redirect_to(tag_path('babies', :q => '#babi.es'))
+          assigns[:normalized_tag_for_query].should be_empty
-    end
+          response.should be_success
+        end
 
-    it 'stay on the page if you search for the empty hash' do
+        it "excludes people who have searchable off" do
-      get :index, :q => '#'
+          eugene2 = Factory(:person,
-      flash[:error].should be_present
+                            :profile => Factory.build(:profile, :first_name => "Eugene",
-    end
+                                                      :last_name => "w", :searchable => false))
-
+          get :index, :q => "Eug"
-    it 'does not fails if you search for the empty term' do
+          assigns[:people].should_not =~ [eugene2]
-      get :index, :q => ''
+        end
-      response.should be_success
+      end
     end
   end
 
@@ -243,7 +265,7 @@ describe PeopleController do
         it "posts include reshares" do
           reshare = @user.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects)
           get :show, :id => @user.person.to_param
-          assigns[:stream].posts.map{|x| x.id}.should include(reshare.id)
+          assigns[:stream].posts.map { |x| x.id }.should include(reshare.id)
         end
 
         it "assigns only public posts" do
@@ -253,7 +275,7 @@ describe PeopleController do
 
         it 'is sorted by created_at desc' do
           get :show, :id => @person.to_param
-          assigns[:stream].stream_posts.should == @public_posts.sort_by{|p| p.created_at}.reverse
+          assigns[:stream].stream_posts.should == @public_posts.sort_by { |p| p.created_at }.reverse
         end
       end
 
@@ -298,7 +320,7 @@ describe PeopleController do
       it "posts include reshares" do
         reshare = @user.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects)
         get :show, :id => @user.person.to_param
-        assigns[:stream].posts.map{|x| x.id}.should include(reshare.id)
+        assigns[:stream].posts.map { |x| x.id }.should include(reshare.id)
       end
     end
 
@@ -331,7 +353,7 @@ describe PeopleController do
       it "posts include reshares" do
         reshare = @user.post(:reshare, :public => true, :root_guid => Factory(:status_message, :public => true).guid, :to => alice.aspects)
         get :show, :id => @user.person.to_param
-        assigns[:stream].posts.map{|x| x.id}.should include(reshare.id)
+        assigns[:stream].posts.map { |x| x.id }.should include(reshare.id)
       end
     end
   end