From 5c061a6f7582fee731c04f6ab466c5c2efd6e843 Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Sun, 17 Dec 2023 19:26:06 -0500 Subject: [PATCH] [MM-56206] Allow for proper paging and sorting (#25726) Co-authored-by: Mattermost Build --- api/v4/source/reports.yaml | 14 +++-- server/channels/api4/report.go | 18 ++++-- server/channels/store/sqlstore/user_store.go | 25 +++++--- server/channels/store/storetest/user_store.go | 59 +++++++++++++++++-- server/public/model/client4.go | 11 ++-- server/public/model/report.go | 17 +++--- webapp/platform/types/src/client4.ts | 5 +- 7 files changed, 114 insertions(+), 35 deletions(-) diff --git a/api/v4/source/reports.yaml b/api/v4/source/reports.yaml index 83e335155d..a13a9a57c3 100644 --- a/api/v4/source/reports.yaml +++ b/api/v4/source/reports.yaml @@ -20,6 +20,12 @@ schema: type: string default: 'Username' + - name: direction + in: query + description: The direction in which to accept paging values from. Will return values ahead of the cursor if "up", and below the cursor if "down". Default is "down". + schema: + type: string + default: 'down' - name: sort_direction in: query description: The sorting direction. Must be one of ("asc", "desc"). Will default to 'asc' if not specified or the input is invalid. @@ -34,14 +40,14 @@ default: 50 minimum: 1 maximum: 100 - - name: last_column_value + - name: from_column_value in: query - description: The value of the sorted column belonging to the last user returned in the page. Should be blank for the first page asked for. + description: The value of the sorted column corresponding to the cursor to read from. Should be blank for the first page asked for. schema: type: string - - name: last_id + - name: from_id in: query - description: The value of the user id belonging to the last user returned in the page. Should be blank for the first page asked for. + description: The value of the user id corresponding to the cursor to read from. Should be blank for the first page asked for. schema: type: string - name: date_range diff --git a/server/channels/api4/report.go b/server/channels/api4/report.go index b824457560..9c532def59 100644 --- a/server/channels/api4/report.go +++ b/server/channels/api4/report.go @@ -28,6 +28,11 @@ func getUsersForReporting(c *Context, w http.ResponseWriter, r *http.Request) { sortColumn = r.URL.Query().Get("sort_column") } + direction := "down" + if r.URL.Query().Get("direction") == "up" { + direction = "up" + } + pageSize := 50 if pageSizeStr, err := strconv.ParseInt(r.URL.Query().Get("page_size"), 10, 64); err == nil { pageSize = int(pageSizeStr) @@ -48,14 +53,15 @@ func getUsersForReporting(c *Context, w http.ResponseWriter, r *http.Request) { options := &model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ - SortColumn: sortColumn, - SortDesc: r.URL.Query().Get("sort_direction") == "desc", - PageSize: pageSize, - LastSortColumnValue: r.URL.Query().Get("last_column_value"), - DateRange: r.URL.Query().Get("date_range"), + Direction: direction, + SortColumn: sortColumn, + SortDesc: r.URL.Query().Get("sort_direction") == "desc", + PageSize: pageSize, + FromColumnValue: r.URL.Query().Get("from_column_value"), + FromId: r.URL.Query().Get("from_id"), + DateRange: r.URL.Query().Get("date_range"), }, Team: teamFilter, - LastUserId: r.URL.Query().Get("last_id"), Role: r.URL.Query().Get("role_filter"), HasNoTeam: r.URL.Query().Get("has_no_team") == "true", HideActive: hideActive, diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index a5fedc08d6..15d1790c6d 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -2297,17 +2297,28 @@ func (us SqlUserStore) GetUserReport(filter *model.UserReportOptions) ([]*model. Select(selectColumns...). From("Users u"). LeftJoin("Status s ON s.UserId = u.Id"). - Where(sq.Or{ - sq.Gt{filter.SortColumn: filter.LastSortColumnValue}, - sq.And{ - sq.Eq{filter.SortColumn: filter.LastSortColumnValue}, - sq.Gt{"u.Id": filter.LastUserId}, - }, - }). Where(sq.Expr("u.Id NOT IN (SELECT UserId FROM Bots)")). GroupBy("u.Id"). OrderBy(sortColumnValue, "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}, + }, + }) + } + if filter.PageSize > 0 { query = query.Limit(uint64(filter.PageSize)) } diff --git a/server/channels/store/storetest/user_store.go b/server/channels/store/storetest/user_store.go index 7b64b1e450..e25d6c5209 100644 --- a/server/channels/store/storetest/user_store.go +++ b/server/channels/store/storetest/user_store.go @@ -6321,11 +6321,62 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { t.Run("should return correct paging", func(t *testing.T) { userReport, err := ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{ - SortColumn: "Username", - PageSize: 50, - LastSortColumnValue: u2.Username, + SortColumn: "Username", + Direction: "down", + PageSize: 50, + FromColumnValue: u2.Username, + FromId: u2.Id, + }, + }) + require.NoError(t, err) + require.NotNil(t, userReport) + require.Equal(t, 1, len(userReport)) + + require.NotNil(t, userReport[0]) + require.Equal(t, u3.Username, userReport[0].Username) + + userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ + ReportingBaseOptions: model.ReportingBaseOptions{ + SortColumn: "Username", + SortDesc: true, + Direction: "down", + PageSize: 50, + FromColumnValue: u2.Username, + FromId: u2.Id, + }, + }) + require.NoError(t, err) + require.NotNil(t, userReport) + require.Equal(t, 1, len(userReport)) + + require.NotNil(t, userReport[0]) + require.Equal(t, u1.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, + }, + }) + require.NoError(t, err) + require.NotNil(t, userReport) + require.Equal(t, 1, len(userReport)) + + require.NotNil(t, userReport[0]) + require.Equal(t, u1.Username, userReport[0].Username) + + userReport, err = ss.User().GetUserReport(&model.UserReportOptions{ + ReportingBaseOptions: model.ReportingBaseOptions{ + SortColumn: "Username", + SortDesc: true, + Direction: "up", + PageSize: 50, + FromColumnValue: u2.Username, + FromId: u2.Id, }, - LastUserId: u2.Id, }) require.NoError(t, err) require.NotNil(t, userReport) diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 17eea08cf0..ba23b9b07f 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -1924,6 +1924,9 @@ func (c *Client4) EnableUserAccessToken(ctx context.Context, tokenId string) (*R func (c *Client4) GetUsersForReporting(ctx context.Context, options *UserReportOptions) ([]*UserReport, *Response, error) { values := url.Values{} + if options.Direction != "" { + values.Set("direction", options.Direction) + } if options.SortColumn != "" { values.Set("sort_column", options.SortColumn) } @@ -1942,11 +1945,11 @@ func (c *Client4) GetUsersForReporting(ctx context.Context, options *UserReportO if options.SortDesc { values.Set("sort_direction", "desc") } - if options.LastSortColumnValue != "" { - values.Set("last_column_value", options.LastSortColumnValue) + if options.FromColumnValue != "" { + values.Set("from_column_value", options.FromColumnValue) } - if options.LastUserId != "" { - values.Set("last_id", options.LastUserId) + if options.FromId != "" { + values.Set("from_id", options.FromId) } if options.Role != "" { values.Set("role_filter", options.Role) diff --git a/server/public/model/report.go b/server/public/model/report.go index 687912da05..f65bbf597b 100644 --- a/server/public/model/report.go +++ b/server/public/model/report.go @@ -23,13 +23,15 @@ var ( ) type ReportingBaseOptions struct { - SortDesc bool - PageSize int - SortColumn string - LastSortColumnValue string - DateRange string - StartAt int64 - EndAt int64 + SortDesc bool + Direction string // Accepts only "up" or "down" + PageSize int + SortColumn string + FromColumnValue string + FromId string + DateRange string + StartAt int64 + EndAt int64 } func (options *ReportingBaseOptions) PopulateDateRange(now time.Time) { @@ -75,7 +77,6 @@ type UserReport struct { type UserReportOptions struct { ReportingBaseOptions - LastUserId string Role string Team string HasNoTeam bool diff --git a/webapp/platform/types/src/client4.ts b/webapp/platform/types/src/client4.ts index a61117bf76..78e362a9da 100644 --- a/webapp/platform/types/src/client4.ts +++ b/webapp/platform/types/src/client4.ts @@ -48,9 +48,10 @@ export type UserReportOptions = { sort_column: 'CreateAt' | 'Username' | 'FirstName' | 'LastName' | 'Nickname' | 'Email', page_size: number, sort_direction?: 'asc' | 'desc', + direction?: 'up' | 'down', date_range?: ReportDuration, - last_column_value?: string, - last_id?: string, + from_column_value?: string, + from_id?: string, role_filter?: string, has_no_team?: boolean, team_filter?: string,