MM 60222 - apply filter to export csv (#28212)

* MM-60222_apply filter to export csv * get the report exporting with the filters ready * add unit tests * cover one more file with some tests * style the confirm modal note * add translations * remove unnecessary print line * disable export button if there is no data to export * fix linter issues * fix linter errors --------- Co-authored-by: Mattermost Build <build@mattermost.com>
2024-10-02 12:23:39 +02:00
--- a/server/channels/api4/report.go
+++ b/server/channels/api4/report.go
@@ -28,6 +28,7 @@ func getUsersForReporting(c *Context, w http.ResponseWriter, r *http.Request) {

 	baseOptions := fillReportingBaseOptions(r.URL.Query())
 	options, err := fillUserReportOptions(r.URL.Query())
+
 	if err != nil {
 		c.Err = err
 		return
@@ -80,13 +81,21 @@ func startUsersBatchExport(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}

-	dateRange := r.URL.Query().Get("date_range")
+	baseOptions := fillReportingBaseOptions(r.URL.Query())
+	options, err := fillUserReportOptions(r.URL.Query())
+
+	if err != nil {
+		c.Err = err
+		return
+	}
+	options.ReportingBaseOptions = baseOptions
+	dateRange := options.ReportingBaseOptions.DateRange
 	if dateRange == "" {
 		dateRange = "all_time"
 	}

 	startAt, endAt := model.GetReportDateRange(dateRange, time.Now())
-	if err := c.App.StartUsersBatchExport(c.AppContext, dateRange, startAt, endAt); err != nil {
+	if err := c.App.StartUsersBatchExport(c.AppContext, options, startAt, endAt); err != nil {
 		c.Err = err
 		return
 	}
@@ -136,7 +145,6 @@ func fillUserReportOptions(values url.Values) (*model.UserReportOptions, *model.
 	}

 	return &model.UserReportOptions{
-
 		Team:         teamFilter,
 		Role:         values.Get("role_filter"),
 		HasNoTeam:    values.Get("has_no_team") == "true",
--- a/server/channels/api4/report_test.go
+++ b/server/channels/api4/report_test.go
@@ -0,0 +1,154 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package api4
+
+import (
+	"context"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/model"
+)
+
+func TestGetUsersForReporting(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+	client := th.Client
+
+	defaultRolePermissions := th.SaveDefaultRolePermissions()
+	defer th.RestoreDefaultRolePermissions(defaultRolePermissions)
+
+	t.Run("should return forbidden error when user lacks permission", func(t *testing.T) {
+		th.RemovePermissionFromRole(model.PermissionSysconsoleReadUserManagementUsers.Id, model.SystemUserRoleId)
+
+		_, resp, err := client.GetUsersForReporting(context.Background(), &model.UserReportOptions{})
+		require.Error(t, err)
+		CheckForbiddenStatus(t, resp)
+	})
+
+	t.Run("should return user reports when user has permission", func(t *testing.T) {
+		th.AddPermissionToRole(model.PermissionSysconsoleReadUserManagementUsers.Id, model.SystemUserRoleId)
+
+		options := &model.UserReportOptions{
+			ReportingBaseOptions: model.ReportingBaseOptions{
+				PageSize: 10,
+			},
+		}
+
+		userReports, resp, err := client.GetUsersForReporting(context.Background(), options)
+		require.NoError(t, err)
+		require.NotNil(t, userReports)
+		require.GreaterOrEqual(t, len(userReports), 1)
+		CheckOKStatus(t, resp)
+	})
+
+	t.Run("should return bad request on invalid parameters", func(t *testing.T) {
+		th.AddPermissionToRole(model.PermissionSysconsoleReadUserManagementUsers.Id, model.SystemUserRoleId)
+
+		options := &model.UserReportOptions{
+			Team: "invalid_team_id",
+		}
+
+		_, resp, err := client.GetUsersForReporting(context.Background(), options)
+		require.Error(t, err)
+		CheckBadRequestStatus(t, resp)
+	})
+}
+
+func TestFillReportingBaseOptions(t *testing.T) {
+	t.Run("default values", func(t *testing.T) {
+		values := url.Values{}
+
+		options := fillReportingBaseOptions(values)
+
+		require.Equal(t, "Username", options.SortColumn)
+		require.Equal(t, "next", options.Direction)
+		require.Equal(t, false, options.SortDesc)
+		require.Equal(t, 50, options.PageSize)
+		require.Equal(t, "", options.FromColumnValue)
+		require.Equal(t, "", options.FromId)
+		require.Equal(t, "", options.DateRange)
+	})
+
+	t.Run("custom values", func(t *testing.T) {
+		values := url.Values{}
+		values.Set("sort_column", "Email")
+		values.Set("direction", "prev")
+		values.Set("sort_direction", "desc")
+		values.Set("page_size", "25")
+		values.Set("from_column_value", "some_value")
+		values.Set("from_id", "some_id")
+		values.Set("date_range", "last_seven")
+
+		options := fillReportingBaseOptions(values)
+
+		require.Equal(t, "Email", options.SortColumn)
+		require.Equal(t, "prev", options.Direction)
+		require.Equal(t, true, options.SortDesc)
+		require.Equal(t, 25, options.PageSize)
+		require.Equal(t, "some_value", options.FromColumnValue)
+		require.Equal(t, "some_id", options.FromId)
+		require.Equal(t, "last_seven", options.DateRange)
+	})
+
+	t.Run("invalid page_size", func(t *testing.T) {
+		values := url.Values{}
+		values.Set("page_size", "an_very_invalid_number")
+
+		options := fillReportingBaseOptions(values)
+
+		require.Equal(t, 50, options.PageSize)
+	})
+
+	t.Run("invalid direction", func(t *testing.T) {
+		values := url.Values{}
+		values.Set("direction", "a_crazy_direction")
+
+		options := fillReportingBaseOptions(values)
+
+		require.Equal(t, "next", options.Direction)
+	})
+}
+
+func TestFillUserReportOptions(t *testing.T) {
+	validTeamID := model.NewId()
+
+	t.Run("default values", func(t *testing.T) {
+		values := url.Values{}
+		values.Set("team_filter", validTeamID)
+
+		options, _ := fillUserReportOptions(values)
+
+		expected := &model.UserReportOptions{
+			Team:         validTeamID,
+			Role:         "",
+			HasNoTeam:    false,
+			HideActive:   false,
+			HideInactive: false,
+			SearchTerm:   "",
+		}
+
+		require.Equal(t, expected, options)
+	})
+
+	t.Run("empty team_filter", func(t *testing.T) {
+		values := url.Values{}
+		values.Set("team_filter", "")
+
+		options, _ := fillUserReportOptions(values)
+
+		require.Equal(t, "", options.Team)
+	})
+
+	t.Run("valid team_filter", func(t *testing.T) {
+		values := url.Values{}
+		values.Set("team_filter", validTeamID)
+
+		options, _ := fillUserReportOptions(values)
+
+		require.Equal(t, validTeamID, options.Team)
+	})
+}
--- a/server/channels/app/app_iface.go
+++ b/server/channels/app/app_iface.go
@@ -1146,7 +1146,7 @@ type AppIface interface {
 	SlackImport(c request.CTX, fileData multipart.File, fileSize int64, teamID string) (*model.AppError, *bytes.Buffer)
 	SoftDeleteTeam(teamID string) *model.AppError
 	Srv() *Server
-	StartUsersBatchExport(rctx request.CTX, dateRange string, startAt int64, endAt int64) *model.AppError
+	StartUsersBatchExport(rctx request.CTX, ro *model.UserReportOptions, startAt int64, endAt int64) *model.AppError
 	SubmitInteractiveDialog(c request.CTX, request model.SubmitDialogRequest) (*model.SubmitDialogResponse, *model.AppError)
 	SwitchEmailToLdap(c request.CTX, email, password, code, ldapLoginId, ldapPassword string) (string, *model.AppError)
 	SwitchEmailToOAuth(c request.CTX, w http.ResponseWriter, r *http.Request, email, password, code, service string) (string, *model.AppError)
--- a/server/channels/app/opentracing/opentracing_layer.go
+++ b/server/channels/app/opentracing/opentracing_layer.go
@@ -17236,7 +17236,7 @@ func (a *OpenTracingAppLayer) SoftDeleteTeam(teamID string) *model.AppError {
 	return resultVar0
 }

-func (a *OpenTracingAppLayer) StartUsersBatchExport(rctx request.CTX, dateRange string, startAt int64, endAt int64) *model.AppError {
+func (a *OpenTracingAppLayer) StartUsersBatchExport(rctx request.CTX, ro *model.UserReportOptions, startAt int64, endAt int64) *model.AppError {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.StartUsersBatchExport")

@@ -17248,7 +17248,7 @@ func (a *OpenTracingAppLayer) StartUsersBatchExport(rctx request.CTX, dateRange
 	}()

 	defer span.Finish()
-	resultVar0 := a.app.StartUsersBatchExport(rctx, dateRange, startAt, endAt)
+	resultVar0 := a.app.StartUsersBatchExport(rctx, ro, startAt, endAt)

 	if resultVar0 != nil {
 		span.LogFields(spanlog.Error(resultVar0))
--- a/server/channels/app/report.go
+++ b/server/channels/app/report.go
@@ -198,41 +198,28 @@ func (a *App) GetUserCountForReport(filter *model.UserReportOptions) (*int64, *m
 	return &count, nil
 }

-func (a *App) StartUsersBatchExport(rctx request.CTX, dateRange string, startAt int64, endAt int64) *model.AppError {
+func (a *App) StartUsersBatchExport(rctx request.CTX, ro *model.UserReportOptions, startAt int64, endAt int64) *model.AppError {
 	if license := a.Srv().License(); license == nil || (license.SkuShortName != model.LicenseShortSkuProfessional && license.SkuShortName != model.LicenseShortSkuEnterprise) {
 		return model.NewAppError("StartUsersBatchExport", "app.report.start_users_batch_export.license_error", nil, "", http.StatusBadRequest)
 	}

 	options := map[string]string{
 		"requesting_user_id": rctx.Session().UserId,
-		"date_range":         dateRange,
+		"date_range":         ro.DateRange,
+		"role":               ro.Role,
+		"team":               ro.Team,
+		"hide_active":        strconv.FormatBool(ro.HideActive),
+		"hide_inactive":      strconv.FormatBool(ro.HideInactive),
 		"start_at":           strconv.FormatInt(startAt, 10),
 		"end_at":             strconv.FormatInt(endAt, 10),
 	}

-	// Check for existing job
-	// TODO: Maybe make this a reusable function?
-	pendingJobs, err := a.Srv().Jobs.GetJobsByTypeAndStatus(rctx, model.JobTypeExportUsersToCSV, model.JobStatusPending)
-	if err != nil {
+	// Check for existing jobs
+	if err := a.checkForExistingJobs(rctx, options, model.JobTypeExportUsersToCSV); err != nil {
 		return err
 	}
-	for _, job := range pendingJobs {
-		if job.Data["date_range"] == options["date_range"] && job.Data["requesting_user_id"] == rctx.Session().UserId {
-			return model.NewAppError("StartUsersBatchExport", "app.report.start_users_batch_export.job_exists", nil, "", http.StatusBadRequest)
-		}
-	}

-	inProgressJobs, err := a.Srv().Jobs.GetJobsByTypeAndStatus(rctx, model.JobTypeExportUsersToCSV, model.JobStatusInProgress)
-	if err != nil {
-		return err
-	}
-	for _, job := range inProgressJobs {
-		if job.Data["date_range"] == options["date_range"] && job.Data["requesting_user_id"] == rctx.Session().UserId {
-			return model.NewAppError("StartUsersBatchExport", "app.report.start_users_batch_export.job_exists", nil, "", http.StatusBadRequest)
-		}
-	}
-
-	_, err = a.Srv().Jobs.CreateJob(rctx, model.JobTypeExportUsersToCSV, options)
+	_, err := a.Srv().Jobs.CreateJob(rctx, model.JobTypeExportUsersToCSV, options)
 	if err != nil {
 		return err
 	}
@@ -258,7 +245,7 @@ func (a *App) StartUsersBatchExport(rctx request.CTX, dateRange string, startAt
 		T := i18n.GetUserTranslations(user.Locale)
 		post := &model.Post{
 			ChannelId: channel.Id,
-			Message:   T("app.report.start_users_batch_export.started_export", map[string]string{"DateRange": getTranslatedDateRange(dateRange)}),
+			Message:   T("app.report.start_users_batch_export.started_export", map[string]string{"DateRange": getTranslatedDateRange(ro.DateRange)}),
 			Type:      model.PostTypeDefault,
 			UserId:    systemBot.UserId,
 		}
@@ -271,6 +258,41 @@ func (a *App) StartUsersBatchExport(rctx request.CTX, dateRange string, startAt
 	return nil
 }

+// Helper function to check for existing or pending jobs
+func (a *App) checkForExistingJobs(rctx request.CTX, options map[string]string, jobType string) *model.AppError {
+	checkJobExists := func(jobs []*model.Job, options map[string]string) bool {
+		for _, job := range jobs {
+			if job.Data["date_range"] == options["date_range"] &&
+				job.Data["requesting_user_id"] == options["requesting_user_id"] &&
+				job.Data["role"] == options["role"] &&
+				job.Data["team"] == options["team"] &&
+				job.Data["hide_active"] == options["hide_active"] &&
+				job.Data["hide_inactive"] == options["hide_inactive"] {
+				return true
+			}
+		}
+		return false
+	}
+
+	pendingJobs, err := a.Srv().Jobs.GetJobsByTypeAndStatus(rctx, jobType, model.JobStatusPending)
+	if err != nil {
+		return err
+	}
+	if checkJobExists(pendingJobs, options) {
+		return model.NewAppError("StartUsersBatchExport", "app.report.start_users_batch_export.job_exists", nil, "", http.StatusBadRequest)
+	}
+
+	inProgressJobs, err := a.Srv().Jobs.GetJobsByTypeAndStatus(rctx, jobType, model.JobStatusInProgress)
+	if err != nil {
+		return err
+	}
+	if checkJobExists(inProgressJobs, options) {
+		return model.NewAppError("StartUsersBatchExport", "app.report.start_users_batch_export.job_exists", nil, "", http.StatusBadRequest)
+	}
+
+	return nil
+}
+
 func getTranslatedDateRange(dateRange string) string {
 	switch dateRange {
 	case model.ReportDurationLast30Days:
--- a/server/channels/app/report_test.go
+++ b/server/channels/app/report_test.go
@@ -10,6 +10,7 @@ import (
 	"time"

 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/request"
 	"github.com/stretchr/testify/require"
 )

@@ -107,3 +108,100 @@ some-other-other-name,600,2022-01-01
 		require.NotNil(t, err)
 	})
 }
+
+func TestCheckForExistingJobs(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	t.Run("should return error if job with same options exists in pending jobs", func(t *testing.T) {
+		app := th.App
+		rctx := request.TestContext(t)
+		options := map[string]string{
+			"date_range":         "last_30_days",
+			"requesting_user_id": th.BasicUser.Id,
+			"role":               "user",
+			"team":               "",
+			"hide_active":        "false",
+			"hide_inactive":      "false",
+		}
+
+		jobType := model.JobTypeExportUsersToCSV
+
+		// Create a pending job with same options
+		job, err := app.Srv().Jobs.CreateJob(rctx, jobType, options)
+		defer func() {
+			_ = app.Srv().Jobs.RequestCancellation(rctx, job.Id)
+		}()
+		require.Nil(t, err)
+		require.NotNil(t, job)
+
+		// checkForExistingJobs
+		appErr := app.checkForExistingJobs(rctx, options, jobType)
+		require.NotNil(t, appErr)
+		require.Equal(t, "app.report.start_users_batch_export.job_exists", appErr.Id)
+	})
+
+	t.Run("should return error if job with same options exists in in-progress jobs", func(t *testing.T) {
+		app := th.App
+		rctx := request.TestContext(t)
+		options := map[string]string{
+			"date_range":         "last_30_days",
+			"requesting_user_id": th.BasicUser.Id,
+			"role":               "user",
+			"team":               "",
+			"hide_active":        "false",
+			"hide_inactive":      "false",
+		}
+
+		jobType := model.JobTypeExportUsersToCSV
+
+		// Create an in-progress job with same options
+		job, err := app.Srv().Jobs.CreateJob(rctx, jobType, options)
+		defer func() {
+			_ = app.Srv().Jobs.RequestCancellation(rctx, job.Id)
+		}()
+		require.Nil(t, err)
+		require.NotNil(t, job)
+
+		// Manually set job status to in-progress
+		err = app.Srv().Jobs.SetJobProgress(job, 60)
+		require.Nil(t, err)
+
+		// Call checkForExistingJobs
+		appErr := app.checkForExistingJobs(rctx, options, jobType)
+		require.NotNil(t, appErr)
+		require.Equal(t, "app.report.start_users_batch_export.job_exists", appErr.Id)
+	})
+
+	t.Run("should not return error if existing jobs have different options", func(t *testing.T) {
+		app := th.App
+		rctx := request.TestContext(t)
+		options := map[string]string{
+			"date_range":         "last_30_days",
+			"requesting_user_id": th.BasicUser.Id,
+			"role":               "user",
+			"team":               "",
+			"hide_active":        "false",
+			"hide_inactive":      "false",
+		}
+
+		jobType := model.JobTypeExportUsersToCSV
+
+		differentOptions := map[string]string{
+			"date_range":         "all_time",
+			"requesting_user_id": th.BasicUser2.Id,
+			"role":               "admin",
+			"team":               "",
+			"hide_active":        "false",
+			"hide_inactive":      "false",
+		}
+
+		job, err := app.Srv().Jobs.CreateJob(rctx, jobType, differentOptions)
+		require.Nil(t, err)
+		require.NotNil(t, job)
+
+		// Call checkForExistingJobs
+		appErr := app.checkForExistingJobs(rctx, options, jobType)
+		require.Nil(t, appErr)
+	})
+}
--- a/server/channels/jobs/export_users_to_csv/export_users_to_csv.go
+++ b/server/channels/jobs/export_users_to_csv/export_users_to_csv.go
@@ -4,6 +4,7 @@
 package export_users_to_csv

 import (
+	"fmt"
 	"strconv"
 	"time"

@@ -60,6 +61,22 @@ func parseJobMetadata(data model.StringMap) (*model.UserReportOptions, error) {
 		return nil, err
 	}

+	hideInactive := false
+	if val, ok := data["hide_inactive"]; ok && val != "" {
+		hideInactive, err = strconv.ParseBool(val)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse hide_inactive: %w", err)
+		}
+	}
+
+	hideActive := false
+	if val, ok := data["hide_active"]; ok && val != "" {
+		hideActive, err = strconv.ParseBool(val)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse hide_active: %w", err)
+		}
+	}
+
 	options := model.UserReportOptions{
 		ReportingBaseOptions: model.ReportingBaseOptions{
 			SortColumn:      "Username",
@@ -69,6 +86,10 @@ func parseJobMetadata(data model.StringMap) (*model.UserReportOptions, error) {
 			StartAt:         startAt,
 			EndAt:           endAt,
 		},
+		HideInactive: hideInactive,
+		HideActive:   hideActive,
+		Role:         data["role"],
+		Team:         data["team"],
 	}

 	return &options, nil