From f1e9c99866aa19579b87d65648482774cbba61ff Mon Sep 17 00:00:00 2001
From: Steffen van Bergerem <svbergerem@online.de>
Date: Tue, 9 Aug 2016 15:45:27 +0200
Subject: [PATCH] Add contacts search

---
 .../app/views/publisher/mention_view.js       |  2 +-
 .../javascripts/app/views/search_base_view.js |  8 ---
 app/controllers/contacts_controller.rb        | 11 ++++
 app/models/person.rb                          | 37 ++++++------
 config/routes.rb                              |  3 +
 spec/controllers/contacts_controller_spec.rb  | 25 ++++++++
 .../app/views/publisher_mention_view_spec.js  |  2 +-
 spec/models/person_spec.rb                    | 59 +++++++++++++++++++
 8 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/app/assets/javascripts/app/views/publisher/mention_view.js b/app/assets/javascripts/app/views/publisher/mention_view.js
index b1c6e39b0e..03e31820fa 100644
--- a/app/assets/javascripts/app/views/publisher/mention_view.js
+++ b/app/assets/javascripts/app/views/publisher/mention_view.js
@@ -32,7 +32,7 @@ app.views.PublisherMention = app.views.SearchBase.extend({
       typeaheadInput: this.typeaheadInput,
       customSearch: true,
       autoselect: true,
-      prefetch: true
+      remoteRoute: "/contacts/search"
     });
   },
 
diff --git a/app/assets/javascripts/app/views/search_base_view.js b/app/assets/javascripts/app/views/search_base_view.js
index 352ff7ddc0..b96a98af82 100644
--- a/app/assets/javascripts/app/views/search_base_view.js
+++ b/app/assets/javascripts/app/views/search_base_view.js
@@ -37,14 +37,6 @@ app.views.SearchBase = app.views.Base.extend({
       };
     }
 
-    if (options.prefetch) {
-      bloodhoundOptions.prefetch = {
-        url: "/contacts.json",
-        transform: this.transformBloodhoundResponse,
-        cache: false
-      };
-    }
-
     this.bloodhound = new Bloodhound(bloodhoundOptions);
   },
 
diff --git a/app/controllers/contacts_controller.rb b/app/controllers/contacts_controller.rb
index 3ffdadeac1..c233803412 100644
--- a/app/controllers/contacts_controller.rb
+++ b/app/controllers/contacts_controller.rb
@@ -23,6 +23,17 @@ class ContactsController < ApplicationController
     end
   end
 
+  def search
+    @people = Person.search(params[:q], current_user, only_contacts: true).limit(15)
+
+    respond_to do |format|
+      format.json do
+        @people = @people.limit(15)
+        render json: @people
+      end
+    end
+  end
+
   def spotlight
     @spotlight = true
     @people = Person.community_spotlight
diff --git a/app/models/person.rb b/app/models/person.rb
index 1fa3acd6c2..3ff161f43c 100644
--- a/app/models/person.rb
+++ b/app/models/person.rb
@@ -145,26 +145,23 @@ class Person < ActiveRecord::Base
     [where_clause, q_tokens]
   end
 
-  def self.search(query, user)
-    return self.where("1 = 0") if query.to_s.blank? || query.to_s.length < 2
-
-    sql, tokens = self.search_query_string(query)
-
-    Person.searchable(user).where(sql, *tokens).joins(
-      "LEFT OUTER JOIN contacts ON contacts.user_id = #{user.id} AND contacts.person_id = people.id"
-    ).includes(:profile).order(search_order)
-  end
-
-  # @return [Array<String>] postgreSQL and mysql deal with null values in orders differently, it seems.
-  def self.search_order
-    @search_order ||= Proc.new {
-      order = if AppConfig.postgres?
-        "ASC"
-      else
-        "DESC"
-      end
-      ["contacts.user_id #{order}", "profiles.last_name ASC", "profiles.first_name ASC"]
-    }.call
+  def self.search(search_str, user, only_contacts: false)
+    search_str.strip!
+    return none if search_str.blank? || search_str.size < 2
+
+    sql, tokens = search_query_string(search_str)
+
+    query = if only_contacts
+              joins(:contacts).where(contacts: {user_id: user.id})
+            else
+              joins(
+                "LEFT OUTER JOIN contacts ON contacts.user_id = #{user.id} AND contacts.person_id = people.id"
+              ).searchable(user)
+            end
+
+    query.where(sql, *tokens)
+         .includes(:profile)
+         .order(["contacts.user_id IS NULL", "profiles.last_name ASC", "profiles.first_name ASC"])
   end
 
   def name(opts = {})
diff --git a/config/routes.rb b/config/routes.rb
index cea28508f1..0695972fcb 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -155,6 +155,9 @@ Diaspora::Application.routes.draw do
 
 
   resources :contacts, only: %i(index)
+
+  get "contacts/search" => "contacts#search"
+
   resources :aspect_memberships, :only  => [:destroy, :create]
   resources :share_visibilities,  :only => [:update]
   resources :blocks, :only => [:create, :destroy]
diff --git a/spec/controllers/contacts_controller_spec.rb b/spec/controllers/contacts_controller_spec.rb
index 5b70a00147..c96131fbac 100644
--- a/spec/controllers/contacts_controller_spec.rb
+++ b/spec/controllers/contacts_controller_spec.rb
@@ -72,6 +72,31 @@ describe ContactsController, :type => :controller do
     end
   end
 
+  describe "#search" do
+    before do
+      @eugene = FactoryGirl.create(:person, profile: FactoryGirl.build(:profile, first_name: "Eugene", last_name: "W"))
+      bob.share_with(@eugene, bob.aspects.first)
+      @casey = FactoryGirl.create(:person, profile: FactoryGirl.build(:profile, first_name: "Casey", last_name: "W"))
+    end
+
+    describe "via json" do
+      it "succeeds" do
+        get :search, q: "Eugene", format: "json"
+        expect(response).to be_success
+      end
+
+      it "responds with json" do
+        get :search, q: "Eugene", format: "json"
+        expect(response.body).to eq([@eugene].to_json)
+      end
+
+      it "only returns contacts" do
+        get :search, q: "Casey", format: "json"
+        expect(response.body).to eq([].to_json)
+      end
+    end
+  end
+
   describe '#spotlight' do
     it 'succeeds' do
       get :spotlight
diff --git a/spec/javascripts/app/views/publisher_mention_view_spec.js b/spec/javascripts/app/views/publisher_mention_view_spec.js
index 985ec49e8a..571b08d898 100644
--- a/spec/javascripts/app/views/publisher_mention_view_spec.js
+++ b/spec/javascripts/app/views/publisher_mention_view_spec.js
@@ -19,7 +19,7 @@ describe("app.views.PublisherMention", function() {
       expect(call.args[0].typeaheadInput.selector).toBe("#publisher .typeahead-mention-box");
       expect(call.args[0].customSearch).toBeTruthy();
       expect(call.args[0].autoselect).toBeTruthy();
-      expect(call.args[0].prefetch).toBeTruthy();
+      expect(call.args[0].remoteRoute).toBe("/contacts/search");
     });
 
     it("calls bindTypeaheadEvents", function() {
diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb
index 04e45066f2..a941d5e0be 100644
--- a/spec/models/person_spec.rb
+++ b/spec/models/person_spec.rb
@@ -409,6 +409,65 @@ describe Person, :type => :model do
       people = Person.search("AAA", @user)
       expect(people.map { |p| p.name }).to eq([@casey_grippi, @yevgeniy_dodis, @robert_grimm, @eugene_weinstein].map { |p| p.name })
     end
+
+    context "only contacts" do
+      before do
+        @robert_contact = @user.contacts.create(person: @robert_grimm, aspects: [@user.aspects.first])
+        @eugene_contact = @user.contacts.create(person: @eugene_weinstein, aspects: [@user.aspects.first])
+        @invisible_contact = @user.contacts.create(person: @invisible_person, aspects: [@user.aspects.first])
+      end
+
+      it "orders results by last name" do
+        @robert_grimm.profile.first_name = "AAA"
+        @robert_grimm.profile.save!
+
+        @eugene_weinstein.profile.first_name = "AAA"
+        @eugene_weinstein.profile.save!
+
+        @casey_grippi.profile.first_name = "AAA"
+        @casey_grippi.profile.save!
+
+        people = Person.search("AAA", @user, only_contacts: true)
+        expect(people.map(&:name)).to eq([@robert_grimm, @eugene_weinstein].map(&:name))
+      end
+
+      it "returns nothing on an empty query" do
+        people = Person.search("", @user, only_contacts: true)
+        expect(people).to be_empty
+      end
+
+      it "returns nothing on a one-character query" do
+        people = Person.search("i", @user, only_contacts: true)
+        expect(people).to be_empty
+      end
+
+      it "returns results for partial names" do
+        people = Person.search("Eug", @user, only_contacts: true)
+        expect(people.count).to eq(1)
+        expect(people.first).to eq(@eugene_weinstein)
+
+        people = Person.search("wEi", @user, only_contacts: true)
+        expect(people.count).to eq(1)
+        expect(people.first).to eq(@eugene_weinstein)
+
+        @user.contacts.create(person: @casey_grippi, aspects: [@user.aspects.first])
+        people = Person.search("gri", @user, only_contacts: true)
+        expect(people.count).to eq(2)
+        expect(people.first).to eq(@robert_grimm)
+        expect(people.second).to eq(@casey_grippi)
+      end
+
+      it "returns results for full names" do
+        people = Person.search("Robert Grimm", @user, only_contacts: true)
+        expect(people.count).to eq(1)
+        expect(people.first).to eq(@robert_grimm)
+      end
+
+      it "returns results for Diaspora handles" do
+        people = Person.search(@robert_grimm.diaspora_handle, @user, only_contacts: true)
+        expect(people).to eq([@robert_grimm])
+      end
+    end
   end
 
   context 'people finders for webfinger' do
-- 
GitLab