diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 433ef60d890..24d2fe99e80 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -288,44 +288,14 @@ class UsersController < ApplicationController end def search_users - - term = (params[:term] || "").strip.downcase + term = params[:term].to_s.strip topic_id = params[:topic_id] topic_id = topic_id.to_i if topic_id - sql = "select username, name, email from users u " - if topic_id - sql << "left join (select distinct p.user_id from posts p where topic_id = :topic_id) s on - s.user_id = u.id " - end + results = UserSearch.search term, topic_id - if term.length > 0 - sql << "where username_lower like :term_like or - to_tsvector('simple', name) @@ - to_tsquery('simple', - regexp_replace( - regexp_replace( - cast(plainto_tsquery(:term) as text) - ,'\''(?: |$)', ':*''', 'g'), - '''', '', 'g') - ) " - - end - - sql << "order by case when username_lower = :term then 0 else 1 end asc, " - if topic_id - sql << " case when s.user_id is null then 0 else 1 end desc, " - end - - sql << " case when last_seen_at is null then 0 else 1 end desc, last_seen_at desc, username asc limit(20)" - - results = User.exec_sql(sql, topic_id: topic_id, term_like: "#{term}%", term: term) - results = results.map do |r| - r["avatar_template"] = User.avatar_template(r["email"]) - r.delete("email") - r - end - render :json => results + render json: { users: results.as_json( only: [ :username, :name ], + methods: :avatar_template ) } end private diff --git a/app/models/user_search.rb b/app/models/user_search.rb new file mode 100644 index 00000000000..42a01fc08e0 --- /dev/null +++ b/app/models/user_search.rb @@ -0,0 +1,43 @@ +class UserSearch + + def self.search term, topic_id = nil + User.find_by_sql sql(term, topic_id) + end + + private + + def self.sql term, topic_id + sql = "select id, username, name, email from users u " + if topic_id + sql << "left join (select distinct p.user_id from posts p where topic_id = :topic_id) s on + s.user_id = u.id " + end + + if term.present? + sql << "where username ilike :term_like or + to_tsvector('simple', name) @@ + to_tsquery('simple', + regexp_replace( + regexp_replace( + cast(plainto_tsquery(:term) as text) + ,'\''(?: |$)', ':*''', 'g'), + '''', '', 'g') + ) " + + end + + sql << "order by case when username_lower = :term then 0 else 1 end asc, " + if topic_id + sql << " case when s.user_id is null then 0 else 1 end desc, " + end + + sql << " case when last_seen_at is null then 0 else 1 end desc, last_seen_at desc, username asc limit(20)" + + sanitize_sql_array(sql, topic_id: topic_id, term_like: "#{term}%", term: term) + end + + def self.sanitize_sql_array *args + ActiveRecord::Base.send(:sanitize_sql_array, args) + end + +end \ No newline at end of file diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index b67f6f7af2e..69996d5a583 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' -describe UsersController do +describe UsersController do before do UsersController.any_instance.stubs(:honeypot_value).returns(nil) UsersController.any_instance.stubs(:challenge_value).returns(nil) end - describe '.show' do + describe '.show' do let!(:user) { log_in } it 'returns success' do @@ -52,7 +52,7 @@ describe UsersController do it 'sets a flash error' do flash[:error].should be_present - end + end end context 'valid token' do @@ -74,7 +74,7 @@ describe UsersController do it 'logs in as the user' do session[:current_user_id].should be_present end - end + end end describe '.activate_account' do @@ -90,7 +90,7 @@ describe UsersController do it 'sets a flash error' do flash[:error].should be_present - end + end end context 'valid token' do @@ -102,13 +102,13 @@ describe UsersController do end it 'enqueues a welcome message if the user object indicates so' do - user.send_welcome_message = true + user.send_welcome_message = true user.expects(:enqueue_welcome_message).with('welcome_user') get :activate_account, token: 'asdfasdf' end it "doesn't enqueue the welcome message if the object returns false" do - user.send_welcome_message = false + user.send_welcome_message = false user.expects(:enqueue_welcome_message).with('welcome_user').never get :activate_account, token: 'asdfasdf' end @@ -131,12 +131,12 @@ describe UsersController do it 'logs in as the user' do session[:current_user_id].should be_present - end + end it "doesn't set @needs_approval" do assigns[:needs_approval].should be_blank - end - + end + end context 'must_approve_users' do @@ -166,7 +166,7 @@ describe UsersController do end end - describe '.change_email' do + describe '.change_email' do let(:new_email) { 'bubblegum@adventuretime.ooo' } it "requires you to be logged in" do @@ -223,11 +223,11 @@ describe UsersController do it 'sets a flash error' do flash[:error].should be_present - end + end it "doesn't log in the user" do session[:current_user_id].should be_blank - end + end end context 'valid token' do @@ -259,17 +259,17 @@ describe UsersController do SiteSetting.expects(:must_approve_users?).returns(true) put :password_reset, token: 'asdfasdf', password: 'newpassword' session[:current_user_id].should be_blank - end + end end end - describe '.create' do - before do + describe '.create' do + before do @user = Fabricate.build(:user) - @user.password = "strongpassword" + @user.password = "strongpassword" Mothership.stubs(:register_nickname).returns([true, nil]) end @@ -282,7 +282,7 @@ describe UsersController do it "doesn't send a welcome email" do User.any_instance.expects(:enqueue_welcome_message).with('welcome_user').never xhr :post, :create, :name => @user.name, :username => @user.username, :password => "strongpassword", :email => @user.email - end + end end context 'when creating an active user (confirmed email)' do @@ -294,7 +294,7 @@ describe UsersController do it 'should enqueue a signup email' do User.any_instance.expects(:enqueue_welcome_message).with('welcome_user') xhr :post, :create, :name => @user.name, :username => @user.username, :password => "strongpassword", :email => @user.email - end + end it "should be logged in" do User.any_instance.expects(:enqueue_welcome_message) @@ -331,16 +331,16 @@ describe UsersController do xhr :post, :create, :name => @user.name, :username => @user.username, :password => "strongpassword", :email => @user.email end - it 'should succeed' do + it 'should succeed' do should respond_with(:success) end it 'has the proper JSON' do json = JSON::parse(response.body) - json["success"].should be_true + json["success"].should be_true end - it 'should not result in an active account' do + it 'should not result in an active account' do User.where(username: @user.username).first.active.should be_false end end @@ -391,7 +391,7 @@ describe UsersController do let(:new_username) { "#{user.username}1234" } it 'raises an error without a new_username param' do - lambda { xhr :put, :username, username: user.username }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :put, :username, username: user.username }.should raise_error(Discourse::InvalidParameters) end it 'raises an error when you don\'t have permission to change the user' do @@ -402,7 +402,7 @@ describe UsersController do it 'raises an error when change_username fails' do User.any_instance.expects(:change_username).with(new_username).returns(false) - lambda { xhr :put, :username, username: user.username, new_username: new_username }.should raise_error(Discourse::InvalidParameters) + lambda { xhr :put, :username, username: user.username, new_username: new_username }.should raise_error(Discourse::InvalidParameters) end it 'should succeed when the change_username returns true' do @@ -671,4 +671,36 @@ describe UsersController do end + describe "search_users" do + + let(:topic) { Fabricate :topic } + let(:user) { Fabricate :user, username: "joecabot", name: "Lawrence Tierney" } + + before do + Fabricate :post, user: user, topic: topic + end + + it "searches when provided the term only" do + xhr :post, :search_users, term: user.name.split(" ").last + response.should be_success + json = JSON.parse(response.body) + json["users"].map { |u| u["username"] }.should include(user.username) + end + + it "searches when provided the topic only" do + xhr :post, :search_users, topic_id: topic.id + response.should be_success + json = JSON.parse(response.body) + json["users"].map { |u| u["username"] }.should include(user.username) + end + + it "searches when provided the term and topic" do + xhr :post, :search_users, term: user.name.split(" ").last, topic_id: topic.id + response.should be_success + json = JSON.parse(response.body) + json["users"].map { |u| u["username"] }.should include(user.username) + end + + end + end diff --git a/spec/models/user_search_spec.rb b/spec/models/user_search_spec.rb new file mode 100644 index 00000000000..e16cd274fd3 --- /dev/null +++ b/spec/models/user_search_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +describe UserSearch do + + let(:topic) { Fabricate :topic } + let(:topic2) { Fabricate :topic } + let(:topic3) { Fabricate :topic } + let(:user1) { Fabricate :user, username: "mrblonde", name: "Michael Madsen" } + let(:user2) { Fabricate :user, username: "mrblue", name: "Eddie Bunker" } + let(:user3) { Fabricate :user, username: "mrorange", name: "Tim Roth" } + let(:user4) { Fabricate :user, username: "mrpink", name: "Steve Buscemi" } + let(:user5) { Fabricate :user, username: "mrbrown", name: "Quentin Tarantino" } + let(:user6) { Fabricate :user, username: "mrwhite", name: "Harvey Keitel" } + + before do + Fabricate :post, user: user1, topic: topic + Fabricate :post, user: user2, topic: topic2 + Fabricate :post, user: user3, topic: topic + Fabricate :post, user: user4, topic: topic + Fabricate :post, user: user5, topic: topic3 + Fabricate :post, user: user6, topic: topic + end + + context "all user search" do + it "searches the user's name" do + results = UserSearch.search user1.name.split(" ").first + results.size.should == 1 + results.first.should == user1 + end + + it "searches the user's name case insensitive" do + results = UserSearch.search user1.name.split(" ").first.downcase + results.size.should == 1 + results.first.should == user1 + end + + it "searches the user's username" do + results = UserSearch.search user4.username + results.size.should == 1 + results.first.should == user4 + end + + it "searches the user's username case insensitive" do + results = UserSearch.search user4.username.upcase + results.size.should == 1 + results.first.should == user4 + end + + it "searches the user's username substring" do + results = UserSearch.search "mr" + results.size.should == 6 + + results = UserSearch.search "mrb" + results.size.should == 3 + end + + it "searches the user's username substring upper case" do + results = UserSearch.search "MR" + results.size.should == 6 + + results = UserSearch.search "MRB" + results.size.should == 3 + end + end + + context "sort order respects users with posts on the topic" do + it "Mr. Blond is first when searching his topic" do + results = UserSearch.search "mrb", topic.id + results.first.should == user1 + end + + it "Mr. Blue is first when searching his topic" do + results = UserSearch.search "mrb", topic2.id + results.first.should == user2 + end + + it "Mr. Brown is first when searching his topic" do + results = UserSearch.search "mrb", topic3.id + results.first.should == user5 + end + end + +end