From bbee234af0b3efb430d3f5d2cf95ea4cba8d49ba Mon Sep 17 00:00:00 2001 From: Pradeep Murugesan Date: Fri, 11 Jan 2019 14:50:32 +0100 Subject: [PATCH] [GH-7494] Added the role to the user search filter (#9976) * 7494 added the role to the user search filter * 7494 changed the getUser function to accept the options * added the role filter for the getAllProfiles method * 7494 added the Inactive filter for AllProfiles * 7494 refactored the where clause generation * 7494 added the roles and inactive filters for inTeam Query * 7494 fixed the review comments --- api4/user.go | 24 +++- app/channel.go | 3 +- app/command_loadtest.go | 3 +- app/plugin_api.go | 3 +- app/user.go | 48 ++------ model/user_get.go | 27 +++++ model/user_search.go | 3 + store/sqlstore/user_store.go | 80 ++++++++++++- store/store.go | 4 +- store/storetest/mocks/UserStore.go | 20 ++-- store/storetest/user_store.go | 176 ++++++++++++++++++++++++++++- 11 files changed, 321 insertions(+), 70 deletions(-) create mode 100644 model/user_get.go diff --git a/api4/user.go b/api4/user.go index 69edb431ab..a146c00c00 100644 --- a/api4/user.go +++ b/api4/user.go @@ -365,6 +365,8 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { inChannelId := r.URL.Query().Get("in_channel") notInChannelId := r.URL.Query().Get("not_in_channel") withoutTeam := r.URL.Query().Get("without_team") + inactive := r.URL.Query().Get("inactive") + role := r.URL.Query().Get("role") sort := r.URL.Query().Get("sort") if len(notInChannelId) > 0 && len(inTeamId) == 0 { @@ -388,6 +390,22 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { return } + withoutTeamBool, _ := strconv.ParseBool(withoutTeam) + inactiveBool, _ := strconv.ParseBool(inactive) + + userGetOptions := &model.UserGetOptions{ + InTeamId: inTeamId, + InChannelId: inChannelId, + NotInTeamId: notInTeamId, + NotInChannelId: notInChannelId, + WithoutTeam: withoutTeamBool, + Inactive: inactiveBool, + Role: role, + Sort: sort, + Page: c.Params.Page, + PerPage: c.Params.PerPage, + } + var profiles []*model.User var err *model.AppError etag := "" @@ -434,8 +452,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { if c.HandleEtag(etag, "Get Users in Team", w, r) { return } - - profiles, err = c.App.GetUsersInTeamPage(inTeamId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + profiles, err = c.App.GetUsersInTeamPage(userGetOptions, c.IsSystemAdmin()) } } else if len(inChannelId) > 0 { if !c.App.SessionHasPermissionToChannel(c.App.Session, inChannelId, model.PERMISSION_READ_CHANNEL) { @@ -454,7 +471,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { if c.HandleEtag(etag, "Get Users", w, r) { return } - profiles, err = c.App.GetUsersPage(c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + profiles, err = c.App.GetUsersPage(userGetOptions, c.IsSystemAdmin()) } if err != nil { @@ -553,6 +570,7 @@ func searchUsers(c *Context, w http.ResponseWriter, r *http.Request) { IsAdmin: c.IsSystemAdmin(), AllowInactive: props.AllowInactive, Limit: props.Limit, + Role: props.Role, } if c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_SYSTEM) { diff --git a/app/channel.go b/app/channel.go index 0a31692e34..b49e947871 100644 --- a/app/channel.go +++ b/app/channel.go @@ -891,7 +891,8 @@ func (a *App) AddChannelMember(userId string, channel *model.Channel, userReques func (a *App) AddDirectChannels(teamId string, user *model.User) *model.AppError { var profiles []*model.User - result := <-a.Srv.Store.User().GetProfiles(teamId, 0, 100) + options := &model.UserGetOptions{InTeamId: teamId, Page: 0, PerPage: 100} + result := <-a.Srv.Store.User().GetProfiles(options) if result.Err != nil { return model.NewAppError("AddDirectChannels", "api.user.add_direct_channels_and_forget.failed.error", map[string]interface{}{"UserId": user.Id, "TeamId": teamId, "Error": result.Err.Error()}, "", http.StatusInternalServerError) } diff --git a/app/command_loadtest.go b/app/command_loadtest.go index 8b3cd2e3b4..c645a10620 100644 --- a/app/command_loadtest.go +++ b/app/command_loadtest.go @@ -288,7 +288,8 @@ func (me *LoadTestProvider) PostsCommand(a *App, args *model.CommandArgs, messag } var usernames []string - if result := <-a.Srv.Store.User().GetProfiles(args.TeamId, 0, 1000); result.Err == nil { + options := &model.UserGetOptions{InTeamId: args.TeamId, Page: 0, PerPage: 1000} + if result := <-a.Srv.Store.User().GetProfiles(options); result.Err == nil { profileUsers := result.Data.([]*model.User) usernames = make([]string, len(profileUsers)) i := 0 diff --git a/app/plugin_api.go b/app/plugin_api.go index 2285b36bbd..62b22f6f3b 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -188,7 +188,8 @@ func (api *PluginAPI) GetUsersByUsernames(usernames []string) ([]*model.User, *m } func (api *PluginAPI) GetUsersInTeam(teamId string, page int, perPage int) ([]*model.User, *model.AppError) { - return api.app.GetUsersInTeam(teamId, page*perPage, perPage) + options := &model.UserGetOptions{InTeamId: teamId, Page: page, PerPage: perPage} + return api.app.GetUsersInTeam(options) } func (api *PluginAPI) UpdateUser(user *model.User) (*model.User, *model.AppError) { diff --git a/app/user.go b/app/user.go index 9dee5df14c..9e048dccc2 100644 --- a/app/user.go +++ b/app/user.go @@ -373,32 +373,16 @@ func (a *App) GetUserByAuth(authData *string, authService string) (*model.User, return result.Data.(*model.User), nil } -func (a *App) GetUsers(offset int, limit int) ([]*model.User, *model.AppError) { - result := <-a.Srv.Store.User().GetAllProfiles(offset, limit) +func (a *App) GetUsers(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + result := <-a.Srv.Store.User().GetAllProfiles(options) if result.Err != nil { return nil, result.Err } return result.Data.([]*model.User), nil } -func (a *App) GetUsersMap(offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { - users, err := a.GetUsers(offset, limit) - if err != nil { - return nil, err - } - - userMap := make(map[string]*model.User, len(users)) - - for _, user := range users { - a.SanitizeProfile(user, asAdmin) - userMap[user.Id] = user - } - - return userMap, nil -} - -func (a *App) GetUsersPage(page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { - users, err := a.GetUsers(page*perPage, perPage) +func (a *App) GetUsersPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := a.GetUsers(options) if err != nil { return nil, err } @@ -410,8 +394,8 @@ func (a *App) GetUsersEtag() string { return fmt.Sprintf("%v.%v.%v", (<-a.Srv.Store.User().GetEtagForAllProfiles()).Data.(string), a.Config().PrivacySettings.ShowFullName, a.Config().PrivacySettings.ShowEmailAddress) } -func (a *App) GetUsersInTeam(teamId string, offset int, limit int) ([]*model.User, *model.AppError) { - result := <-a.Srv.Store.User().GetProfiles(teamId, offset, limit) +func (a *App) GetUsersInTeam(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + result := <-a.Srv.Store.User().GetProfiles(options) if result.Err != nil { return nil, result.Err } @@ -426,24 +410,8 @@ func (a *App) GetUsersNotInTeam(teamId string, offset int, limit int) ([]*model. return result.Data.([]*model.User), nil } -func (a *App) GetUsersInTeamMap(teamId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { - users, err := a.GetUsersInTeam(teamId, offset, limit) - if err != nil { - return nil, err - } - - userMap := make(map[string]*model.User, len(users)) - - for _, user := range users { - a.SanitizeProfile(user, asAdmin) - userMap[user.Id] = user - } - - return userMap, nil -} - -func (a *App) GetUsersInTeamPage(teamId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { - users, err := a.GetUsersInTeam(teamId, page*perPage, perPage) +func (a *App) GetUsersInTeamPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := a.GetUsersInTeam(options) if err != nil { return nil, err } diff --git a/model/user_get.go b/model/user_get.go new file mode 100644 index 0000000000..7ec604f721 --- /dev/null +++ b/model/user_get.go @@ -0,0 +1,27 @@ +// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package model + +type UserGetOptions struct { + // Filters the users in the team + InTeamId string + // Filters the users not in the team + NotInTeamId string + // Filters the users in the channel + InChannelId string + // Filters the users not in the channel + NotInChannelId string + // Filters the users without a team + WithoutTeam bool + // Filters the inactive users + Inactive bool + // Filters for the given role + Role string + // Sorting option + Sort string + // Page + Page int + // Page size + PerPage int +} diff --git a/model/user_search.go b/model/user_search.go index 68749fbe5f..f8da5aeeef 100644 --- a/model/user_search.go +++ b/model/user_search.go @@ -21,6 +21,7 @@ type UserSearch struct { AllowInactive bool `json:"allow_inactive"` WithoutTeam bool `json:"without_team"` Limit int `json:"limit"` + Role string `json:"role"` } // ToJson convert a User to a json string @@ -54,4 +55,6 @@ type UserSearchOptions struct { AllowInactive bool // Limit limits the total number of results returned. Limit int + // Filters for the given role + Role string } diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 1b46ae8077..964266faf4 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -364,11 +364,23 @@ func (s SqlUserStore) GetEtagForAllProfiles() store.StoreChannel { }) } -func (us SqlUserStore) GetAllProfiles(offset int, limit int) store.StoreChannel { +func (us SqlUserStore) GetAllProfiles(options *model.UserGetOptions) store.StoreChannel { + isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES return store.Do(func(result *store.StoreResult) { var users []*model.User + offset := options.Page * options.PerPage + limit := options.PerPage - if _, err := us.GetReplica().Select(&users, "SELECT * FROM Users ORDER BY Username ASC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"Offset": offset, "Limit": limit}); err != nil { + searchQuery := ` + SELECT * FROM Users + WHERE_CONDITION + ORDER BY Username ASC LIMIT :Limit OFFSET :Offset + ` + + parameters := map[string]interface{}{"Offset": offset, "Limit": limit} + searchQuery = substituteWhereClause(searchQuery, options, parameters, isPostgreSQL) + + if _, err := us.GetReplica().Select(&users, searchQuery, parameters); err != nil { result.Err = model.NewAppError("SqlUserStore.GetAllProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) } else { @@ -381,6 +393,37 @@ func (us SqlUserStore) GetAllProfiles(offset int, limit int) store.StoreChannel }) } +func substituteWhereClause(searchQuery string, options *model.UserGetOptions, parameters map[string]interface{}, isPostgreSQL bool) string { + whereClause := "" + whereClauses := []string{} + if options.Role != "" { + whereClauses = append(whereClauses, getRoleFilter(isPostgreSQL)) + parameters["Role"] = fmt.Sprintf("%%%s%%", options.Role) + } + if options.Inactive { + whereClauses = append(whereClauses, " Users.DeleteAt != 0 ") + } + + if len(whereClauses) > 0 { + whereClause = strings.Join(whereClauses, " AND ") + searchQuery = strings.Replace(searchQuery, "WHERE_CONDITION", fmt.Sprintf(" WHERE %s ", whereClause), 1) + searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf(" AND %s ", whereClause), 1) + } else { + searchQuery = strings.Replace(searchQuery, "WHERE_CONDITION", "", 1) + searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1) + } + + return searchQuery +} + +func getRoleFilter(isPostgreSQL bool) string { + if isPostgreSQL { + return fmt.Sprintf("Users.Roles like lower(%s)", ":Role") + } else { + return fmt.Sprintf("Users.Roles LIKE %s escape '*' ", ":Role") + } +} + func (s SqlUserStore) GetEtagForProfiles(teamId string) store.StoreChannel { return store.Do(func(result *store.StoreResult) { updateAt, err := s.GetReplica().SelectInt("SELECT UpdateAt FROM Users, TeamMembers WHERE TeamMembers.TeamId = :TeamId AND Users.Id = TeamMembers.UserId ORDER BY UpdateAt DESC LIMIT 1", map[string]interface{}{"TeamId": teamId}) @@ -392,11 +435,26 @@ func (s SqlUserStore) GetEtagForProfiles(teamId string) store.StoreChannel { }) } -func (us SqlUserStore) GetProfiles(teamId string, offset int, limit int) store.StoreChannel { +func (us SqlUserStore) GetProfiles(options *model.UserGetOptions) store.StoreChannel { + isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES + teamId := options.InTeamId + offset := options.Page * options.PerPage + limit := options.PerPage + + searchQuery := ` + SELECT Users.* FROM Users, TeamMembers + WHERE TeamMembers.TeamId = :TeamId AND Users.Id = TeamMembers.UserId AND TeamMembers.DeleteAt = 0 + SEARCH_CLAUSE + ORDER BY Users.Username ASC LIMIT :Limit OFFSET :Offset + ` + + parameters := map[string]interface{}{"TeamId": teamId, "Offset": offset, "Limit": limit} + searchQuery = substituteWhereClause(searchQuery, options, parameters, isPostgreSQL) + return store.Do(func(result *store.StoreResult) { var users []*model.User - if _, err := us.GetReplica().Select(&users, "SELECT Users.* FROM Users, TeamMembers WHERE TeamMembers.TeamId = :TeamId AND Users.Id = TeamMembers.UserId AND TeamMembers.DeleteAt = 0 ORDER BY Users.Username ASC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"TeamId": teamId, "Offset": offset, "Limit": limit}); err != nil { + if _, err := us.GetReplica().Select(&users, searchQuery, parameters); err != nil { result.Err = model.NewAppError("SqlUserStore.GetProfiles", "store.sql_user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) } else { @@ -1154,7 +1212,7 @@ var spaceFulltextSearchChar = []string{ "@", } -func generateSearchQuery(searchQuery string, terms []string, fields []string, parameters map[string]interface{}, isPostgreSQL bool) string { +func generateSearchQuery(searchQuery string, terms []string, fields []string, parameters map[string]interface{}, isPostgreSQL bool, role string) string { searchTerms := []string{} for i, term := range terms { searchFields := []string{} @@ -1169,6 +1227,11 @@ func generateSearchQuery(searchQuery string, terms []string, fields []string, pa parameters[fmt.Sprintf("Term%d", i)] = fmt.Sprintf("%s%%", strings.TrimLeft(term, "@")) } + if role != "" { + searchTerms = append(searchTerms, getRoleFilter(isPostgreSQL)) + parameters["Role"] = fmt.Sprintf("%%%s%%", role) + } + searchClause := strings.Join(searchTerms, " AND ") return strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf(" AND %s ", searchClause), 1) } @@ -1201,6 +1264,11 @@ func (us SqlUserStore) performSearch(searchQuery string, term string, options *m } } + role := "" + if options.Role != "" { + role = options.Role + } + if ok := options.AllowInactive; ok { searchQuery = strings.Replace(searchQuery, "INACTIVE_CLAUSE", "", 1) } else { @@ -1211,7 +1279,7 @@ func (us SqlUserStore) performSearch(searchQuery string, term string, options *m searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1) } else { isPostgreSQL := us.DriverName() == model.DATABASE_DRIVER_POSTGRES - searchQuery = generateSearchQuery(searchQuery, strings.Fields(term), searchType, parameters, isPostgreSQL) + searchQuery = generateSearchQuery(searchQuery, strings.Fields(term), searchType, parameters, isPostgreSQL, role) } var users []*model.User diff --git a/store/store.go b/store/store.go index 4124e48885..398529882d 100644 --- a/store/store.go +++ b/store/store.go @@ -249,8 +249,8 @@ type UserStore interface { GetProfilesNotInChannel(teamId string, channelId string, offset int, limit int) StoreChannel GetProfilesWithoutTeam(offset int, limit int) StoreChannel GetProfilesByUsernames(usernames []string, teamId string) StoreChannel - GetAllProfiles(offset int, limit int) StoreChannel - GetProfiles(teamId string, offset int, limit int) StoreChannel + GetAllProfiles(options *model.UserGetOptions) StoreChannel + GetProfiles(options *model.UserGetOptions) StoreChannel GetProfileByIds(userId []string, allowFromCache bool) StoreChannel InvalidatProfileCacheForUser(userId string) GetByEmail(email string) StoreChannel diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index de836e95d9..d3081c8910 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -146,13 +146,13 @@ func (_m *UserStore) GetAllAfter(limit int, afterId string) store.StoreChannel { return r0 } -// GetAllProfiles provides a mock function with given fields: offset, limit -func (_m *UserStore) GetAllProfiles(offset int, limit int) store.StoreChannel { - ret := _m.Called(offset, limit) +// GetAllProfiles provides a mock function with given fields: options +func (_m *UserStore) GetAllProfiles(options *model.UserGetOptions) store.StoreChannel { + ret := _m.Called(options) var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(int, int) store.StoreChannel); ok { - r0 = rf(offset, limit) + if rf, ok := ret.Get(0).(func(*model.UserGetOptions) store.StoreChannel); ok { + r0 = rf(options) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(store.StoreChannel) @@ -354,13 +354,13 @@ func (_m *UserStore) GetProfileByIds(userId []string, allowFromCache bool) store return r0 } -// GetProfiles provides a mock function with given fields: teamId, offset, limit -func (_m *UserStore) GetProfiles(teamId string, offset int, limit int) store.StoreChannel { - ret := _m.Called(teamId, offset, limit) +// GetProfiles provides a mock function with given fields: options +func (_m *UserStore) GetProfiles(options *model.UserGetOptions) store.StoreChannel { + ret := _m.Called(options) var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, int, int) store.StoreChannel); ok { - r0 = rf(teamId, offset, limit) + if rf, ok := ret.Get(0).(func(*model.UserGetOptions) store.StoreChannel); ok { + r0 = rf(options) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(store.StoreChannel) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index a7abf37e03..ce2421cf51 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -307,7 +307,9 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { store.Must(ss.User().Save(u2)) defer func() { store.Must(ss.User().PermanentDelete(u2.Id)) }() - if r1 := <-ss.User().GetAllProfiles(0, 100); r1.Err != nil { + options := &model.UserGetOptions{Page: 0, PerPage: 100} + + if r1 := <-ss.User().GetAllProfiles(options); r1.Err != nil { t.Fatal(r1.Err) } else { users := r1.Data.([]*model.User) @@ -316,7 +318,8 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { } } - if r2 := <-ss.User().GetAllProfiles(0, 1); r2.Err != nil { + options = &model.UserGetOptions{Page: 0, PerPage: 1} + if r2 := <-ss.User().GetAllProfiles(options); r2.Err != nil { t.Fatal(r2.Err) } else { users := r2.Data.([]*model.User) @@ -343,6 +346,7 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { u3 := &model.User{} u3.Email = MakeEmail() + u3.Roles = "system_user some-other-role" store.Must(ss.User().Save(u3)) defer func() { store.Must(ss.User().PermanentDelete(u3.Id)) }() @@ -353,6 +357,64 @@ func testUserStoreGetAllProfiles(t *testing.T, ss store.Store) { t.Fatal("etags should not match") } } + + u4 := &model.User{} + u4.Email = MakeEmail() + u4.Roles = "system_admin some-other-role" + store.Must(ss.User().Save(u4)) + defer func() { store.Must(ss.User().PermanentDelete(u4.Id)) }() + + u5 := &model.User{} + u5.Email = MakeEmail() + u5.Roles = "system_admin" + store.Must(ss.User().Save(u5)) + defer func() { store.Must(ss.User().PermanentDelete(u5.Id)) }() + + options = &model.UserGetOptions{Page: 0, PerPage: 10, Role: "system_admin"} + if r2 := <-ss.User().GetAllProfiles(options); r2.Err != nil { + t.Fatal(r2.Err) + } else { + users := r2.Data.([]*model.User) + if len(users) != 2 { + t.Fatal("invalid returned users, role filter did not work") + } + assert.ElementsMatch(t, []string{u4.Id, u5.Id}, []string{users[0].Id, users[1].Id}) + } + + u6 := &model.User{} + u6.Email = MakeEmail() + u6.DeleteAt = model.GetMillis() + u6.Roles = "system_admin" + store.Must(ss.User().Save(u6)) + defer func() { store.Must(ss.User().PermanentDelete(u6.Id)) }() + + u7 := &model.User{} + u7.Email = MakeEmail() + u7.DeleteAt = model.GetMillis() + store.Must(ss.User().Save(u7)) + defer func() { store.Must(ss.User().PermanentDelete(u7.Id)) }() + + options = &model.UserGetOptions{Page: 0, PerPage: 10, Role: "system_admin", Inactive: true} + if r2 := <-ss.User().GetAllProfiles(options); r2.Err != nil { + t.Fatal(r2.Err) + } else { + users := r2.Data.([]*model.User) + if len(users) != 1 { + t.Fatal("invalid returned users, Role and Inactive filter did not work") + } + assert.Equal(t, u6.Id, users[0].Id) + } + + options = &model.UserGetOptions{Page: 0, PerPage: 10, Inactive: true} + if r2 := <-ss.User().GetAllProfiles(options); r2.Err != nil { + t.Fatal(r2.Err) + } else { + users := r2.Data.([]*model.User) + if len(users) != 2 { + t.Fatal("invalid returned users, Inactive filter did not work") + } + assert.ElementsMatch(t, []string{u6.Id, u7.Id}, []string{users[0].Id, users[1].Id}) + } } func testUserStoreGetProfiles(t *testing.T, ss store.Store) { @@ -370,7 +432,8 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { defer func() { store.Must(ss.User().PermanentDelete(u2.Id)) }() store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u2.Id}, -1)) - if r1 := <-ss.User().GetProfiles(teamId, 0, 100); r1.Err != nil { + options := &model.UserGetOptions{InTeamId: teamId, Page: 0, PerPage: 100} + if r1 := <-ss.User().GetProfiles(options); r1.Err != nil { t.Fatal(r1.Err) } else { users := r1.Data.([]*model.User) @@ -390,7 +453,8 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { } } - if r2 := <-ss.User().GetProfiles("123", 0, 100); r2.Err != nil { + options = &model.UserGetOptions{InTeamId: "123", Page: 0, PerPage: 100} + if r2 := <-ss.User().GetProfiles(options); r2.Err != nil { t.Fatal(r2.Err) } else { if len(r2.Data.([]*model.User)) != 0 { @@ -418,6 +482,53 @@ func testUserStoreGetProfiles(t *testing.T, ss store.Store) { t.Fatal("etags should not match") } } + + u4 := &model.User{} + u4.Email = MakeEmail() + u4.Roles = "system_admin" + store.Must(ss.User().Save(u4)) + defer func() { store.Must(ss.User().PermanentDelete(u4.Id)) }() + store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u4.Id}, -1)) + + u5 := &model.User{} + u5.Email = MakeEmail() + u5.DeleteAt = model.GetMillis() + store.Must(ss.User().Save(u5)) + defer func() { store.Must(ss.User().PermanentDelete(u5.Id)) }() + store.Must(ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u5.Id}, -1)) + + options = &model.UserGetOptions{InTeamId: teamId, Page: 0, PerPage: 100} + if r1 := <-ss.User().GetProfiles(options); r1.Err != nil { + t.Fatal(r1.Err) + } else { + users := r1.Data.([]*model.User) + if len(users) != 5 { + t.Fatal("invalid returned users") + } + } + + options = &model.UserGetOptions{InTeamId: teamId, Role: "system_admin", Inactive: false, Page: 0, PerPage: 100} + if r1 := <-ss.User().GetProfiles(options); r1.Err != nil { + t.Fatal(r1.Err) + } else { + users := r1.Data.([]*model.User) + if len(users) != 1 { + t.Fatal("invalid returned users") + } + assert.Equal(t, u4.Id, users[0].Id) + } + + options = &model.UserGetOptions{InTeamId: teamId, Inactive: true, Page: 0, PerPage: 100} + if r1 := <-ss.User().GetProfiles(options); r1.Err != nil { + t.Fatal(r1.Err) + } else { + users := r1.Data.([]*model.User) + if len(users) != 1 { + t.Fatal("invalid returned users") + } + assert.Equal(t, u5.Id, users[0].Id) + } + } func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) { @@ -937,7 +1048,8 @@ func testUserStoreGetProfilesByIds(t *testing.T, ss store.Store) { } } - if r2 := <-ss.User().GetProfiles("123", 0, 100); r2.Err != nil { + options := &model.UserGetOptions{InTeamId: "123", Page: 0, PerPage: 100} + if r2 := <-ss.User().GetProfiles(options); r2.Err != nil { t.Fatal(r2.Err) } else { if len(r2.Data.([]*model.User)) != 0 { @@ -1410,6 +1522,22 @@ func assertUsers(t *testing.T, expected, actual []*model.User) { } } +func assertUsersMatchInAnyOrder(t *testing.T, expected, actual []*model.User) { + expectedUsernames := make([]string, 0, len(expected)) + for _, user := range expected { + expectedUsernames = append(expectedUsernames, user.Username) + } + + actualUsernames := make([]string, 0, len(actual)) + for _, user := range actual { + actualUsernames = append(actualUsernames, user.Username) + } + + if assert.ElementsMatch(t, expectedUsernames, actualUsernames) { + assert.ElementsMatch(t, expected, actual) + } +} + func testUserStoreSearch(t *testing.T, ss store.Store) { u1 := &model.User{ Username: "jimbo1" + model.NewId(), @@ -1417,6 +1545,7 @@ func testUserStoreSearch(t *testing.T, ss store.Store) { LastName: "Bill", Nickname: "Rob", Email: "harold" + model.NewId() + "@simulator.amazonses.com", + Roles: "system_user system_admin", } store.Must(ss.User().Save(u1)) defer func() { store.Must(ss.User().PermanentDelete(u1.Id)) }() @@ -1424,6 +1553,7 @@ func testUserStoreSearch(t *testing.T, ss store.Store) { u2 := &model.User{ Username: "jim-bobby" + model.NewId(), Email: MakeEmail(), + Roles: "system_user", } store.Must(ss.User().Save(u2)) defer func() { store.Must(ss.User().PermanentDelete(u2.Id)) }() @@ -1432,6 +1562,7 @@ func testUserStoreSearch(t *testing.T, ss store.Store) { Username: "jimbo3" + model.NewId(), Email: MakeEmail(), DeleteAt: 1, + Roles: "system_admin", } store.Must(ss.User().Save(u3)) defer func() { store.Must(ss.User().PermanentDelete(u3.Id)) }() @@ -1668,13 +1799,46 @@ func testUserStoreSearch(t *testing.T, ss store.Store) { }, []*model.User{u1}, }, + { + "search jim-bobby with system_user roles", + tid, + "jim-bobby", + &model.UserSearchOptions{ + AllowFullNames: true, + Limit: model.USER_SEARCH_DEFAULT_LIMIT, + Role: "system_user", + }, + []*model.User{u2}, + }, + { + "search jim with system_admin roles", + tid, + "jim", + &model.UserSearchOptions{ + AllowFullNames: true, + Limit: model.USER_SEARCH_DEFAULT_LIMIT, + Role: "system_admin", + }, + []*model.User{u1}, + }, + { + "search ji with system_user roles", + tid, + "ji", + &model.UserSearchOptions{ + AllowFullNames: true, + Limit: model.USER_SEARCH_DEFAULT_LIMIT, + Role: "system_user", + }, + []*model.User{u1, u2}, + }, } for _, testCase := range testCases { t.Run(testCase.Description, func(t *testing.T) { result := <-ss.User().Search(testCase.TeamId, testCase.Term, testCase.Options) require.Nil(t, result.Err) - assertUsers(t, testCase.Expected, result.Data.([]*model.User)) + assertUsersMatchInAnyOrder(t, testCase.Expected, result.Data.([]*model.User)) }) }