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

2 files changed, 68 insertions, 27 deletions

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")
 	}
 }