IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fixed user reporting pagination (#25780)

Browse Source 
* Fixed pagination and sorting for Postgres

* Updated tests
This commit is contained in:
Harshil Sharma
2024-01-02 21:11:00 +05:30
committed by GitHub
parent 39b2ecf2dd
commit 9016e30044
2 changed files with 147 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
server/channels/store/sqlstore/user_store.go
View File

@@ -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")
}

187
server/channels/store/storetest/user_store.go
View File

@@ -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)
})
}