From 137ade29d061e158543da814ecd0d06d7e992c1f Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Wed, 2 Nov 2016 14:38:34 -0400 Subject: PLT-4535/PLT-4503 Fix inactive users in searches and add option functionality to DB user search (#4413) * Add options to user database search * Fix inactive users showing up incorrectly in some user searches * Read JSON for searchUsers API into anonymous struct * Move anonymous struct to be a normal struct in model directory and upadte client to use it * Added clarification comment about slightly odd query condition in search --- api/user.go | 37 +++++++++++++++++++----------------- api/user_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 68 insertions(+), 27 deletions(-) (limited to 'api') diff --git a/api/user.go b/api/user.go index 37f8d6818..2c00dd4c8 100644 --- a/api/user.go +++ b/api/user.go @@ -2592,33 +2592,36 @@ func sanitizeProfile(c *Context, user *model.User) *model.User { } func searchUsers(c *Context, w http.ResponseWriter, r *http.Request) { - props := model.MapFromJson(r.Body) + props := model.UserSearchFromJson(r.Body) + if props == nil { + c.SetInvalidParam("searchUsers", "") + return + } - term := props["term"] - if len(term) == 0 { + if len(props.Term) == 0 { c.SetInvalidParam("searchUsers", "term") return } - teamId := props["team_id"] - inChannelId := props["in_channel"] - notInChannelId := props["not_in_channel"] - - if inChannelId != "" && !HasPermissionToChannelContext(c, inChannelId, model.PERMISSION_READ_CHANNEL) { + if props.InChannelId != "" && !HasPermissionToChannelContext(c, props.InChannelId, model.PERMISSION_READ_CHANNEL) { return } - if notInChannelId != "" && !HasPermissionToChannelContext(c, notInChannelId, model.PERMISSION_READ_CHANNEL) { + if props.NotInChannelId != "" && !HasPermissionToChannelContext(c, props.NotInChannelId, model.PERMISSION_READ_CHANNEL) { return } + searchOptions := map[string]bool{} + searchOptions[store.USER_SEARCH_OPTION_USERNAME_ONLY] = true + searchOptions[store.USER_SEARCH_OPTION_ALLOW_INACTIVE] = props.AllowInactive + var uchan store.StoreChannel - if inChannelId != "" { - uchan = Srv.Store.User().SearchInChannel(inChannelId, term, store.USER_SEARCH_TYPE_USERNAME) - } else if notInChannelId != "" { - uchan = Srv.Store.User().SearchNotInChannel(teamId, notInChannelId, term, store.USER_SEARCH_TYPE_USERNAME) + if props.InChannelId != "" { + uchan = Srv.Store.User().SearchInChannel(props.InChannelId, props.Term, searchOptions) + } else if props.NotInChannelId != "" { + uchan = Srv.Store.User().SearchNotInChannel(props.TeamId, props.NotInChannelId, props.Term, searchOptions) } else { - uchan = Srv.Store.User().Search(teamId, term, store.USER_SEARCH_TYPE_USERNAME) + uchan = Srv.Store.User().Search(props.TeamId, props.Term, searchOptions) } if result := <-uchan; result.Err != nil { @@ -2674,8 +2677,8 @@ func autocompleteUsersInChannel(c *Context, w http.ResponseWriter, r *http.Reque return } - uchan := Srv.Store.User().SearchInChannel(channelId, term, store.USER_SEARCH_TYPE_ALL) - nuchan := Srv.Store.User().SearchNotInChannel(teamId, channelId, term, store.USER_SEARCH_TYPE_ALL) + uchan := Srv.Store.User().SearchInChannel(channelId, term, map[string]bool{}) + nuchan := Srv.Store.User().SearchNotInChannel(teamId, channelId, term, map[string]bool{}) autocomplete := &model.UserAutocompleteInChannel{} @@ -2720,7 +2723,7 @@ func autocompleteUsersInTeam(c *Context, w http.ResponseWriter, r *http.Request) } } - uchan := Srv.Store.User().Search(teamId, term, store.USER_SEARCH_TYPE_ALL) + uchan := Srv.Store.User().Search(teamId, term, map[string]bool{}) autocomplete := &model.UserAutocompleteInTeam{} diff --git a/api/user_test.go b/api/user_test.go index 2c6238c54..75e246ab3 100644 --- a/api/user_test.go +++ b/api/user_test.go @@ -2034,10 +2034,14 @@ func TestGetProfilesNotInChannel(t *testing.T) { } func TestSearchUsers(t *testing.T) { - th := Setup().InitBasic() + th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient - if result, err := Client.SearchUsers(th.BasicUser.Username, "", map[string]string{}); err != nil { + inactiveUser := th.CreateUser(Client) + LinkUserToTeam(inactiveUser, th.BasicTeam) + th.SystemAdminClient.Must(th.SystemAdminClient.UpdateActive(inactiveUser.Id, false)) + + if result, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser.Username}); err != nil { t.Fatal(err) } else { users := result.Data.([]*model.User) @@ -2054,7 +2058,41 @@ func TestSearchUsers(t *testing.T) { } } - if result, err := Client.SearchUsers(th.BasicUser.Username, "", map[string]string{"in_channel": th.BasicChannel.Id}); err != nil { + if result, err := Client.SearchUsers(model.UserSearch{Term: inactiveUser.Username, TeamId: th.BasicTeam.Id}); err != nil { + t.Fatal(err) + } else { + users := result.Data.([]*model.User) + + found := false + for _, user := range users { + if user.Id == inactiveUser.Id { + found = true + } + } + + if found { + t.Fatal("should not have found inactive user") + } + } + + if result, err := Client.SearchUsers(model.UserSearch{Term: inactiveUser.Username, TeamId: th.BasicTeam.Id, AllowInactive: true}); err != nil { + t.Fatal(err) + } else { + users := result.Data.([]*model.User) + + found := false + for _, user := range users { + if user.Id == inactiveUser.Id { + found = true + } + } + + if !found { + t.Fatal("should have found inactive user") + } + } + + if result, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser.Username, InChannelId: th.BasicChannel.Id}); err != nil { t.Fatal(err) } else { users := result.Data.([]*model.User) @@ -2075,7 +2113,7 @@ func TestSearchUsers(t *testing.T) { } } - if result, err := Client.SearchUsers(th.BasicUser2.Username, "", map[string]string{"not_in_channel": th.BasicChannel.Id}); err != nil { + if result, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser2.Username, NotInChannelId: th.BasicChannel.Id}); err != nil { t.Fatal(err) } else { users := result.Data.([]*model.User) @@ -2102,7 +2140,7 @@ func TestSearchUsers(t *testing.T) { } } - if result, err := Client.SearchUsers(th.BasicUser2.Username, th.BasicTeam.Id, map[string]string{"not_in_channel": th.BasicChannel.Id}); err != nil { + if result, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser2.Username, TeamId: th.BasicTeam.Id, NotInChannelId: th.BasicChannel.Id}); err != nil { t.Fatal(err) } else { users := result.Data.([]*model.User) @@ -2129,7 +2167,7 @@ func TestSearchUsers(t *testing.T) { } } - if result, err := Client.SearchUsers(th.BasicUser.Username, "junk", map[string]string{"not_in_channel": th.BasicChannel.Id}); err != nil { + if result, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser.Username, TeamId: "junk", NotInChannelId: th.BasicChannel.Id}); err != nil { t.Fatal(err) } else { users := result.Data.([]*model.User) @@ -2141,7 +2179,7 @@ func TestSearchUsers(t *testing.T) { th.LoginBasic2() - if result, err := Client.SearchUsers(th.BasicUser.Username, "", map[string]string{}); err != nil { + if result, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser.Username}); err != nil { t.Fatal(err) } else { users := result.Data.([]*model.User) @@ -2158,15 +2196,15 @@ func TestSearchUsers(t *testing.T) { } } - if _, err := Client.SearchUsers("", "", map[string]string{}); err == nil { + if _, err := Client.SearchUsers(model.UserSearch{}); err == nil { t.Fatal("should have errored - blank term") } - if _, err := Client.SearchUsers(th.BasicUser.Username, "", map[string]string{"in_channel": th.BasicChannel.Id}); err == nil { + if _, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser.Username, InChannelId: th.BasicChannel.Id}); err == nil { t.Fatal("should not have access") } - if _, err := Client.SearchUsers(th.BasicUser.Username, "", map[string]string{"not_in_channel": th.BasicChannel.Id}); err == nil { + if _, err := Client.SearchUsers(model.UserSearch{Term: th.BasicUser.Username, NotInChannelId: th.BasicChannel.Id}); err == nil { t.Fatal("should not have access") } } -- cgit v1.2.3-1-g7c22