diff --git a/app/models/person.rb b/app/models/person.rb index 02fd76dc2..61ff6a2bd 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -39,18 +39,24 @@ class Person < ActiveRecord::Base def self.search(query) return [] if query.to_s.empty? - query_tokens = query.to_s.strip.split(" ") - p = [] - query_tokens.each do |raw_token| + where_clause = <<-SQL + profiles.first_name LIKE ? OR + profiles.last_name LIKE ? OR + people.diaspora_handle LIKE ? + SQL + sql = "" + tokens = [] + + query_tokens = query.to_s.strip.split(" ") + query_tokens.each_with_index do |raw_token, i| token = "%#{raw_token}%" - p = Person.searchable.where('profiles.first_name LIKE :token', :token => token).limit(30) \ - | Person.searchable.where('profiles.last_name LIKE :token', :token => token).limit(30) \ - | Person.searchable.where('profiles.diaspora_handle LIKE :token', :token => token).limit(30) \ - | p + sql << " OR " unless i == 0 + sql << where_clause + tokens.concat([token, token, token]) end - return p + Person.searchable.where(sql, *tokens).order("profiles.first_name") end def name diff --git a/spec/factories.rb b/spec/factories.rb index 2d6a24a02..dc087c475 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -24,6 +24,12 @@ Factory.define :person do |p| p.serialized_public_key OpenSSL::PKey::RSA.generate(1024).public_key.export end +Factory.define :searchable_person, :parent => :person do |p| + p.after_build do |person| + person.profile.searchable = true + end +end + Factory.define :user do |u| u.sequence(:username) { |n| "bob#{n}#{r_str}" } u.sequence(:email) { |n| "bob#{n}#{r_str}@pivotallabs.com" } diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index f0151e42d..d6df13f37 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -140,56 +140,57 @@ describe Person do describe '.search' do before do - @connected_person_one = Factory.create(:person) - @connected_person_two = Factory.create(:person) - @connected_person_three = Factory.create(:person) - @connected_person_four = Factory.create(:person) + Person.delete_all + @robert_grimm = Factory.create(:searchable_person) + @eugene_weinstein = Factory.create(:searchable_person) + @yevgeniy_dodis = Factory.create(:searchable_person) + @casey_grippi = Factory.create(:searchable_person) - @connected_person_one.profile.first_name = "Robert" - @connected_person_one.profile.last_name = "Grimm" - @connected_person_one.profile.save + @robert_grimm.profile.first_name = "Robert" + @robert_grimm.profile.last_name = "Grimm" + @robert_grimm.profile.save + @robert_grimm.reload - @connected_person_two.profile.first_name = "Eugene" - @connected_person_two.profile.last_name = "Weinstein" - @connected_person_two.save + @eugene_weinstein.profile.first_name = "Eugene" + @eugene_weinstein.profile.last_name = "Weinstein" + @eugene_weinstein.profile.save + @eugene_weinstein.reload - @connected_person_three.profile.first_name = "Yevgeniy" - @connected_person_three.profile.last_name = "Dodis" - @connected_person_three.save + @yevgeniy_dodis.profile.first_name = "Yevgeniy" + @yevgeniy_dodis.profile.last_name = "Dodis" + @yevgeniy_dodis.profile.save + @yevgeniy_dodis.reload - @connected_person_four.profile.first_name = "Casey" - @connected_person_four.profile.last_name = "Grippi" - @connected_person_four.save + @casey_grippi.profile.first_name = "Casey" + @casey_grippi.profile.last_name = "Grippi" + @casey_grippi.profile.save + @casey_grippi.reload end - it 'should return nothing on an emprty query' do + it 'should return nothing on an empty query' do people = Person.search("") people.empty?.should be true end it 'should yield search results on partial names' do people = Person.search("Eu") - people.include?(@connected_person_two).should == true - people.include?(@connected_person_one).should == false - people.include?(@connected_person_three).should == false - people.include?(@connected_person_four).should == false + people.count.should == 1 + people.first.should == @eugene_weinstein people = Person.search("wEi") - people.include?(@connected_person_two).should == true - people.include?(@connected_person_one).should == false - people.include?(@connected_person_three).should == false - people.include?(@connected_person_four).should == false + people.count.should == 1 + people.first.should == @eugene_weinstein people = Person.search("gri") - people.include?(@connected_person_one).should == true - people.include?(@connected_person_four).should == true - people.include?(@connected_person_two).should == false - people.include?(@connected_person_three).should == false + people.count.should == 2 + people.first.should == @casey_grippi + people.second.should == @robert_grimm end it 'should yield results on full names' do people = Person.search("Casey Grippi") - people.should == [@connected_person_four] + people.count.should == 1 + people.first.should == @casey_grippi end it 'should only display searchable people' do @@ -199,7 +200,9 @@ describe Person do end it 'should search on handles' do - Person.search(@connected_person_one.diaspora_handle).should include @connected_person_one + people = Person.search(@robert_grimm.diaspora_handle) + people.count.should == 1 + people.first.should == @robert_grimm end end