diff --git a/Changelog.md b/Changelog.md
index e6779a10b28a55252931fbbae4a9c0e01a225c5f..705d5e6167c7b553c0e00b1f9c127fcb3000f6a9 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -169,6 +169,7 @@ before.
* Add 'Be excellent to each other!' to the sidebar [#6914](https://github.com/diaspora/diaspora/pull/6910)
* Expose Sidekiq dead queue configuration options
* Properly support pluralization in timeago strings [#6926](https://github.com/diaspora/diaspora/pull/6926)
+* Return all contacts in people search [#6951](https://github.com/diaspora/diaspora/pull/6951)
# 0.5.11.0
diff --git a/app/assets/javascripts/app/views/publisher/mention_view.js b/app/assets/javascripts/app/views/publisher/mention_view.js
index f19ec7d1e24e6da57c0b46d82dd519a5ecaa2e0e..2a1c65591956b20c5853741d79df232a6ab7813d 100644
--- a/app/assets/javascripts/app/views/publisher/mention_view.js
+++ b/app/assets/javascripts/app/views/publisher/mention_view.js
@@ -31,7 +31,8 @@ app.views.PublisherMention = app.views.SearchBase.extend({
app.views.SearchBase.prototype.initialize.call(this, {
typeaheadInput: this.typeaheadInput,
customSearch: true,
- autoselect: true
+ autoselect: true,
+ remoteRoute: "/contacts"
});
},
diff --git a/app/assets/javascripts/app/views/search_base_view.js b/app/assets/javascripts/app/views/search_base_view.js
index d70f21cebb4df05e74c0b60b77ea70e58dba467c..b96a98af82439952cb60afd3d7fdb6edc55feeb5 100644
--- a/app/assets/javascripts/app/views/search_base_view.js
+++ b/app/assets/javascripts/app/views/search_base_view.js
@@ -25,11 +25,6 @@ app.views.SearchBase = app.views.Base.extend({
return this.bloodhoundTokenizer(datum.name).concat(datum.handle);
}.bind(this),
queryTokenizer: Bloodhound.tokenizers.whitespace,
- prefetch: {
- url: "/contacts.json",
- transform: this.transformBloodhoundResponse,
- cache: false
- },
sufficient: 5
};
diff --git a/app/controllers/contacts_controller.rb b/app/controllers/contacts_controller.rb
index 3ffdadeac1902ac4b90ab8d78764fb7d0bba4896..d3734da9b765ba0fddbd0c273b634791b03d824c 100644
--- a/app/controllers/contacts_controller.rb
+++ b/app/controllers/contacts_controller.rb
@@ -14,11 +14,10 @@ class ContactsController < ApplicationController
# Used by the mobile site
format.mobile { set_up_contacts_mobile }
- # Used to populate mentions in the publisher
+ # Used for mentions in the publisher
format.json {
- aspect_ids = params[:aspect_ids] || current_user.aspects.map(&:id)
- @people = Person.all_from_aspects(aspect_ids, current_user).for_json
- render :json => @people.to_json
+ @people = Person.search(params[:q], current_user, only_contacts: true).limit(15)
+ render json: @people
}
end
end
diff --git a/app/models/person.rb b/app/models/person.rb
index 5b400cd30b5122b321fbc351971a4e65f98ea6bd..3ff161f43cc327569228b44ae682af6c768dd25b 100644
--- a/app/models/person.rb
+++ b/app/models/person.rb
@@ -56,7 +56,9 @@ class Person < ActiveRecord::Base
validates :serialized_public_key, :presence => true
validates :diaspora_handle, :uniqueness => true
- scope :searchable, -> { joins(:profile).where(:profiles => {:searchable => true}) }
+ scope :searchable, -> (user) {
+ joins(:profile).where("profiles.searchable = true OR contacts.user_id = ?", user.id)
+ }
scope :remote, -> { where('people.owner_id IS NULL') }
scope :local, -> { where('people.owner_id IS NOT NULL') }
scope :for_json, -> {
@@ -143,27 +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
+ def self.search(search_str, user, only_contacts: false)
+ search_str.strip!
+ return none if search_str.blank? || search_str.size < 2
- sql, tokens = self.search_query_string(query)
+ sql, tokens = search_query_string(search_str)
- Person.searchable.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
+ 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
- # @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
+ 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/spec/controllers/contacts_controller_spec.rb b/spec/controllers/contacts_controller_spec.rb
index 5b70a00147d5f963baf47a0cd37054b29d33e0df..0f343e43b49f4e553da72e9c699613b7b233364b 100644
--- a/spec/controllers/contacts_controller_spec.rb
+++ b/spec/controllers/contacts_controller_spec.rb
@@ -49,25 +49,26 @@ describe ContactsController, :type => :controller do
end
end
- context 'format json' do
- it 'assumes all aspects if none are specified' do
- get :index, :format => 'json'
- expect(assigns[:people].map(&:id)).to match_array(bob.contacts.map { |c| c.person.id })
- expect(response).to be_success
+ context "format json" do
+ before do
+ @person1 = FactoryGirl.create(:person)
+ bob.share_with(@person1, bob.aspects.first)
+ @person2 = FactoryGirl.create(:person)
end
- it 'returns the contacts for multiple aspects' do
- get :index, :aspect_ids => bob.aspect_ids, :format => 'json'
- expect(assigns[:people].map(&:id)).to match_array(bob.contacts.map { |c| c.person.id })
+ it "succeeds" do
+ get :index, q: @person1.first_name, format: "json"
expect(response).to be_success
end
- it 'does not return duplicate contacts' do
- aspect = bob.aspects.create(:name => 'hilarious people')
- aspect.contacts << bob.contact_for(eve.person)
- get :index, :format => 'json', :aspect_ids => bob.aspect_ids
- expect(assigns[:people].map { |p| p.id }.uniq).to eq(assigns[:people].map { |p| p.id })
- expect(assigns[:people].map(&:id)).to match_array(bob.contacts.map { |c| c.person.id })
+ it "responds with json" do
+ get :index, q: @person1.first_name, format: "json"
+ expect(response.body).to eq([@person1].to_json)
+ end
+
+ it "only returns contacts" do
+ get :index, q: @person2.first_name, format: "json"
+ expect(response.body).to eq([].to_json)
end
end
end
diff --git a/spec/factories.rb b/spec/factories.rb
index 1c1c5e839ec497ec5f4799b62e446a9803877029..4b5363f70663fbf47d5a622b249ad7c8b3487bba 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -52,12 +52,6 @@ FactoryGirl.define do
end
end
- factory :searchable_person, :parent => :person do
- after(:build) do |person|
- person.profile = FactoryGirl.build(:profile, :person => person, :searchable => true)
- end
- end
-
factory :like do
association :author, :factory => :person
association :target, :factory => :status_message
diff --git a/spec/javascripts/app/views/publisher_mention_view_spec.js b/spec/javascripts/app/views/publisher_mention_view_spec.js
index 68397e36e333e090ec856c94730b20525f73bdc1..a28a393e7e3516b4b713c0201df865f93dae426a 100644
--- a/spec/javascripts/app/views/publisher_mention_view_spec.js
+++ b/spec/javascripts/app/views/publisher_mention_view_spec.js
@@ -19,6 +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].remoteRoute).toBe("/contacts");
});
it("calls bindTypeaheadEvents", function() {
diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb
index c376da2b188d06f873c7daed69cb02f91fe24704..a941d5e0be306df11cb58671d3331f0436a09e6f 100644
--- a/spec/models/person_spec.rb
+++ b/spec/models/person_spec.rb
@@ -294,10 +294,11 @@ describe Person, :type => :model do
user_profile.last_name = "asdji"
user_profile.save
- @robert_grimm = FactoryGirl.build(:searchable_person)
- @eugene_weinstein = FactoryGirl.build(:searchable_person)
- @yevgeniy_dodis = FactoryGirl.build(:searchable_person)
- @casey_grippi = FactoryGirl.build(:searchable_person)
+ @robert_grimm = FactoryGirl.build(:person)
+ @eugene_weinstein = FactoryGirl.build(:person)
+ @yevgeniy_dodis = FactoryGirl.build(:person)
+ @casey_grippi = FactoryGirl.build(:person)
+ @invisible_person = FactoryGirl.build(:person)
@robert_grimm.profile.first_name = "Robert"
@robert_grimm.profile.last_name = "Grimm"
@@ -318,7 +319,14 @@ describe Person, :type => :model do
@casey_grippi.profile.last_name = "Grippi"
@casey_grippi.profile.save
@casey_grippi.reload
+
+ @invisible_person.profile.first_name = "Johnson"
+ @invisible_person.profile.last_name = "Invisible"
+ @invisible_person.profile.searchable = false
+ @invisible_person.profile.save
+ @invisible_person.reload
end
+
it 'orders results by last name' do
@robert_grimm.profile.first_name = "AAA"
@robert_grimm.profile.save!
@@ -367,10 +375,15 @@ describe Person, :type => :model do
expect(people.first).to eq(@casey_grippi)
end
- it 'only displays searchable people' do
- invisible_person = FactoryGirl.build(:person, :profile => FactoryGirl.build(:profile, :searchable => false, :first_name => "johnson"))
- expect(Person.search("johnson", @user)).not_to include invisible_person
- expect(Person.search("", @user)).not_to include invisible_person
+ it "doesn't display people that are neither searchable nor contacts" do
+ expect(Person.search("Johnson", @user)).to be_empty
+ end
+
+ it "displays contacts that are not searchable" do
+ @user.contacts.create(person: @invisible_person, aspects: [@user.aspects.first])
+ people = Person.search("Johnson", @user)
+ expect(people.count).to eq(1)
+ expect(people.first).to eq(@invisible_person)
end
it 'returns results for Diaspora handles' do
@@ -396,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