From b45ff0be5d61e0f5be402f1fbad6ea3de837f92a Mon Sep 17 00:00:00 2001 From: Julien Tant <785518+JulienTant@users.noreply.github.com> Date: Thu, 30 Jun 2022 09:20:14 -0700 Subject: [PATCH] [MM-44159] Add user sort by admin status in channels (#20562) --- api4/user.go | 8 ++- app/app_iface.go | 2 + app/opentracing/opentracing_layer.go | 44 ++++++++++++ app/user.go | 17 +++++ store/opentracinglayer/opentracinglayer.go | 18 +++++ store/retrylayer/retrylayer.go | 21 ++++++ store/sqlstore/user_store.go | 31 +++++++++ store/store.go | 1 + store/storetest/mocks/UserStore.go | 23 +++++++ store/storetest/user_store.go | 80 ++++++++++++++++++++++ store/timerlayer/timerlayer.go | 16 +++++ 11 files changed, 260 insertions(+), 1 deletion(-) diff --git a/api4/user.go b/api4/user.go index 3b45822a0b..32de510554 100644 --- a/api4/user.go +++ b/api4/user.go @@ -652,7 +652,7 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { return } - if sort != "" && sort != "last_activity_at" && sort != "create_at" && sort != "status" { + if sort != "" && sort != "last_activity_at" && sort != "create_at" && sort != "status" && sort != "admin" { c.SetInvalidURLParam("sort") return } @@ -667,6 +667,10 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { c.SetInvalidURLParam("sort") return } + if sort == "admin" && inChannelId == "" { + c.SetInvalidURLParam("sort") + return + } withoutTeamBool, _ := strconv.ParseBool(withoutTeam) groupConstrainedBool, _ := strconv.ParseBool(groupConstrained) @@ -799,6 +803,8 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) { if sort == "status" { profiles, err = c.App.GetUsersInChannelPageByStatus(userGetOptions, c.IsSystemAdmin()) + } else if sort == "admin" { + profiles, err = c.App.GetUsersInChannelPageByAdmin(userGetOptions, c.IsSystemAdmin()) } else { profiles, err = c.App.GetUsersInChannelPage(userGetOptions, c.IsSystemAdmin()) } diff --git a/app/app_iface.go b/app/app_iface.go index c75fad5249..58baa227d8 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -803,9 +803,11 @@ type AppIface interface { GetUsersEtag(restrictionsHash string) string GetUsersFromProfiles(options *model.UserGetOptions) ([]*model.User, *model.AppError) GetUsersInChannel(options *model.UserGetOptions) ([]*model.User, *model.AppError) + GetUsersInChannelByAdmin(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) + GetUsersInChannelPageByAdmin(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 diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index a824fef3f1..288071b431 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -10383,6 +10383,28 @@ func (a *OpenTracingAppLayer) GetUsersInChannel(options *model.UserGetOptions) ( return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) GetUsersInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelByAdmin") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetUsersInChannelByAdmin(options) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetUsersInChannelByStatus(options *model.UserGetOptions) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelByStatus") @@ -10449,6 +10471,28 @@ func (a *OpenTracingAppLayer) GetUsersInChannelPage(options *model.UserGetOption return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) GetUsersInChannelPageByAdmin(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelPageByAdmin") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetUsersInChannelPageByAdmin(options, asAdmin) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetUsersInChannelPageByStatus(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetUsersInChannelPageByStatus") diff --git a/app/user.go b/app/user.go index 563d4e644c..ffdc112ba8 100644 --- a/app/user.go +++ b/app/user.go @@ -530,6 +530,15 @@ func (a *App) GetUsersInChannelByStatus(options *model.UserGetOptions) ([]*model return users, nil } +func (a *App) GetUsersInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, *model.AppError) { + users, err := a.Srv().Store.User().GetProfilesInChannelByAdmin(options) + if err != nil { + return nil, model.NewAppError("GetUsersInChannelByAdmin", "app.user.get_profiles.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return users, nil +} + func (a *App) GetUsersInChannelMap(options *model.UserGetOptions, asAdmin bool) (map[string]*model.User, *model.AppError) { users, err := a.GetUsersInChannel(options) if err != nil { @@ -562,6 +571,14 @@ func (a *App) GetUsersInChannelPageByStatus(options *model.UserGetOptions, asAdm return a.sanitizeProfiles(users, asAdmin), nil } +func (a *App) GetUsersInChannelPageByAdmin(options *model.UserGetOptions, asAdmin bool) ([]*model.User, *model.AppError) { + users, err := a.GetUsersInChannelByAdmin(options) + if err != nil { + return nil, err + } + return a.sanitizeProfiles(users, asAdmin), nil +} + func (a *App) GetUsersNotInChannel(teamID string, channelID string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, *model.AppError) { users, err := a.Srv().Store.User().GetProfilesNotInChannel(teamID, channelID, groupConstrained, offset, limit, viewRestrictions) if err != nil { diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 9215f35763..06e0b9b999 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -10719,6 +10719,24 @@ func (s *OpenTracingLayerUserStore) GetProfilesInChannel(options *model.UserGetO return result, err } +func (s *OpenTracingLayerUserStore) GetProfilesInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetProfilesInChannelByAdmin") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.UserStore.GetProfilesInChannelByAdmin(options) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetProfilesInChannelByStatus") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index aada214d48..2accc6c080 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -12230,6 +12230,27 @@ func (s *RetryLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions } +func (s *RetryLayerUserStore) GetProfilesInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, error) { + + tries := 0 + for { + result, err := s.UserStore.GetProfilesInChannelByAdmin(options) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, error) { tries := 0 diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index ecf1e33b80..38d441001e 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -771,6 +771,37 @@ func (us SqlUserStore) GetProfilesInChannelByStatus(options *model.UserGetOption return users, nil } +func (us SqlUserStore) GetProfilesInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, error) { + query := us.usersQuery. + Join("ChannelMembers cm ON ( cm.UserId = u.Id )"). + Where("cm.ChannelId = ?", options.InChannelId). + OrderBy(`cm.SchemeAdmin DESC`). + OrderBy("u.Username ASC"). + 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 { + return nil, errors.Wrap(err, "get_profiles_in_channel_by_admin_tosql") + } + + users := []*model.User{} + if err := us.GetReplicaX().Select(&users, queryString, args...); err != nil { + return nil, errors.Wrap(err, "failed to find Users") + } + + for _, u := range users { + u.Sanitize(map[string]bool{}) + } + + return users, nil +} + func (us SqlUserStore) GetAllProfilesInChannel(ctx context.Context, channelID string, allowFromCache bool) (map[string]*model.User, error) { query := us.usersQuery. Join("ChannelMembers cm ON ( cm.UserId = u.Id )"). diff --git a/store/store.go b/store/store.go index 9d535e0449..801351add5 100644 --- a/store/store.go +++ b/store/store.go @@ -407,6 +407,7 @@ type UserStore interface { InvalidateProfilesInChannelCache(channelID string) GetProfilesInChannel(options *model.UserGetOptions) ([]*model.User, error) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, error) + GetProfilesInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, error) GetAllProfilesInChannel(ctx context.Context, channelID string, allowFromCache bool) (map[string]*model.User, error) GetProfilesNotInChannel(teamID string, channelId string, groupConstrained bool, offset int, limit int, viewRestrictions *model.ViewUsersRestrictions) ([]*model.User, error) GetProfilesWithoutTeam(options *model.UserGetOptions) ([]*model.User, error) diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 64063378a9..afb1ae0c9a 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -776,6 +776,29 @@ func (_m *UserStore) GetProfilesInChannel(options *model.UserGetOptions) ([]*mod return r0, r1 } +// GetProfilesInChannelByAdmin provides a mock function with given fields: options +func (_m *UserStore) GetProfilesInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, error) { + ret := _m.Called(options) + + var r0 []*model.User + 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) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.UserGetOptions) error); ok { + r1 = rf(options) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetProfilesInChannelByStatus provides a mock function with given fields: options func (_m *UserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, error) { ret := _m.Called(options) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index ed15020c6c..7214b98b0b 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -54,6 +54,7 @@ func TestUserStore(t *testing.T, ss store.Store, s SqlStore) { t.Run("GetProfiles", func(t *testing.T) { testUserStoreGetProfiles(t, ss) }) t.Run("GetProfilesInChannel", func(t *testing.T) { testUserStoreGetProfilesInChannel(t, ss) }) t.Run("GetProfilesInChannelByStatus", func(t *testing.T) { testUserStoreGetProfilesInChannelByStatus(t, ss, s) }) + t.Run("GetProfilesInChannelByAdmin", func(t *testing.T) { testUserStoreGetProfilesInChannelByAdmin(t, ss, s) }) t.Run("GetProfilesWithoutTeam", func(t *testing.T) { testUserStoreGetProfilesWithoutTeam(t, ss) }) t.Run("GetAllProfilesInChannel", func(t *testing.T) { testUserStoreGetAllProfilesInChannel(t, ss) }) t.Run("GetProfilesNotInChannel", func(t *testing.T) { testUserStoreGetProfilesNotInChannel(t, ss) }) @@ -981,6 +982,85 @@ func testUserStoreGetProfilesInChannel(t *testing.T, ss store.Store) { }) } +func testUserStoreGetProfilesInChannelByAdmin(t *testing.T, ss store.Store, s SqlStore) { + + cleanupStatusStore(t, s) + + teamId := model.NewId() + + user1, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "aaa" + model.NewId(), + }) + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(user1.Id)) }() + _, nErr := ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user1.Id}, -1) + require.NoError(t, nErr) + + user2Admin, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "bbb" + model.NewId(), + }) + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(user2Admin.Id)) }() + _, nErr = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user2Admin.Id}, -1) + require.NoError(t, nErr) + + user3, err := ss.User().Save(&model.User{ + Email: MakeEmail(), + Username: "ccc" + model.NewId(), + }) + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(user3.Id)) }() + _, nErr = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user3.Id}, -1) + require.NoError(t, nErr) + + ch1 := &model.Channel{ + TeamId: teamId, + DisplayName: "Profiles in channel by admin", + Name: "profiles-" + model.NewId(), + Type: model.ChannelTypeOpen, + } + c1, nErr := ss.Channel().Save(ch1, -1) + require.NoError(t, nErr) + + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: c1.Id, + UserId: user1.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, nErr) + + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: c1.Id, + UserId: user2Admin.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + ExplicitRoles: "channel_admin", + }) + require.NoError(t, nErr) + ss.Channel().UpdateMembersRole(c1.Id, []string{user2Admin.Id}) + + _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ + ChannelId: c1.Id, + UserId: user3.Id, + NotifyProps: model.GetDefaultChannelNotifyProps(), + }) + require.NoError(t, nErr) + + t.Run("get users in admin, offset 0, limit 100", func(t *testing.T) { + users, err := ss.User().GetProfilesInChannelByAdmin(&model.UserGetOptions{ + InChannelId: c1.Id, + Page: 0, + PerPage: 100, + }) + require.NoError(t, err) + require.Len(t, users, 3) + require.Equal(t, user2Admin.Username, users[0].Username) + require.Equal(t, user1.Username, users[1].Username) + require.Equal(t, user3.Username, users[2].Username) + }) +} + func testUserStoreGetProfilesInChannelByStatus(t *testing.T, ss store.Store, s SqlStore) { cleanupStatusStore(t, s) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index cc3178473c..209f6c528e 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -9653,6 +9653,22 @@ func (s *TimerLayerUserStore) GetProfilesInChannel(options *model.UserGetOptions return result, err } +func (s *TimerLayerUserStore) GetProfilesInChannelByAdmin(options *model.UserGetOptions) ([]*model.User, error) { + start := time.Now() + + result, err := s.UserStore.GetProfilesInChannelByAdmin(options) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("UserStore.GetProfilesInChannelByAdmin", success, elapsed) + } + return result, err +} + func (s *TimerLayerUserStore) GetProfilesInChannelByStatus(options *model.UserGetOptions) ([]*model.User, error) { start := time.Now()