diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index 15d1790c6d..7209779f85 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -2288,9 +2288,9 @@ func (us SqlUserStore) GetUserReport(filter *model.UserReportOptions) ([]*model. ) } - sortColumnValue := filter.SortColumn + sortDirection := "ASC" if filter.SortDesc { - sortColumnValue += " DESC" + sortDirection = "DESC" } query := us.getQueryBuilder(). @@ -2298,27 +2298,36 @@ func (us SqlUserStore) GetUserReport(filter *model.UserReportOptions) ([]*model. From("Users u"). LeftJoin("Status s ON s.UserId = u.Id"). Where(sq.Expr("u.Id NOT IN (SELECT UserId FROM Bots)")). - GroupBy("u.Id"). - OrderBy(sortColumnValue, "u.Id") + GroupBy("u.Id") - if (filter.Direction == "up" && !filter.SortDesc) || (filter.Direction == "down" && filter.SortDesc) { - query = query.Where(sq.Or{ - sq.Lt{filter.SortColumn: filter.FromColumnValue}, - sq.And{ - sq.Eq{filter.SortColumn: filter.FromColumnValue}, - sq.Lt{"u.Id": filter.FromId}, - }, - }) - } else { - query = query.Where(sq.Or{ - sq.Gt{filter.SortColumn: filter.FromColumnValue}, - sq.And{ - sq.Eq{filter.SortColumn: filter.FromColumnValue}, - sq.Gt{"u.Id": filter.FromId}, - }, - }) + // no need to apply any filtering and pagination if there are no + // previous element ID and value provided. + if filter.FromId != "" && filter.FromColumnValue != "" { + if (filter.Direction == "up" && !filter.SortDesc) || (filter.Direction == "down" && filter.SortDesc) { + sortDirection = "DESC" + + query = query.Where(sq.Or{ + sq.Lt{filter.SortColumn: filter.FromColumnValue}, + sq.And{ + sq.Eq{filter.SortColumn: filter.FromColumnValue}, + sq.Lt{"u.Id": filter.FromId}, + }, + }) + } else { + sortDirection = "ASC" + + query = query.Where(sq.Or{ + sq.Gt{filter.SortColumn: filter.FromColumnValue}, + sq.And{ + sq.Eq{filter.SortColumn: filter.FromColumnValue}, + sq.Gt{"u.Id": filter.FromId}, + }, + }) + } } + query = query.OrderBy(filter.SortColumn+" "+sortDirection, "u.Id") + if filter.PageSize > 0 { query = query.Limit(uint64(filter.PageSize)) } @@ -2369,8 +2378,26 @@ func (us SqlUserStore) GetUserReport(filter *model.UserReportOptions) ([]*model. query = query.Where(sq.Eq{"u.DeleteAt": 0}) } + parentQuery := query + // If we're going a page back... + // + // The way pagination works, we get the previous page's rows + // in reverse order. So, we use parent query on it to + // reverse the order in database itself. + if filter.Direction == "up" { + reverseSortDirection := "ASC" + if sortDirection == "ASC" { + reverseSortDirection = "DESC" + } + + parentQuery = us.getQueryBuilder(). + Select("*"). + FromSelect(query, "data"). + OrderBy(filter.SortColumn+" "+reverseSortDirection, "Id") + } + userResults := []*model.UserReportQuery{} - err := us.GetReplicaX().SelectBuilder(&userResults, query) + err := us.GetReplicaX().SelectBuilder(&userResults, parentQuery) if err != nil { return nil, errors.Wrap(err, "failed to get users for reporting") } diff --git a/server/channels/store/storetest/user_store.go b/server/channels/store/storetest/user_store.go index 870e9ef8b2..966d0a3f9a 100644 --- a/server/channels/store/storetest/user_store.go +++ b/server/channels/store/storetest/user_store.go @@ -6,6 +6,8 @@ package storetest import ( "context" "errors" + "fmt" + "sort" "strings" "testing" "time" @@ -6188,59 +6190,72 @@ func testUpdateLastLogin(t *testing.T, rctx request.CTX, ss store.Store) { } func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { + numRegularUsers := 90 + numSysAdmins := 10 + numInactiveUsers := 15 + numUsers := numRegularUsers + numSysAdmins + numInactiveUsers + + numPostsPerUser := 10 + now := time.Now() - u1 := &model.User{Username: "u1" + model.NewId(), DeleteAt: 0} - u1.Email = MakeEmail() - u1, err := ss.User().Save(u1) - require.NoError(t, err) - defer func() { require.NoError(t, ss.User().PermanentDelete(u1.Id)) }() + users := make([]*model.User, numUsers) + for i := 0; i < numUsers; i++ { + user := &model.User{Username: fmt.Sprintf("username_%d", i), DeleteAt: 0} + user.Email = MakeEmail() - for i := 0; i < 5; i++ { - p := model.Post{UserId: u1.Id, ChannelId: model.NewId(), Message: NewTestId(), CreateAt: now.AddDate(0, 0, -i).UnixMilli()} - _, err = ss.Post().Save(&p) + if i >= numRegularUsers && i < numRegularUsers+numSysAdmins { + user.Roles = "system" + } + + if i >= numRegularUsers+numSysAdmins { + user.DeleteAt = now.UnixMilli() + } + + user, err := ss.User().Save(user) require.NoError(t, err) + users[i] = user } - u2 := &model.User{Username: "u2" + model.NewId(), Roles: "system", DeleteAt: 0} - u2.Email = MakeEmail() - u2, err = ss.User().Save(u2) - require.NoError(t, err) - defer func() { require.NoError(t, ss.User().PermanentDelete(u2.Id)) }() + sort.Slice(users, func(i, j int) bool { + return users[i].Username < users[j].Username + }) - for i := 0; i < 5; i++ { - p := model.Post{UserId: u2.Id, ChannelId: model.NewId(), Message: NewTestId(), CreateAt: now.AddDate(0, 0, -i).UnixMilli()} - _, err = ss.Post().Save(&p) - require.NoError(t, err) + // cleanup users after the test + defer func() { + for _, user := range users { + require.NoError(t, ss.User().PermanentDelete(user.Id)) + } + }() + + for _, user := range users { + for i := 0; i < numPostsPerUser; i++ { + post := model.Post{UserId: user.Id, ChannelId: model.NewId(), Message: NewTestId(), CreateAt: now.AddDate(0, 0, -i).UnixMilli()} + _, err := ss.Post().Save(&post) + require.NoError(t, err) + } } - u3 := &model.User{Username: "u3" + model.NewId(), Roles: "guest", DeleteAt: now.UnixMilli()} - u3.Email = MakeEmail() - u3, err = ss.User().Save(u3) - require.NoError(t, err) - team, err := ss.Team().Save(&model.Team{ - DisplayName: "DisplayName", + DisplayName: model.NewId(), Name: NewTestId(), Email: MakeEmail(), Type: model.TeamOpen, }) require.NoError(t, err) - _, err = ss.Team().SaveMember(&model.TeamMember{UserId: u3.Id, TeamId: team.Id}, 1) + + _, err = ss.Team().SaveMember(&model.TeamMember{UserId: users[0].Id, TeamId: team.Id}, 100) require.NoError(t, err) + + _, err = ss.Team().SaveMember(&model.TeamMember{UserId: users[1].Id, TeamId: team.Id}, 100) + require.NoError(t, err) + defer func() { - require.NoError(t, ss.Team().RemoveMember(rctx, team.Id, u3.Id)) + require.NoError(t, ss.Team().RemoveMember(rctx, team.Id, users[0].Id)) + require.NoError(t, ss.Team().RemoveMember(rctx, team.Id, users[1].Id)) require.NoError(t, ss.Team().PermanentDelete(team.Id)) }() - defer func() { require.NoError(t, ss.User().PermanentDelete(u3.Id)) }() - - for i := 0; i < 5; i++ { - p := model.Post{UserId: u3.Id, ChannelId: model.NewId(), Message: NewTestId(), CreateAt: now.AddDate(0, 0, -i).UnixMilli()} - _, err = ss.Post().Save(&p) - require.NoError(t, err) - } - err = ss.User().RefreshPostStatsForUsers() require.NoError(t, err) @@ -6253,16 +6268,16 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { }) require.NoError(t, err) require.NotNil(t, userReport) - require.Len(t, userReport, 3) + require.Len(t, userReport, 50) require.NotNil(t, userReport[0]) - require.Equal(t, u1.Username, userReport[0].Username) + require.Equal(t, users[0].Username, userReport[0].Username) require.NotNil(t, userReport[1]) - require.Equal(t, u2.Username, userReport[1].Username) + require.Equal(t, users[1].Username, userReport[1].Username) require.NotNil(t, userReport[2]) - require.Equal(t, u3.Username, userReport[2].Username) + require.Equal(t, users[2].Username, userReport[2].Username) }) t.Run("should return in the correct order", func(t *testing.T) { @@ -6275,16 +6290,16 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { }) require.NoError(t, err) require.NotNil(t, userReport) - require.Equal(t, 3, len(userReport)) + require.Len(t, userReport, 50) require.NotNil(t, userReport[0]) - require.Equal(t, u3.Username, userReport[0].Username) + require.Equal(t, users[numUsers-1].Username, userReport[0].Username) require.NotNil(t, userReport[1]) - require.Equal(t, u2.Username, userReport[1].Username) + require.Equal(t, users[numUsers-2].Username, userReport[1].Username) require.NotNil(t, userReport[2]) - require.Equal(t, u1.Username, userReport[2].Username) + require.Equal(t, users[numUsers-3].Username, userReport[2].Username) }) t.Run("should fail on invalid sort column", func(t *testing.T) { @@ -6311,10 +6326,10 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { require.Equal(t, 2, len(userReport)) require.NotNil(t, userReport[0]) - require.Equal(t, u1.Username, userReport[0].Username) + require.Equal(t, users[0].Username, userReport[0].Username) require.NotNil(t, userReport[1]) - require.Equal(t, u2.Username, userReport[1].Username) + require.Equal(t, users[1].Username, userReport[1].Username) }) t.Run("should return correct paging", func(t *testing.T) { @@ -6323,16 +6338,16 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { SortColumn: "Username", Direction: "down", PageSize: 50, - FromColumnValue: u2.Username, - FromId: u2.Id, + FromColumnValue: users[10].Username, + FromId: users[10].Id, }, }) require.NoError(t, err) require.NotNil(t, userReport) - require.Equal(t, 1, len(userReport)) + require.Equal(t, 50, len(userReport)) require.NotNil(t, userReport[0]) - require.Equal(t, u3.Username, userReport[0].Username) + require.Equal(t, users[11].Username, userReport[0].Username) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ @@ -6340,32 +6355,32 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { SortDesc: true, Direction: "down", PageSize: 50, - FromColumnValue: u2.Username, - FromId: u2.Id, + FromColumnValue: users[10].Username, + FromId: users[10].Id, }, }) require.NoError(t, err) require.NotNil(t, userReport) - require.Equal(t, 1, len(userReport)) + require.Equal(t, 10, len(userReport)) require.NotNil(t, userReport[0]) - require.Equal(t, u1.Username, userReport[0].Username) + require.Equal(t, users[9].Username, userReport[0].Username) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ SortColumn: "Username", Direction: "up", PageSize: 50, - FromColumnValue: u2.Username, - FromId: u2.Id, + FromColumnValue: users[10].Username, + FromId: users[10].Id, }, }) require.NoError(t, err) require.NotNil(t, userReport) - require.Equal(t, 1, len(userReport)) + require.Equal(t, 10, len(userReport)) require.NotNil(t, userReport[0]) - require.Equal(t, u1.Username, userReport[0].Username) + require.Equal(t, users[0].Username, userReport[0].Username) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ @@ -6373,16 +6388,16 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { SortDesc: true, Direction: "up", PageSize: 50, - FromColumnValue: u2.Username, - FromId: u2.Id, + FromColumnValue: users[10].Username, + FromId: users[10].Id, }, }) require.NoError(t, err) require.NotNil(t, userReport) - require.Equal(t, 1, len(userReport)) + require.Equal(t, 50, len(userReport)) require.NotNil(t, userReport[0]) - require.Equal(t, u3.Username, userReport[0].Username) + require.Equal(t, users[60].Username, userReport[0].Username) }) t.Run("should return accurate post stats for various date ranges", func(t *testing.T) { @@ -6393,16 +6408,16 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { }, }) require.NoError(t, err) - require.Len(t, userReport, 3) + require.Len(t, userReport, 50) - require.Equal(t, 5, *userReport[0].TotalPosts) - require.Equal(t, 5, *userReport[0].DaysActive) + require.Equal(t, numPostsPerUser, *userReport[0].TotalPosts) + require.Equal(t, numPostsPerUser, *userReport[0].DaysActive) require.Equal(t, now.UnixMilli(), *userReport[0].LastPostDate) - require.Equal(t, 5, *userReport[1].TotalPosts) - require.Equal(t, 5, *userReport[1].DaysActive) + require.Equal(t, numPostsPerUser, *userReport[1].TotalPosts) + require.Equal(t, numPostsPerUser, *userReport[1].DaysActive) require.Equal(t, now.UnixMilli(), *userReport[1].LastPostDate) - require.Equal(t, 5, *userReport[2].TotalPosts) - require.Equal(t, 5, *userReport[2].DaysActive) + require.Equal(t, numPostsPerUser, *userReport[2].TotalPosts) + require.Equal(t, numPostsPerUser, *userReport[2].DaysActive) require.Equal(t, now.UnixMilli(), *userReport[2].LastPostDate) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ @@ -6413,7 +6428,7 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { }, }) require.NoError(t, err) - require.Len(t, userReport, 3) + require.Len(t, userReport, 50) require.Equal(t, 3, *userReport[0].TotalPosts) require.Equal(t, 3, *userReport[0].DaysActive) @@ -6433,16 +6448,16 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { }, }) require.NoError(t, err) - require.Len(t, userReport, 3) + require.Len(t, userReport, 50) - require.Equal(t, 2, *userReport[0].TotalPosts) - require.Equal(t, 2, *userReport[0].DaysActive) + require.Equal(t, 7, *userReport[0].TotalPosts) + require.Equal(t, 7, *userReport[0].DaysActive) require.Equal(t, now.AddDate(0, 0, -3).UnixMilli(), *userReport[0].LastPostDate) - require.Equal(t, 2, *userReport[1].TotalPosts) - require.Equal(t, 2, *userReport[1].DaysActive) + require.Equal(t, 7, *userReport[1].TotalPosts) + require.Equal(t, 7, *userReport[1].DaysActive) require.Equal(t, now.AddDate(0, 0, -3).UnixMilli(), *userReport[1].LastPostDate) - require.Equal(t, 2, *userReport[2].TotalPosts) - require.Equal(t, 2, *userReport[2].DaysActive) + require.Equal(t, 7, *userReport[2].TotalPosts) + require.Equal(t, 7, *userReport[2].DaysActive) require.Equal(t, now.AddDate(0, 0, -3).UnixMilli(), *userReport[2].LastPostDate) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ @@ -6454,7 +6469,7 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { }, }) require.NoError(t, err) - require.Len(t, userReport, 3) + require.Len(t, userReport, 50) require.Equal(t, 1, *userReport[0].TotalPosts) require.Equal(t, 1, *userReport[0].DaysActive) @@ -6476,9 +6491,7 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { Role: "system", }) require.NoError(t, err) - require.Len(t, userReport, 1) - require.Equal(t, u2.Id, userReport[0].Id) - require.Equal(t, u2.Roles, "system") + require.Len(t, userReport, 10) }) t.Run("should filter on teams", func(t *testing.T) { @@ -6490,9 +6503,9 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { HasNoTeam: true, }) require.NoError(t, err) - require.Len(t, userReport, 2) - require.Equal(t, u1.Id, userReport[0].Id) - require.Equal(t, u2.Id, userReport[1].Id) + require.Len(t, userReport, 50) + require.Equal(t, users[2].Id, userReport[0].Id) + require.Equal(t, users[3].Id, userReport[1].Id) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ @@ -6502,8 +6515,9 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { Team: team.Id, }) require.NoError(t, err) - require.Len(t, userReport, 1) - require.Equal(t, u3.Id, userReport[0].Id) + require.Len(t, userReport, 2) + require.Equal(t, users[0].Id, userReport[0].Id) + require.Equal(t, users[1].Id, userReport[1].Id) }) t.Run("should filter on activation", func(t *testing.T) { @@ -6515,9 +6529,7 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { HideInactive: true, }) require.NoError(t, err) - require.Len(t, userReport, 2) - require.Equal(t, u1.Id, userReport[0].Id) - require.Equal(t, u2.Id, userReport[1].Id) + require.Len(t, userReport, 50) userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ @@ -6527,7 +6539,6 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { HideActive: true, }) require.NoError(t, err) - require.Len(t, userReport, 1) - require.Equal(t, u3.Id, userReport[0].Id) + require.Len(t, userReport, 15) }) }