From 33269c4172f43ea79c1720b00b4d4d33bb41a24e Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 20 Feb 2019 17:28:12 +1100 Subject: [PATCH] FEATURE: do no search for groups unless a term is specified Do not allow `/u/search/users.json` to list any group matches unless a specific `term` is specified in the API call. Adding groups should always be done when an actual search term exists, blank search is only supported for users within a topic --- app/controllers/users_controller.rb | 4 +++ spec/requests/users_controller_spec.rb | 38 +++++++++++++++++++------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5d7c99b13ca..efe426fb5f2 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -867,6 +867,10 @@ class UsersController < ApplicationController include_groups = params[:include_groups] == "true" + # blank term is only handy for in-topic search of users after @ + # we do not want group results ever if term is blank + include_groups = groups = nil if term.blank? + if include_groups || groups groups = Group.search_groups(term, groups: groups) groups = groups.where(visibility_level: Group.visibility_levels[:public]) if include_groups diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 11dda380c03..3d278abee76 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2802,7 +2802,8 @@ describe UsersController do Fabricate(:group, mentionable_level: 99, messageable_level: 0, - visibility_level: 0 + visibility_level: 0, + name: 'aaa1' ) end @@ -2810,14 +2811,16 @@ describe UsersController do Fabricate(:group, mentionable_level: 99, messageable_level: 0, - visibility_level: 1 + visibility_level: 1, + name: 'aaa2' ) end let!(:messageable_group) do Fabricate(:group, mentionable_level: 0, - messageable_level: 99 + messageable_level: 99, + name: 'aaa3' ) end @@ -2826,11 +2829,20 @@ describe UsersController do sign_in(user) end - it "only returns visible groups" do + it "does not search for groups if there is no term" do get "/u/search/users.json", params: { include_groups: "true" } expect(response.status).to eq(200) + groups = JSON.parse(response.body)["groups"] + expect(groups).to eq(nil) + end + + it "only returns visible groups" do + get "/u/search/users.json", params: { include_groups: "true", term: 'a' } + + expect(response.status).to eq(200) + groups = JSON.parse(response.body)["groups"] expect(groups.map { |group| group['name'] }) @@ -2840,7 +2852,8 @@ describe UsersController do it "doesn't search for groups" do get "/u/search/users.json", params: { include_mentionable_groups: 'false', - include_messageable_groups: 'false' + include_messageable_groups: 'false', + term: 'a' } expect(response.status).to eq(200) @@ -2850,7 +2863,8 @@ describe UsersController do it "searches for messageable groups" do get "/u/search/users.json", params: { include_mentionable_groups: 'false', - include_messageable_groups: 'true' + include_messageable_groups: 'true', + term: 'a' } expect(response.status).to eq(200) @@ -2862,7 +2876,8 @@ describe UsersController do it 'searches for mentionable groups' do get "/u/search/users.json", params: { include_messageable_groups: 'false', - include_mentionable_groups: 'true' + include_mentionable_groups: 'true', + term: 'a' } expect(response.status).to eq(200) @@ -2878,7 +2893,8 @@ describe UsersController do it 'should not include mentionable/messageable groups' do get "/u/search/users.json", params: { include_mentionable_groups: 'false', - include_messageable_groups: 'false' + include_messageable_groups: 'false', + term: 'a' } expect(response.status).to eq(200) @@ -2886,7 +2902,8 @@ describe UsersController do get "/u/search/users.json", params: { include_mentionable_groups: 'false', - include_messageable_groups: 'true' + include_messageable_groups: 'true', + term: 'a' } expect(response.status).to eq(200) @@ -2894,7 +2911,8 @@ describe UsersController do get "/u/search/users.json", params: { include_messageable_groups: 'false', - include_mentionable_groups: 'true' + include_mentionable_groups: 'true', + term: 'a' } expect(response.status).to eq(200)