From e69a2a41ca70fbb92fd7874e0bd74b7d6d732832 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Wed, 16 Sep 2020 11:04:17 +0300 Subject: [PATCH] [MM-28210] sqlstore/user_store: filter deleted users for GetProfilesInChannel (#15390) * sqlstore/user_store: filter deleted users for GetProfilesInChannel * allow GetProfilesInChannel use userGetOptions * sqlstore/user_store: add more test cases * store/user_store: refine filter --- api4/user.go | 4 +- api4/user_local.go | 7 +- app/app_iface.go | 10 +- app/opentracing/opentracing_layer.go | 20 ++-- app/plugin_api.go | 12 +- app/user.go | 20 ++-- app/user_test.go | 25 +++- store/opentracinglayer/opentracinglayer.go | 8 +- store/retrylayer/retrylayer.go | 8 +- store/sqlstore/user_store.go | 24 +++- store/store.go | 4 +- store/storetest/mocks/UserStore.go | 28 ++--- store/storetest/user_store.go | 128 +++++++++++++++++++-- store/timerlayer/timerlayer.go | 8 +- 14 files changed, 229 insertions(+), 77 deletions(-) diff --git a/api4/user.go b/api4/user.go index a3853663f7..0d6e928a08 100644 --- a/api4/user.go +++ b/api4/user.go @@ -752,9 +752,9 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { return } if sort == "status" { - profiles, err = c.App.GetUsersInChannelPageByStatus(inChannelId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + profiles, err = c.App.GetUsersInChannelPageByStatus(userGetOptions, c.IsSystemAdmin()) } else { - profiles, err = c.App.GetUsersInChannelPage(inChannelId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + profiles, err = c.App.GetUsersInChannelPage(userGetOptions, c.IsSystemAdmin()) } } else if len(inGroupId) > 0 { if c.App.Srv().License() == nil || !*c.App.Srv().License().Features.LDAPGroups { diff --git a/api4/user_local.go b/api4/user_local.go index abdc5c0cb2..5b4b048828 100644 --- a/api4/user_local.go +++ b/api4/user_local.go @@ -47,6 +47,7 @@ func localGetUsers(c *Context, w http.ResponseWriter, r *http.Request) { notInChannelId := r.URL.Query().Get("not_in_channel") groupConstrained := r.URL.Query().Get("group_constrained") withoutTeam := r.URL.Query().Get("without_team") + active := r.URL.Query().Get("active") inactive := r.URL.Query().Get("inactive") role := r.URL.Query().Get("role") sort := r.URL.Query().Get("sort") @@ -74,6 +75,7 @@ func localGetUsers(c *Context, w http.ResponseWriter, r *http.Request) { withoutTeamBool, _ := strconv.ParseBool(withoutTeam) groupConstrainedBool, _ := strconv.ParseBool(groupConstrained) + activeBool, _ := strconv.ParseBool(active) inactiveBool, _ := strconv.ParseBool(inactive) userGetOptions := &model.UserGetOptions{ @@ -83,6 +85,7 @@ func localGetUsers(c *Context, w http.ResponseWriter, r *http.Request) { NotInChannelId: notInChannelId, GroupConstrained: groupConstrainedBool, WithoutTeam: withoutTeamBool, + Active: activeBool, Inactive: inactiveBool, Role: role, Sort: sort, @@ -120,9 +123,9 @@ func localGetUsers(c *Context, w http.ResponseWriter, r *http.Request) { } } else if len(inChannelId) > 0 { if sort == "status" { - profiles, err = c.App.GetUsersInChannelPageByStatus(inChannelId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + profiles, err = c.App.GetUsersInChannelPageByStatus(userGetOptions, c.IsSystemAdmin()) } else { - profiles, err = c.App.GetUsersInChannelPage(inChannelId, c.Params.Page, c.Params.PerPage, c.IsSystemAdmin()) + profiles, err = c.App.GetUsersInChannelPage(userGetOptions, c.IsSystemAdmin()) } } else { profiles, err = c.App.GetUsersPage(userGetOptions, c.IsSystemAdmin()) diff --git a/app/app_iface.go b/app/app_iface.go index e532f7060d..3b07d5e029 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -688,11 +688,11 @@ type AppIface interface { GetUsersByIds(userIds []string, options *store.UserGetByIdsOpts) ([]*model.User, *model.AppError) GetUsersByUsernames(usernames []string, asAdmin bool, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetUsersEtag(restrictionsHash string) string - GetUsersInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) - GetUsersInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) - GetUsersInChannelMap(channelId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) - GetUsersInChannelPage(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) - GetUsersInChannelPageByStatus(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) + GetUsersInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) + GetUsersInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) + GetUsersInChannelMap(options *model.UserGetOptions, asAdmin bool) (map[string]*model.User, *model.AppError) + GetUsersInChannelPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) + GetUsersInChannelPageByStatus(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) GetUsersInTeam(options *model.UserGetOptions) ([]*model.User, *model.AppError) GetUsersInTeamEtag(teamId string, restrictionsHash string) string GetUsersInTeamPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 0298961878..07953950b5 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -8683,7 +8683,7 @@ func (a *OpenTracingAppLayer) GetUsersEtag(restrictionsHash string) string { return resultVar0 } -func (a *OpenTracingAppLayer) GetUsersInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) GetUsersInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannel") @@ -8695,7 +8695,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannel(channelId string, offset int, li }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetUsersInChannel(channelId, offset, limit) + resultVar0, resultVar1 := a.app.GetUsersInChannel(options) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -8705,7 +8705,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannel(channelId string, offset int, li return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetUsersInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) GetUsersInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelByStatus") @@ -8717,7 +8717,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelByStatus(channelId string, offset }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetUsersInChannelByStatus(channelId, offset, limit) + resultVar0, resultVar1 := a.app.GetUsersInChannelByStatus(options) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -8727,7 +8727,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelByStatus(channelId string, offset return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetUsersInChannelMap(channelId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) GetUsersInChannelMap(options *model.UserGetOptions, asAdmin bool) (map[string]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelMap") @@ -8739,7 +8739,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelMap(channelId string, offset int, }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetUsersInChannelMap(channelId, offset, limit, asAdmin) + resultVar0, resultVar1 := a.app.GetUsersInChannelMap(options, asAdmin) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -8749,7 +8749,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelMap(channelId string, offset int, return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetUsersInChannelPage(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) GetUsersInChannelPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelPage") @@ -8761,7 +8761,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelPage(channelId string, page int, }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetUsersInChannelPage(channelId, page, perPage, asAdmin) + resultVar0, resultVar1 := a.app.GetUsersInChannelPage(options, asAdmin) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -8771,7 +8771,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelPage(channelId string, page int, return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetUsersInChannelPageByStatus(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { +func (a *OpenTracingAppLayer) GetUsersInChannelPageByStatus(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelPageByStatus") @@ -8783,7 +8783,7 @@ func (a *OpenTracingAppLayer) GetUsersInChannelPageByStatus(channelId string, pa }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetUsersInChannelPageByStatus(channelId, page, perPage, asAdmin) + resultVar0, resultVar1 := a.app.GetUsersInChannelPageByStatus(options, asAdmin) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/app/plugin_api.go b/app/plugin_api.go index 7bdb8cc66c..5198ecb71c 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -314,9 +314,17 @@ func (api *PluginAPI) UpdateUserStatus(userId, status string) (*model.Status, *m func (api *PluginAPI) GetUsersInChannel(channelId, sortBy string, page, perPage int) ([]*model.User, *model.AppError) { switch sortBy { case model.CHANNEL_SORT_BY_USERNAME: - return api.app.GetUsersInChannel(channelId, page*perPage, perPage) + return api.app.GetUsersInChannel(&model.UserGetOptions{ + InChannelId: channelId, + Page: page, + PerPage: perPage, + }) case model.CHANNEL_SORT_BY_STATUS: - return api.app.GetUsersInChannelByStatus(channelId, page*perPage, perPage) + return api.app.GetUsersInChannelByStatus(&model.UserGetOptions{ + InChannelId: channelId, + Page: page, + PerPage: perPage, + }) default: return nil, model.NewAppError("GetUsersInChannel", "plugin.api.get_users_in_channel", nil, "invalid sort option", http.StatusBadRequest) } diff --git a/app/user.go b/app/user.go index 761310b9d7..5e480c4ed4 100644 --- a/app/user.go +++ b/app/user.go @@ -505,16 +505,16 @@ func (a *App) GetUsersNotInTeamEtag(teamId string, restrictionsHash string) stri return fmt.Sprintf("%v.%v.%v.%v", a.Srv().Store.User().GetEtagForProfilesNotInTeam(teamId), a.Config().PrivacySettings.ShowFullName, a.Config().PrivacySettings.ShowEmailAddress, restrictionsHash) } -func (a *App) GetUsersInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { - return a.Srv().Store.User().GetProfilesInChannel(channelId, offset, limit) +func (a *App) GetUsersInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + return a.Srv().Store.User().GetProfilesInChannel(options) } -func (a *App) GetUsersInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { - return a.Srv().Store.User().GetProfilesInChannelByStatus(channelId, offset, limit) +func (a *App) GetUsersInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + return a.Srv().Store.User().GetProfilesInChannelByStatus(options) } -func (a *App) GetUsersInChannelMap(channelId string, offset int, limit int, asAdmin bool) (map[string]*model.User, *model.AppError) { - users, err := a.GetUsersInChannel(channelId, offset, limit) +func (a *App) GetUsersInChannelMap(options *model.UserGetOptions, asAdmin bool) (map[string]*model.User, *model.AppError) { + users, err := a.GetUsersInChannel(options) if err != nil { return nil, err } @@ -529,16 +529,16 @@ func (a *App) GetUsersInChannelMap(channelId string, offset int, limit int, asAd return userMap, nil } -func (a *App) GetUsersInChannelPage(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { - users, err := a.GetUsersInChannel(channelId, page*perPage, perPage) +func (a *App) GetUsersInChannelPage(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := a.GetUsersInChannel(options) if err != nil { return nil, err } return a.sanitizeProfiles(users, asAdmin), nil } -func (a *App) GetUsersInChannelPageByStatus(channelId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { - users, err := a.GetUsersInChannelByStatus(channelId, page*perPage, perPage) +func (a *App) GetUsersInChannelPageByStatus(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := a.GetUsersInChannelByStatus(options) if err != nil { return nil, err } diff --git a/app/user_test.go b/app/user_test.go index 0c41eabb3c..32b6964bdf 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -528,7 +528,11 @@ func TestGetUsersByStatus(t *testing.T) { onlineUser2 := createUserWithStatus("online2", model.STATUS_ONLINE) t.Run("sorting by status then alphabetical", func(t *testing.T) { - usersByStatus, err := th.App.GetUsersInChannelPageByStatus(channel.Id, 0, 8, true) + usersByStatus, err := th.App.GetUsersInChannelPageByStatus(&model.UserGetOptions{ + InChannelId: channel.Id, + Page: 0, + PerPage: 8, + }, true) require.Nil(t, err) expectedUsersByStatus := []*model.User{ @@ -550,7 +554,11 @@ func TestGetUsersByStatus(t *testing.T) { }) t.Run("paging", func(t *testing.T) { - usersByStatus, err := th.App.GetUsersInChannelPageByStatus(channel.Id, 0, 3, true) + usersByStatus, err := th.App.GetUsersInChannelPageByStatus(&model.UserGetOptions{ + InChannelId: channel.Id, + Page: 0, + PerPage: 3, + }, true) require.Nil(t, err) require.Equal(t, 3, len(usersByStatus), "received too many users") @@ -563,9 +571,14 @@ func TestGetUsersByStatus(t *testing.T) { require.Equal(t, awayUser1.Id, usersByStatus[2].Id, "expected to receive away users second") - usersByStatus, err = th.App.GetUsersInChannelPageByStatus(channel.Id, 1, 3, true) + usersByStatus, err = th.App.GetUsersInChannelPageByStatus(&model.UserGetOptions{ + InChannelId: channel.Id, + Page: 1, + PerPage: 3, + }, true) require.Nil(t, err) + require.NotEmpty(t, usersByStatus, "at least some users are expected") require.Equal(t, awayUser2.Id, usersByStatus[0].Id, "expected to receive away users second") require.False( @@ -574,7 +587,11 @@ func TestGetUsersByStatus(t *testing.T) { "expected to receive dnd users third", ) - usersByStatus, err = th.App.GetUsersInChannelPageByStatus(channel.Id, 1, 4, true) + usersByStatus, err = th.App.GetUsersInChannelPageByStatus(&model.UserGetOptions{ + InChannelId: channel.Id, + Page: 1, + PerPage: 4, + }, true) require.Nil(t, err) require.Equal(t, 4, len(usersByStatus), "received too many users") diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 572c455a87..5d6edfc3c2 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -8215,7 +8215,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesByUsernames(usernames []string, v return result, err } -func (s *OpenTracingLayerUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (s *OpenTracingLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetProfilesInChannel") s.Root.Store.SetContext(newCtx) @@ -8224,7 +8224,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannel(channelId string, offse }() defer span.Finish() - result, err := s.UserStore.GetProfilesInChannel(channelId, offset, limit) + result, err := s.UserStore.GetProfilesInChannel(options) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) @@ -8233,7 +8233,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannel(channelId string, offse return result, err } -func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetProfilesInChannelByStatus") s.Root.Store.SetContext(newCtx) @@ -8242,7 +8242,7 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(channelId strin }() defer span.Finish() - result, err := s.UserStore.GetProfilesInChannelByStatus(channelId, offset, limit) + result, err := s.UserStore.GetProfilesInChannelByStatus(options) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index e5955191f0..e31d212313 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -6844,15 +6844,15 @@ func (s *RetryLayerUserStore) GetProfilesByUsernames(usernames []string, viewRes } -func (s *RetryLayerUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (s *RetryLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { - return s.UserStore.GetProfilesInChannel(channelId, offset, limit) + return s.UserStore.GetProfilesInChannel(options) } -func (s *RetryLayerUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (s *RetryLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { - return s.UserStore.GetProfilesInChannelByStatus(channelId, offset, limit) + return s.UserStore.GetProfilesInChannelByStatus(options) } diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 91814f7ec9..bc49e717fc 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -667,12 +667,18 @@ func (us SqlUserStore) InvalidateProfilesInChannelCacheByUser(userId string) {} func (us SqlUserStore) InvalidateProfilesInChannelCache(channelId string) {} -func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (us SqlUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { query := us.usersQuery. Join("ChannelMembers cm ON ( cm.UserId = u.Id )"). - Where("cm.ChannelId = ?", channelId). + Where("cm.ChannelId = ?", options.InChannelId). OrderBy("u.Username ASC"). - Offset(uint64(offset)).Limit(uint64(limit)) + Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage)) + + if options.Inactive { + query = query.Where("u.DeleteAt != 0") + } else if options.Active { + query = query.Where("u.DeleteAt = 0") + } queryString, args, err := query.ToSql() if err != nil { @@ -691,11 +697,11 @@ func (us SqlUserStore) GetProfilesInChannel(channelId string, offset int, limit return users, nil } -func (us SqlUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (us SqlUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { query := us.usersQuery. Join("ChannelMembers cm ON ( cm.UserId = u.Id )"). LeftJoin("Status s ON ( s.UserId = u.Id )"). - Where("cm.ChannelId = ?", channelId). + Where("cm.ChannelId = ?", options.InChannelId). OrderBy(` CASE s.Status WHEN 'online' THEN 1 @@ -705,7 +711,13 @@ func (us SqlUserStore) GetProfilesInChannelByStatus(channelId string, offset int END `). OrderBy("u.Username ASC"). - Offset(uint64(offset)).Limit(uint64(limit)) + Offset(uint64(options.Page * options.PerPage)).Limit(uint64(options.PerPage)) + + if options.Inactive && !options.Active { + query = query.Where("u.DeleteAt != 0") + } else if options.Active && !options.Inactive { + query = query.Where("u.DeleteAt = 0") + } queryString, args, err := query.ToSql() if err != nil { diff --git a/store/store.go b/store/store.go index ce44c2fa06..c94b78d09e 100644 --- a/store/store.go +++ b/store/store.go @@ -302,8 +302,8 @@ type UserStore interface { ClearCaches() InvalidateProfilesInChannelCacheByUser(userId string) InvalidateProfilesInChannelCache(channelId string) - GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) - GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) + GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) + GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) GetAllProfilesInChannel(channelId string, allowFromCache bool) (map[string]*model.User, *model.AppError) GetProfilesNotInChannel(teamId string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) GetProfilesWithoutTeam(options *model.UserGetOptions) ([]*model.User, *model.AppError) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 8500a844f3..20e838ca62 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -732,13 +732,13 @@ func (_m *UserStore) GetProfilesByUsernames(usernames []string, viewRestrictions return r0, r1 } -// GetProfilesInChannel provides a mock function with given fields: channelId, offset, limit -func (_m *UserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { - ret := _m.Called(channelId, offset, limit) +// GetProfilesInChannel provides a mock function with given fields: options +func (_m *UserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + ret := _m.Called(options) var r0 []*model.User - if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok { - r0 = rf(channelId, offset, limit) + if rf, ok := ret.Get(0).(func(*model.UserGetOptions) []*model.User); ok { + r0 = rf(options) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.User) @@ -746,8 +746,8 @@ func (_m *UserStore) GetProfilesInChannel(channelId string, offset int, limit in } var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok { - r1 = rf(channelId, offset, limit) + if rf, ok := ret.Get(1).(func(*model.UserGetOptions) *model.AppError); ok { + r1 = rf(options) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError) @@ -757,13 +757,13 @@ func (_m *UserStore) GetProfilesInChannel(channelId string, offset int, limit in return r0, r1 } -// GetProfilesInChannelByStatus provides a mock function with given fields: channelId, offset, limit -func (_m *UserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { - ret := _m.Called(channelId, offset, limit) +// GetProfilesInChannelByStatus provides a mock function with given fields: options +func (_m *UserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + ret := _m.Called(options) var r0 []*model.User - if rf, ok := ret.Get(0).(func(string, int, int) []*model.User); ok { - r0 = rf(channelId, offset, limit) + if rf, ok := ret.Get(0).(func(*model.UserGetOptions) []*model.User); ok { + r0 = rf(options) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.User) @@ -771,8 +771,8 @@ func (_m *UserStore) GetProfilesInChannelByStatus(channelId string, offset int, } var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok { - r1 = rf(channelId, offset, limit) + if rf, ok := ret.Get(1).(func(*model.UserGetOptions) *model.AppError); ok { + r1 = rf(options) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 1e968c714d..08c385220a 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -756,6 +756,15 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) { u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() + u4, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "u4" + model.NewId(), + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(u4.Id)) }() + _, nErr = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u4.Id}, -1) + require.Nil(t, nErr) + ch1 := &model.Channel{ TeamId: teamId, DisplayName: "Profiles in channel", @@ -795,26 +804,79 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) { }) require.Nil(t, nErr) + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: c1.Id, + UserId: u4.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.Nil(t, nErr) + + u4.DeleteAt = 1 + _, err = ss.User().Update(u4, true) + require.Nil(t, err) + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ ChannelId: c2.Id, UserId: u1.Id, NotifyProps: model.GetDefaultChannelNotifyProps(), }) require.Nil(t, nErr) - t.Run("get in channel 1, offset 0, limit 100", func(t *testing.T) { - users, err := ss.User().GetProfilesInChannel(c1.Id, 0, 100) + + t.Run("get all users in channel 1, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + }) + require.Nil(t, err) + assert.Equal(t, []*model.User{sanitized(u1), sanitized(u2), sanitized(u3), sanitized(u4)}, users) + }) + + t.Run("get only active users in channel 1, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + Active: true, + }) require.Nil(t, err) assert.Equal(t, []*model.User{sanitized(u1), sanitized(u2), sanitized(u3)}, users) }) - t.Run("get in channel 1, offset 1, limit 2", func(t *testing.T) { - users, err := ss.User().GetProfilesInChannel(c1.Id, 1, 2) + t.Run("get inactive users in channel 1, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + Inactive: true, + }) require.Nil(t, err) + assert.Equal(t, []*model.User{sanitized(u4)}, users) + }) + + t.Run("get in channel 1, offset 1, limit 2", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 1, + PerPage: 1, + }) + require.Nil(t, err) + users_p2, err2 := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 2, + PerPage: 1, + }) + require.Nil(t, err2) + users = append(users, users_p2...) assert.Equal(t, []*model.User{sanitized(u2), sanitized(u3)}, users) }) t.Run("get in channel 2, offset 0, limit 1", func(t *testing.T) { - users, err := ss.User().GetProfilesInChannel(c2.Id, 0, 1) + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c2.Id, + Page: 0, + PerPage: 1, + }) require.Nil(t, err) assert.Equal(t, []*model.User{sanitized(u1)}, users) }) @@ -861,6 +923,15 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S u3.IsBot = true defer func() { require.Nil(t, ss.Bot().PermanentDelete(u3.Id)) }() + u4, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "u4" + model.NewId(), + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(u4.Id)) }() + _, nErr = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u4.Id}, -1) + require.Nil(t, nErr) + ch1 := &model.Channel{ TeamId: teamId, DisplayName: "Profiles in channel", @@ -900,6 +971,17 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S }) require.Nil(t, nErr) + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: c1.Id, + UserId: u4.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.Nil(t, nErr) + + u4.DeleteAt = 1 + _, err = ss.User().Update(u4, true) + require.Nil(t, err) + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ ChannelId: c2.Id, UserId: u1.Id, @@ -919,14 +1001,44 @@ func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s S Status: model.STATUS_ONLINE, })) - t.Run("get in channel 1 by status, offset 0, limit 100", func(t *testing.T) { - users, err := ss.User().GetProfilesInChannelByStatus(c1.Id, 0, 100) + t.Run("get all users in channel 1, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + }) + require.Nil(t, err) + assert.Equal(t, []*model.User{sanitized(u1), sanitized(u2), sanitized(u3), sanitized(u4)}, users) + }) + + t.Run("get active in channel 1 by status, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannelByStatus(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + Active: true, + }) require.Nil(t, err) assert.Equal(t, []*model.User{sanitized(u3), sanitized(u2), sanitized(u1)}, users) }) + t.Run("get inactive users in channel 1, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannel(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + Inactive: true, + }) + require.Nil(t, err) + assert.Equal(t, []*model.User{sanitized(u4)}, users) + }) + t.Run("get in channel 2 by status, offset 0, limit 1", func(t *testing.T) { - users, err := ss.User().GetProfilesInChannelByStatus(c2.Id, 0, 1) + users, err := ss.User().GetProfilesInChannelByStatus(&model.UserGetOptions{ + InChannelId: c2.Id, + Page: 0, + PerPage: 1, + }) require.Nil(t, err) assert.Equal(t, []*model.User{sanitized(u1)}, users) }) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index d7819241a3..1d3f62bb69 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -7426,10 +7426,10 @@ func (s *TimerLayerUserStore) GetProfilesByUsernames(usernames []string, viewRes return result, err } -func (s *TimerLayerUserStore) GetProfilesInChannel(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (s *TimerLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) { start := timemodule.Now() - result, err := s.UserStore.GetProfilesInChannel(channelId, offset, limit) + result, err := s.UserStore.GetProfilesInChannel(options) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil { @@ -7442,10 +7442,10 @@ func (s *TimerLayerUserStore) GetProfilesInChannel(channelId string, offset int, return result, err } -func (s *TimerLayerUserStore) GetProfilesInChannelByStatus(channelId string, offset int, limit int) ([]*model.User, *model.AppError) { +func (s *TimerLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { start := timemodule.Now() - result, err := s.UserStore.GetProfilesInChannelByStatus(channelId, offset, limit) + result, err := s.UserStore.GetProfilesInChannelByStatus(options) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil {