From bbe432b3b0050787ec8685711e6acef37ec2103b Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Thu, 18 Jan 2024 14:06:30 -0500 Subject: [PATCH] [MM-56398] Fix date range query for user reporting (#25960) * [MM-56398] Fix date range query for user reporting * Missing debug stuff * Fix tests --- server/channels/api4/report.go | 10 +++--- server/channels/store/sqlstore/user_store.go | 34 +++++++++---------- server/channels/store/storetest/user_store.go | 14 ++++++++ 3 files changed, 35 insertions(+), 23 deletions(-) diff --git a/server/channels/api4/report.go b/server/channels/api4/report.go index f226ee05a9..5cf6898190 100644 --- a/server/channels/api4/report.go +++ b/server/channels/api4/report.go @@ -89,7 +89,7 @@ func fillReportingBaseOptions(values url.Values) model.ReportingBaseOptions { pageSize = int(pageSizeStr) } - return model.ReportingBaseOptions{ + options := model.ReportingBaseOptions{ Direction: direction, SortColumn: sortColumn, SortDesc: values.Get("sort_direction") == "desc", @@ -98,6 +98,8 @@ func fillReportingBaseOptions(values url.Values) model.ReportingBaseOptions { FromId: values.Get("from_id"), DateRange: values.Get("date_range"), } + options.PopulateDateRange(time.Now()) + return options } func fillUserReportOptions(values url.Values) (*model.UserReportOptions, *model.AppError) { @@ -112,7 +114,7 @@ func fillUserReportOptions(values url.Values) (*model.UserReportOptions, *model. return nil, model.NewAppError("getUsersForReporting", "api.getUsersForReporting.invalid_active_filter", nil, "", http.StatusBadRequest) } - options := &model.UserReportOptions{ + return &model.UserReportOptions{ Team: teamFilter, Role: values.Get("role_filter"), @@ -120,7 +122,5 @@ func fillUserReportOptions(values url.Values) (*model.UserReportOptions, *model. HideActive: hideActive, HideInactive: hideInactive, SearchTerm: values.Get("search_term"), - } - options.PopulateDateRange(time.Now()) - return options, nil + }, nil } diff --git a/server/channels/store/sqlstore/user_store.go b/server/channels/store/sqlstore/user_store.go index efa91f4ba5..70c0d59b4a 100644 --- a/server/channels/store/sqlstore/user_store.go +++ b/server/channels/store/sqlstore/user_store.go @@ -2377,35 +2377,33 @@ func (us SqlUserStore) GetUserReport(filter *model.UserReportOptions) ([]*model. } if isPostgres { - query = query.LeftJoin("PostStats ps ON ps.UserId = u.Id") + joinSql := sq.And{} if filter.StartAt > 0 { startDate := time.UnixMilli(filter.StartAt) - query = query.Where(sq.Or{ - sq.Expr("ps.UserId IS NULL"), - sq.GtOrEq{"ps.Day": startDate.Format("2006-01-02")}, - }) + joinSql = append(joinSql, sq.GtOrEq{"ps.Day": startDate.Format("2006-01-02")}) } if filter.EndAt > 0 { endDate := time.UnixMilli(filter.EndAt) - query = query.Where(sq.Or{ - sq.Expr("ps.UserId IS NULL"), - sq.Lt{"ps.Day": endDate.Format("2006-01-02")}, - }) + joinSql = append(joinSql, sq.Lt{"ps.Day": endDate.Format("2006-01-02")}) } + sql, args, err := joinSql.ToSql() + if err != nil { + return nil, err + } + query = query.LeftJoin("PostStats ps ON ps.UserId = u.Id AND "+sql, args...) } else { - query = query.LeftJoin("Posts p on p.UserId = u.Id") + joinSql := sq.And{} if filter.StartAt > 0 { - query = query.Where(sq.Or{ - sq.Expr("p.UserId IS NULL"), - sq.GtOrEq{"p.CreateAt": filter.StartAt}, - }) + joinSql = append(joinSql, sq.GtOrEq{"p.CreateAt": filter.StartAt}) } if filter.EndAt > 0 { - query = query.Where(sq.Or{ - sq.Expr("p.UserId IS NULL"), - sq.Lt{"p.CreateAt": filter.EndAt}, - }) + joinSql = append(joinSql, sq.Lt{"p.CreateAt": filter.EndAt}) } + sql, args, err := joinSql.ToSql() + if err != nil { + return nil, err + } + query = query.LeftJoin("Posts p ON p.UserId = u.Id AND "+sql, args...) } query = applyUserReportFilter(query, filter, isPostgres) diff --git a/server/channels/store/storetest/user_store.go b/server/channels/store/storetest/user_store.go index 02f1cdbcf4..a08636b8d4 100644 --- a/server/channels/store/storetest/user_store.go +++ b/server/channels/store/storetest/user_store.go @@ -6400,6 +6400,20 @@ func testGetUserReport(t *testing.T, rctx request.CTX, ss store.Store) { require.Equal(t, users[60].Username, userReport[0].Username) }) + t.Run("should return all users regardless of date range", func(t *testing.T) { + userReport, err := ss.User().GetUserReport(&model.UserReportOptions{ + ReportingBaseOptions: model.ReportingBaseOptions{ + SortColumn: "Username", + PageSize: 50, + StartAt: now.AddDate(1000, 0, 0).UnixMilli(), + EndAt: now.AddDate(1000, 0, 0).UnixMilli(), + }, + }) + + require.NoError(t, err) + require.Len(t, userReport, 50) + }) + t.Run("should return accurate post stats for various date ranges", func(t *testing.T) { userReport, err := ss.User().GetUserReport(&model.UserReportOptions{ ReportingBaseOptions: model.ReportingBaseOptions{