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

3 files changed, 131 insertions, 44 deletions

diff --git a/store/sql_user_store.go b/store/sql_user_store.go
index 4f5e11d00..17fdcbc85 100644
--- a/store/sql_user_store.go
+++ b/store/sql_user_store.go
@@ -15,12 +15,14 @@ import (
 )
 
 const (
-	MISSING_ACCOUNT_ERROR          = "store.sql_user.missing_account.const"
-	MISSING_AUTH_ACCOUNT_ERROR     = "store.sql_user.get_by_auth.missing_account.app_error"
-	PROFILES_IN_CHANNEL_CACHE_SIZE = 5000
-	PROFILES_IN_CHANNEL_CACHE_SEC  = 900 // 15 mins
-	USER_SEARCH_TYPE_ALL           = "Username, FirstName, LastName, Nickname"
-	USER_SEARCH_TYPE_USERNAME      = "Username"
+	MISSING_ACCOUNT_ERROR             = "store.sql_user.missing_account.const"
+	MISSING_AUTH_ACCOUNT_ERROR        = "store.sql_user.get_by_auth.missing_account.app_error"
+	PROFILES_IN_CHANNEL_CACHE_SIZE    = 5000
+	PROFILES_IN_CHANNEL_CACHE_SEC     = 900 // 15 mins
+	USER_SEARCH_OPTION_USERNAME_ONLY  = "username_only"
+	USER_SEARCH_OPTION_ALLOW_INACTIVE = "allow_inactive"
+	USER_SEARCH_TYPE_ALL              = "Username, FirstName, LastName, Nickname"
+	USER_SEARCH_TYPE_USERNAME         = "Username"
 )
 
 type SqlUserStore struct {
@@ -1085,20 +1087,24 @@ func (us SqlUserStore) GetUnreadCountForChannel(userId string, channelId string)
 	return storeChannel
 }
 
-func (us SqlUserStore) Search(teamId string, term string, searchType string) StoreChannel {
+func (us SqlUserStore) Search(teamId string, term string, options map[string]bool) StoreChannel {
 	storeChannel := make(StoreChannel, 1)
 
 	go func() {
 		searchQuery := ""
+
 		if teamId == "" {
+
+			// Id != '' is added because both SEARCH_CLAUSE and INACTIVE_CLAUSE start with an AND
 			searchQuery = `
 			SELECT
 				*
 			FROM
 				Users
 			WHERE
-				DeleteAt = 0
+				Id != ''
 				SEARCH_CLAUSE
+				INACTIVE_CLAUSE
 				ORDER BY Username ASC
 			LIMIT 50`
 		} else {
@@ -1110,14 +1116,14 @@ func (us SqlUserStore) Search(teamId string, term string, searchType string) Sto
 			WHERE
 				TeamMembers.TeamId = :TeamId
 				AND Users.Id = TeamMembers.UserId
-				AND Users.DeleteAt = 0
 				AND TeamMembers.DeleteAt = 0
 				SEARCH_CLAUSE
+				INACTIVE_CLAUSE
 				ORDER BY Users.Username ASC
 			LIMIT 100`
 		}
 
-		storeChannel <- us.performSearch(searchQuery, term, searchType, map[string]interface{}{"TeamId": teamId})
+		storeChannel <- us.performSearch(searchQuery, term, options, map[string]interface{}{"TeamId": teamId})
 		close(storeChannel)
 
 	}()
@@ -1125,7 +1131,7 @@ func (us SqlUserStore) Search(teamId string, term string, searchType string) Sto
 	return storeChannel
 }
 
-func (us SqlUserStore) SearchNotInChannel(teamId string, channelId string, term string, searchType string) StoreChannel {
+func (us SqlUserStore) SearchNotInChannel(teamId string, channelId string, term string, options map[string]bool) StoreChannel {
 	storeChannel := make(StoreChannel, 1)
 
 	go func() {
@@ -1133,34 +1139,38 @@ func (us SqlUserStore) SearchNotInChannel(teamId string, channelId string, term
 		if teamId == "" {
 			searchQuery = `
 			SELECT
-				u.*
-			FROM Users u
+				Users.*
+			FROM Users
 			LEFT JOIN ChannelMembers cm
-				ON cm.UserId = u.Id
+				ON cm.UserId = Users.Id
 				AND cm.ChannelId = :ChannelId
-			WHERE cm.UserId IS NULL
-			SEARCH_CLAUSE
-			ORDER BY u.Username ASC
+			WHERE
+				cm.UserId IS NULL
+				SEARCH_CLAUSE
+				INACTIVE_CLAUSE
+			ORDER BY Users.Username ASC
 			LIMIT 100`
 		} else {
 			searchQuery = `
 			SELECT
-				u.*
-			FROM Users u
+				Users.*
+			FROM Users
 			INNER JOIN TeamMembers tm
-				ON tm.UserId = u.Id
+				ON tm.UserId = Users.Id
 				AND tm.TeamId = :TeamId
 				AND tm.DeleteAt = 0
 			LEFT JOIN ChannelMembers cm
-				ON cm.UserId = u.Id
+				ON cm.UserId = Users.Id
 				AND cm.ChannelId = :ChannelId
-			WHERE cm.UserId IS NULL
-			SEARCH_CLAUSE
-			ORDER BY u.Username ASC
+			WHERE
+				cm.UserId IS NULL
+				SEARCH_CLAUSE
+				INACTIVE_CLAUSE
+			ORDER BY Users.Username ASC
 			LIMIT 100`
 		}
 
-		storeChannel <- us.performSearch(searchQuery, term, searchType, map[string]interface{}{"TeamId": teamId, "ChannelId": channelId})
+		storeChannel <- us.performSearch(searchQuery, term, options, map[string]interface{}{"TeamId": teamId, "ChannelId": channelId})
 		close(storeChannel)
 
 	}()
@@ -1168,7 +1178,7 @@ func (us SqlUserStore) SearchNotInChannel(teamId string, channelId string, term
 	return storeChannel
 }
 
-func (us SqlUserStore) SearchInChannel(channelId string, term string, searchType string) StoreChannel {
+func (us SqlUserStore) SearchInChannel(channelId string, term string, options map[string]bool) StoreChannel {
 	storeChannel := make(StoreChannel, 1)
 
 	go func() {
@@ -1180,12 +1190,12 @@ func (us SqlUserStore) SearchInChannel(channelId string, term string, searchType
         WHERE
             ChannelMembers.ChannelId = :ChannelId
             AND ChannelMembers.UserId = Users.Id
-            AND Users.DeleteAt = 0
             SEARCH_CLAUSE
+            INACTIVE_CLAUSE
             ORDER BY Users.Username ASC
         LIMIT 100`
 
-		storeChannel <- us.performSearch(searchQuery, term, searchType, map[string]interface{}{"ChannelId": channelId})
+		storeChannel <- us.performSearch(searchQuery, term, options, map[string]interface{}{"ChannelId": channelId})
 		close(storeChannel)
 
 	}()
@@ -1193,7 +1203,7 @@ func (us SqlUserStore) SearchInChannel(channelId string, term string, searchType
 	return storeChannel
 }
 
-func (us SqlUserStore) performSearch(searchQuery string, term string, searchType string, parameters map[string]interface{}) StoreResult {
+func (us SqlUserStore) performSearch(searchQuery string, term string, options map[string]bool, parameters map[string]interface{}) StoreResult {
 	result := StoreResult{}
 
 	// these chars have special meaning and can be treated as spaces
@@ -1201,6 +1211,17 @@ func (us SqlUserStore) performSearch(searchQuery string, term string, searchType
 		term = strings.Replace(term, c, " ", -1)
 	}
 
+	searchType := USER_SEARCH_TYPE_ALL
+	if ok := options[USER_SEARCH_OPTION_USERNAME_ONLY]; ok {
+		searchType = USER_SEARCH_TYPE_USERNAME
+	}
+
+	if ok := options[USER_SEARCH_OPTION_ALLOW_INACTIVE]; ok {
+		searchQuery = strings.Replace(searchQuery, "INACTIVE_CLAUSE", "", 1)
+	} else {
+		searchQuery = strings.Replace(searchQuery, "INACTIVE_CLAUSE", "AND Users.DeleteAt = 0", 1)
+	}
+
 	if term == "" {
 		searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1)
 	} else if utils.Cfg.SqlSettings.DriverName == model.DATABASE_DRIVER_POSTGRES {
diff --git a/store/sql_user_store_test.go b/store/sql_user_store_test.go
index 23c124cb7..bc7cc69c5 100644
--- a/store/sql_user_store_test.go
+++ b/store/sql_user_store_test.go
@@ -942,11 +942,75 @@ func TestUserStoreSearch(t *testing.T) {
 	u2.Email = model.NewId()
 	Must(store.User().Save(u2))
 
+	u3 := &model.User{}
+	u3.Username = "jimbo" + model.NewId()
+	u3.Email = model.NewId()
+	u3.DeleteAt = 1
+	Must(store.User().Save(u3))
+
 	tid := model.NewId()
 	Must(store.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u1.Id}))
 	Must(store.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u2.Id}))
+	Must(store.Team().SaveMember(&model.TeamMember{TeamId: tid, UserId: u3.Id}))
+
+	searchOptions := map[string]bool{}
+	searchOptions[USER_SEARCH_OPTION_USERNAME_ONLY] = true
+
+	if r1 := <-store.User().Search(tid, "jimb", searchOptions); r1.Err != nil {
+		t.Fatal(r1.Err)
+	} else {
+		profiles := r1.Data.([]*model.User)
+		found1 := false
+		found2 := false
+		for _, profile := range profiles {
+			if profile.Id == u1.Id {
+				found1 = true
+			}
+
+			if profile.Id == u3.Id {
+				found2 = true
+			}
+		}
+
+		if !found1 {
+			t.Fatal("should have found user")
+		}
+
+		if found2 {
+			t.Fatal("should not have found inactive user")
+		}
+	}
 
-	if r1 := <-store.User().Search(tid, "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	searchOptions[USER_SEARCH_OPTION_ALLOW_INACTIVE] = true
+
+	if r1 := <-store.User().Search(tid, "jimb", searchOptions); r1.Err != nil {
+		t.Fatal(r1.Err)
+	} else {
+		profiles := r1.Data.([]*model.User)
+		found1 := false
+		found2 := false
+		for _, profile := range profiles {
+			if profile.Id == u1.Id {
+				found1 = true
+			}
+
+			if profile.Id == u3.Id {
+				found2 = true
+			}
+		}
+
+		if !found1 {
+			t.Fatal("should have found user")
+		}
+
+		if !found2 {
+			t.Fatal("should have found inactive user")
+		}
+	}
+
+	searchOptions[USER_SEARCH_OPTION_ALLOW_INACTIVE] = false
+
+	if r1 := <-store.User().Search(tid, "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -963,7 +1027,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().Search("", "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().Search("", "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -980,7 +1044,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().Search("", "jim-bobb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().Search("", "jim-bobb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -998,7 +1062,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().Search(tid, "", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().Search(tid, "", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	}
 
@@ -1009,7 +1073,7 @@ func TestUserStoreSearch(t *testing.T) {
 	c1.Type = model.CHANNEL_OPEN
 	c1 = *Must(store.Channel().Save(&c1)).(*model.Channel)
 
-	if r1 := <-store.User().SearchNotInChannel(tid, c1.Id, "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().SearchNotInChannel(tid, c1.Id, "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1026,7 +1090,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().SearchNotInChannel("", c1.Id, "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().SearchNotInChannel("", c1.Id, "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1043,7 +1107,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().SearchNotInChannel("junk", c1.Id, "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().SearchNotInChannel("junk", c1.Id, "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1060,7 +1124,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().SearchInChannel(c1.Id, "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().SearchInChannel(c1.Id, "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1079,7 +1143,7 @@ func TestUserStoreSearch(t *testing.T) {
 
 	Must(store.Channel().SaveMember(&model.ChannelMember{ChannelId: c1.Id, UserId: u1.Id, NotifyProps: model.GetDefaultChannelNotifyProps()}))
 
-	if r1 := <-store.User().SearchInChannel(c1.Id, "jimb", USER_SEARCH_TYPE_USERNAME); r1.Err != nil {
+	if r1 := <-store.User().SearchInChannel(c1.Id, "jimb", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1096,7 +1160,9 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().Search(tid, "Tim", USER_SEARCH_TYPE_ALL); r1.Err != nil {
+	searchOptions = map[string]bool{}
+
+	if r1 := <-store.User().Search(tid, "Tim", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1113,7 +1179,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().Search(tid, "Bill", USER_SEARCH_TYPE_ALL); r1.Err != nil {
+	if r1 := <-store.User().Search(tid, "Bill", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
@@ -1130,7 +1196,7 @@ func TestUserStoreSearch(t *testing.T) {
 		}
 	}
 
-	if r1 := <-store.User().Search(tid, "Rob", USER_SEARCH_TYPE_ALL); r1.Err != nil {
+	if r1 := <-store.User().Search(tid, "Rob", searchOptions); r1.Err != nil {
 		t.Fatal(r1.Err)
 	} else {
 		profiles := r1.Data.([]*model.User)
diff --git a/store/store.go b/store/store.go
index 6cf216699..85a1ad398 100644
--- a/store/store.go
+++ b/store/store.go
@@ -164,9 +164,9 @@ type UserStore interface {
 	GetUnreadCount(userId string) StoreChannel
 	GetUnreadCountForChannel(userId string, channelId string) StoreChannel
 	GetRecentlyActiveUsersForTeam(teamId string) StoreChannel
-	Search(teamId string, term string, searchType string) StoreChannel
-	SearchInChannel(channelId string, term string, searchType string) StoreChannel
-	SearchNotInChannel(teamId string, channelId string, term string, searchType string) StoreChannel
+	Search(teamId string, term string, options map[string]bool) StoreChannel
+	SearchInChannel(channelId string, term string, options map[string]bool) StoreChannel
+	SearchNotInChannel(teamId string, channelId string, term string, options map[string]bool) StoreChannel
 }
 
 type SessionStore interface {