diff --git a/app/models/user_search.rb b/app/models/user_search.rb index 2b9b05d0f85..aafc0d3e812 100644 --- a/app/models/user_search.rb +++ b/app/models/user_search.rb @@ -64,10 +64,11 @@ class UserSearch def search_ids users = Set.new - # 1. exact username matches + # 1. exact username matches active in the past year if @term.present? scoped_users .where(username_lower: @term) + .where('last_seen_at > ?', 1.year.ago) .limit(@limit) .pluck(:id) .each { |id| users << id } diff --git a/spec/models/user_search_spec.rb b/spec/models/user_search_spec.rb index 5dad83c32be..d631d59d972 100644 --- a/spec/models/user_search_spec.rb +++ b/spec/models/user_search_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'rails_helper' +require "rails_helper" describe UserSearch do @@ -11,170 +11,163 @@ describe UserSearch do fab!(:topic2) { Fabricate :topic } fab!(:topic3) { Fabricate :topic } fab!(:topic4) { Fabricate :topic } - fab!(:user1) { Fabricate :user, username: "mrb", name: "Michael Madsen", last_seen_at: 10.days.ago } - fab!(:user2) { Fabricate :user, username: "mrblue", name: "Eddie Code", last_seen_at: 9.days.ago } - fab!(:user3) { Fabricate :user, username: "mrorange", name: "Tim Roth", last_seen_at: 8.days.ago } - fab!(:user4) { Fabricate :user, username: "mrpink", name: "Steve Buscemi", last_seen_at: 7.days.ago } - fab!(:user5) { Fabricate :user, username: "mrbrown", name: "Quentin Tarantino", last_seen_at: 6.days.ago } - fab!(:user6) { Fabricate :user, username: "mrwhite", name: "Harvey Keitel", last_seen_at: 5.days.ago } - fab!(:inactive) { Fabricate :user, username: "Ghost", active: false } + fab!(:mr_b) { Fabricate :user, username: "mrb", name: "Michael Madsen", last_seen_at: 10.days.ago } + fab!(:mr_blue) { Fabricate :user, username: "mrblue", name: "Eddie Code", last_seen_at: 9.days.ago } + fab!(:mr_orange) { Fabricate :user, username: "mrorange", name: "Tim Roth", last_seen_at: 8.days.ago } + fab!(:mr_pink) { Fabricate :user, username: "mrpink", name: "Steve Buscemi", last_seen_at: 7.days.ago } + fab!(:mr_brown) { Fabricate :user, username: "mrbrown", name: "Quentin Tarantino", last_seen_at: 6.days.ago } + fab!(:mr_white) { Fabricate :user, username: "mrwhite", name: "Harvey Keitel", last_seen_at: 5.days.ago } + fab!(:inactive) { Fabricate :user, username: "Ghost", active: false } fab!(:admin) { Fabricate :admin, username: "theadmin" } fab!(:moderator) { Fabricate :moderator, username: "themod" } fab!(:staged) { Fabricate :staged } def search_for(*args) - UserSearch.new(*args).search + # mapping "username" so it's easier to debug + UserSearch.new(*args).search.map(&:username) end - context 'with a secure category' do - fab!(:group) { Fabricate :group } - fab!(:user) { Fabricate :user } - fab!(:searching_user) { Fabricate :user } + context "with a secure category" do + fab!(:user) { Fabricate(:user) } + fab!(:searching_user) { Fabricate(:user) } + fab!(:group) { Fabricate(:group) } + fab!(:category) { Fabricate(:category, read_restricted: true, user: user) } + before_all do + Fabricate(:category_group, category: category, group: group) + group.add(user) group.add(searching_user) group.save end - fab!(:category) { Fabricate(:category, - read_restricted: true, - user: user) - } - before_all { Fabricate(:category_group, category: category, group: group) } - it 'autocompletes with people in the category' do + it "autocompletes with people in the category" do results = search_for("", searching_user: searching_user, category_id: category.id) - - expect(user.username).to eq(results[0].username) - expect(results.length).to eq(1) + expect(results).to eq [user.username] end - it 'will lookup the category from the topic id' do + it "will lookup the category from the topic id" do topic = Fabricate(:topic, category: category) - _post = Fabricate(:post, user: topic.user, topic: topic) + Fabricate(:post, user: topic.user, topic: topic) results = search_for("", searching_user: searching_user, topic_id: topic.id) - expect(results.length).to eq(2) - - expect(results.map(&:username)).to contain_exactly( - user.username, topic.user.username - ) + expect(results).to eq [topic.user, user].map(&:username) end - it 'will raise an error if the user cannot see the category' do + it "will raise an error if the user cannot see the category" do expect do search_for("", searching_user: Fabricate(:user), category_id: category.id) end.to raise_error(Discourse::InvalidAccess) end - it 'will respect the group member visibility setting' do + it "will respect the group member visibility setting" do group.update(members_visibility_level: Group.visibility_levels[:owners]) results = search_for("", searching_user: searching_user, category_id: category.id) - expect(results.length).to eq(0) + expect(results).to be_blank group.add_owner(searching_user) results = search_for("", searching_user: searching_user, category_id: category.id) - expect(results.length).to eq(1) + expect(results).to eq [user.username] end end - it 'allows for correct underscore searching' do - Fabricate(:user, username: 'Under_Score') - Fabricate(:user, username: 'undertaker') + it "allows for correct underscore searching" do + Fabricate(:user, username: "undertaker") + under_score = Fabricate(:user, username: "Under_Score") - expect(search_for("under_sc").length).to eq(1) - expect(search_for("under_").length).to eq(1) + expect(search_for("under_sc")).to eq [under_score.username] + expect(search_for("under_")).to eq [under_score.username] end - it 'allows filtering by group' do + it "allows filtering by group" do + sam = Fabricate(:user, username: "sam") + Fabricate(:user, username: "samantha") + group = Fabricate(:group) - sam = Fabricate(:user, username: 'sam') - _samantha = Fabricate(:user, username: 'samantha') group.add(sam) results = search_for("sam", groups: [group]) - expect(results.count).to eq(1) + expect(results).to eq [sam.username] end - it 'allows filtering by multiple groups' do + it "allows filtering by multiple groups" do + sam = Fabricate(:user, username: "sam") + samantha = Fabricate(:user, username: "samantha") + group_1 = Fabricate(:group) - sam = Fabricate(:user, username: 'sam') - group_2 = Fabricate(:group) - samantha = Fabricate(:user, username: 'samantha') group_1.add(sam) + + group_2 = Fabricate(:group) group_2.add(samantha) results = search_for("sam", groups: [group_1, group_2]) - expect(results.count).to eq(2) + expect(results).to eq [sam, samantha].map(&:username) end context "with seed data" do - fab!(:post1) { Fabricate :post, user: user1, topic: topic } - fab!(:post2) { Fabricate :post, user: user2, topic: topic2 } - fab!(:post3) { Fabricate :post, user: user3, topic: topic } - fab!(:post4) { Fabricate :post, user: user4, topic: topic } - fab!(:post5) { Fabricate :post, user: user5, topic: topic3 } - fab!(:post6) { Fabricate :post, user: user6, topic: topic } + fab!(:post1) { Fabricate :post, user: mr_b, topic: topic } + fab!(:post2) { Fabricate :post, user: mr_blue, topic: topic2 } + fab!(:post3) { Fabricate :post, user: mr_orange, topic: topic } + fab!(:post4) { Fabricate :post, user: mr_pink, topic: topic } + fab!(:post5) { Fabricate :post, user: mr_brown, topic: topic3 } + fab!(:post6) { Fabricate :post, user: mr_white, topic: topic } fab!(:post7) { Fabricate :post, user: staged, topic: topic4 } - before { user6.update(suspended_at: 1.day.ago, suspended_till: 1.year.from_now) } + before { mr_white.update(suspended_at: 1.day.ago, suspended_till: 1.year.from_now) } it "can search by name and username" do # normal search - results = search_for(user1.name.split(" ").first) - expect(results.size).to eq(1) - expect(results.first.username).to eq(user1.username) + results = search_for(mr_b.name.split.first) + expect(results).to eq [mr_b.username] # lower case - results = search_for(user1.name.split(" ").first.downcase) - expect(results.size).to eq(1) - expect(results.first).to eq(user1) + results = search_for(mr_b.name.split.first.downcase) + expect(results).to eq [mr_b.username] # username - results = search_for(user4.username) - expect(results.size).to eq(1) - expect(results.first).to eq(user4) + results = search_for(mr_pink.username) + expect(results).to eq [mr_pink.username] # case insensitive - results = search_for(user4.username.upcase) - expect(results.size).to eq(1) - expect(results.first).to eq(user4) + results = search_for(mr_pink.username.upcase) + expect(results).to eq [mr_pink.username] end it "handles substring search correctly" do - # substrings - # only staff members see suspended users in results results = search_for("mr") - expect(results.size).to eq(5) - expect(results).not_to include(user6) - expect(search_for("mr", searching_user: user1).size).to eq(5) + expect(results).to eq [mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username) + + results = search_for("mr", searching_user: mr_b) + expect(results).to eq [mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username) + + # only staff members see suspended users in results + results = search_for("mr", searching_user: moderator) + expect(results).to eq [mr_white, mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username) results = search_for("mr", searching_user: admin) - expect(results.size).to eq(6) - expect(results).to include(user6) - expect(search_for("mr", searching_user: moderator).size).to eq(6) + expect(results).to eq [mr_white, mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username) - results = search_for(user1.username, searching_user: admin) - expect(results.size).to eq(3) + results = search_for(mr_b.username, searching_user: admin) + expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username) results = search_for("MR", searching_user: admin) - expect(results.size).to eq(6) + expect(results).to eq [mr_white, mr_brown, mr_pink, mr_orange, mr_blue, mr_b].map(&:username) results = search_for("MRB", searching_user: admin, limit: 2) - expect(results.size).to eq(2) + expect(results).to eq [mr_b, mr_brown].map(&:username) end it "prioritises topic participants" do - # topic priority - results = search_for(user1.username, topic_id: topic.id) - expect(results.first).to eq(user1) + results = search_for(mr_b.username, topic_id: topic.id) + expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username) - results = search_for(user1.username, topic_id: topic2.id) - expect(results[1]).to eq(user2) + results = search_for(mr_b.username, topic_id: topic2.id) + expect(results).to eq [mr_b, mr_blue, mr_brown].map(&:username) - results = search_for(user1.username, topic_id: topic3.id) - expect(results[1]).to eq(user5) + results = search_for(mr_b.username, topic_id: topic3.id) + expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username) end it "only reveals topic participants to people with permission" do @@ -187,37 +180,46 @@ describe UserSearch do # Random user, does not have access expect do - search_for("", topic_id: pm_topic.id, searching_user: user1) + search_for("", topic_id: pm_topic.id, searching_user: mr_b) end.to raise_error(Discourse::InvalidAccess) - pm_topic.invite(pm_topic.user, user1.username) - results = search_for("", topic_id: pm_topic.id, searching_user: user1) - expect(results.length).to eq(1) - expect(results[0]).to eq(pm_topic.user) + pm_topic.invite(pm_topic.user, mr_b.username) + + results = search_for("", topic_id: pm_topic.id, searching_user: mr_b) + expect(results).to eq [pm_topic.user.username] end it "only searches by name when enabled" do # When searching by name is enabled, it returns the record SiteSetting.enable_names = true results = search_for("Tarantino") - expect(results.size).to eq(1) + expect(results).to eq [mr_brown.username] results = search_for("coding") - expect(results.size).to eq(0) + expect(results).to be_blank results = search_for("z") - expect(results.size).to eq(0) + expect(results).to be_blank # When searching by name is disabled, it will not return the record SiteSetting.enable_names = false results = search_for("Tarantino") - expect(results.size).to eq(0) + expect(results).to be_blank end it "prioritises exact matches" do - # find an exact match first results = search_for("mrB") - expect(results.first.username).to eq(user1.username) + expect(results).to eq [mr_b, mr_brown, mr_blue].map(&:username) + end + + it "doesn't prioritises exact matches for users who haven't been seen in more than 1 year" do + abcdef = Fabricate(:user, username: "abcdef", last_seen_at: 2.days.ago) + abcde = Fabricate(:user, username: "abcde", last_seen_at: 2.weeks.ago) + abcd = Fabricate(:user, username: "abcd", last_seen_at: 2.months.ago) + abc = Fabricate(:user, username: "abc", last_seen_at: 2.years.ago) + + results = search_for("abc") + expect(results).to eq [abcdef, abcde, abcd, abc].map(&:username) end it "does not include self, staged or inactive" do @@ -230,12 +232,11 @@ describe UserSearch do expect(results).to be_blank results = search_for(staged.username, include_staged_users: true) - expect(results.first.username).to eq(staged.username) + expect(results).to eq [staged.username] - results = search_for("", topic_id: topic.id, searching_user: user1) - - # mrb is omitted, mrb is current user - expect(results.map(&:username)).to eq(["mrpink", "mrorange"]) + # mrb is omitted since they're the searching user + results = search_for("", topic_id: topic.id, searching_user: mr_b) + expect(results).to eq [mr_pink, mr_orange].map(&:username) end end end