From 0ee2a3ca3523d05a3797a636f7c5ae7ca17c1dd3 Mon Sep 17 00:00:00 2001 From: Allan Guwatudde Date: Mon, 15 Nov 2021 18:42:15 +0300 Subject: [PATCH] [MM-39060] - A/B Test: Reminder to Join Workspace email (#18894) * [MM-39060] - A/B Test: Reminder to Join Workspace email * Fix error and add error handling * feedback impl * run make i18n-extract * improvements * make i18n-extract * setup ability to do telemetry on reminder invite emails * improvement * add telemetry Co-authored-by: Mattermod --- api4/team.go | 9 +- api4/team_local.go | 4 +- app/app_iface.go | 2 +- app/email/email.go | 13 ++- app/email/email_test.go | 2 +- app/email_test.go | 2 +- app/opentracing/opentracing_layer.go | 4 +- app/team.go | 11 +- i18n/en.json | 4 + jobs/resend_invitation_email/worker.go | 154 +++++++++++++++++++------ model/feature_flags.go | 5 + model/team_member.go | 4 + 12 files changed, 166 insertions(+), 48 deletions(-) diff --git a/api4/team.go b/api4/team.go index c5fd6b003c..3e3a5e99a6 100644 --- a/api4/team.go +++ b/api4/team.go @@ -1322,16 +1322,21 @@ func inviteUsersToTeam(c *Context, w http.ResponseWriter, r *http.Request) { } // we then manually schedule the job - _, e := c.App.Srv().Jobs.CreateJob(model.JobTypeResendInvitationEmail, jobData) + j, e := c.App.Srv().Jobs.CreateJob(model.JobTypeResendInvitationEmail, jobData) if e != nil { c.Err = model.NewAppError("Api4.inviteUsersToTeam", e.Id, nil, e.Error(), e.StatusCode) return } + sysVar := &model.System{Name: j.Id, Value: "0"} + if sysValErr := c.App.Srv().Store.System().SaveOrUpdate(sysVar); sysValErr != nil { + mlog.Warn("Error while saving system value", mlog.Err(sysValErr)) + } + var invitesWithError []*model.EmailInviteWithError var err *model.AppError if emailList != nil { - invitesWithError, err = c.App.InviteNewUsersToTeamGracefully(emailList, c.Params.TeamId, c.AppContext.Session().UserId) + invitesWithError, err = c.App.InviteNewUsersToTeamGracefully(emailList, c.Params.TeamId, c.AppContext.Session().UserId, "") } if len(invitesOverLimit) > 0 { diff --git a/api4/team_local.go b/api4/team_local.go index c0e923dbb4..d15bdbe4ab 100644 --- a/api4/team_local.go +++ b/api4/team_local.go @@ -128,7 +128,7 @@ func localInviteUsersToTeam(c *Context, w http.ResponseWriter, r *http.Request) } auditRec.AddMeta("errors", errList) if len(goodEmails) > 0 { - err := c.App.Srv().EmailService.SendInviteEmails(team, "Administrator", "mmctl "+model.NewId(), goodEmails, *c.App.Config().ServiceSettings.SiteURL) + err := c.App.Srv().EmailService.SendInviteEmails(team, "Administrator", "mmctl "+model.NewId(), goodEmails, *c.App.Config().ServiceSettings.SiteURL, nil) if err != nil { switch { case errors.Is(err, email.NoRateLimiterError): @@ -161,7 +161,7 @@ func localInviteUsersToTeam(c *Context, w http.ResponseWriter, r *http.Request) c.Err = model.NewAppError("localInviteUsersToTeam", "api.team.invite_members.invalid_email.app_error", map[string]interface{}{"Addresses": s}, "", http.StatusBadRequest) return } - err := c.App.Srv().EmailService.SendInviteEmails(team, "Administrator", "mmctl "+model.NewId(), emailList, *c.App.Config().ServiceSettings.SiteURL) + err := c.App.Srv().EmailService.SendInviteEmails(team, "Administrator", "mmctl "+model.NewId(), emailList, *c.App.Config().ServiceSettings.SiteURL, nil) if err != nil { switch { case errors.Is(err, email.NoRateLimiterError): diff --git a/app/app_iface.go b/app/app_iface.go index 1419a85c01..929a7d3f85 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -810,7 +810,7 @@ type AppIface interface { InviteGuestsToChannels(teamID string, guestsInvite *model.GuestsInvite, senderId string) *model.AppError InviteGuestsToChannelsGracefully(teamID string, guestsInvite *model.GuestsInvite, senderId string) ([]*model.EmailInviteWithError, *model.AppError) InviteNewUsersToTeam(emailList []string, teamID, senderId string) *model.AppError - InviteNewUsersToTeamGracefully(emailList []string, teamID, senderId string) ([]*model.EmailInviteWithError, *model.AppError) + InviteNewUsersToTeamGracefully(emailList []string, teamID, senderId string, reminderInterval string) ([]*model.EmailInviteWithError, *model.AppError) IsCRTEnabledForUser(userID string) bool IsFirstUserAccount() bool IsLeader() bool diff --git a/app/email/email.go b/app/email/email.go index 501c90f930..5ddd208070 100644 --- a/app/email/email.go +++ b/app/email/email.go @@ -426,7 +426,7 @@ func (es *Service) SendMfaChangeEmail(email string, activated bool, locale, site return nil } -func (es *Service) SendInviteEmails(team *model.Team, senderName string, senderUserId string, invites []string, siteURL string) error { +func (es *Service) SendInviteEmails(team *model.Team, senderName string, senderUserId string, invites []string, siteURL string, reminderData *model.TeamInviteReminderData) error { if es.PerHourEmailRateLimiter == nil { return NoRateLimiterError } @@ -450,7 +450,6 @@ func (es *Service) SendInviteEmails(team *model.Team, senderName string, senderU data := es.NewEmailTemplateData("") data.Props["SiteURL"] = siteURL - data.Props["Title"] = i18n.T("api.templates.invite_body.title", map[string]interface{}{"SenderName": senderName, "TeamDisplayName": team.DisplayName}) data.Props["SubTitle"] = i18n.T("api.templates.invite_body.subTitle") data.Props["Button"] = i18n.T("api.templates.invite_body.button") data.Props["SenderName"] = senderName @@ -467,6 +466,16 @@ func (es *Service) SendInviteEmails(team *model.Team, senderName string, senderU tokenProps["email"] = invite tokenProps["display_name"] = team.DisplayName tokenProps["name"] = team.Name + + title := i18n.T("api.templates.invite_body.title", map[string]interface{}{"SenderName": senderName, "TeamDisplayName": team.DisplayName}) + if reminderData != nil { + reminder := i18n.T("api.templates.invite_body.title.reminder") + title = fmt.Sprintf("%s: %s", reminder, title) + tokenProps["reminder_interval"] = reminderData.Interval + } + + data.Props["Title"] = title + tokenData := model.MapToJSON(tokenProps) if err := es.store.Token().Save(token); err != nil { diff --git a/app/email/email_test.go b/app/email/email_test.go index d7d109065f..6b922f1a70 100644 --- a/app/email/email_test.go +++ b/app/email/email_test.go @@ -70,7 +70,7 @@ func TestSendInviteEmails(t *testing.T) { t.Run("SendInviteEmails", func(t *testing.T) { mail.DeleteMailBox(emailTo) - err := th.service.SendInviteEmails(th.BasicTeam, "test-user", th.BasicUser.Id, []string{emailTo}, "http://testserver") + err := th.service.SendInviteEmails(th.BasicTeam, "test-user", th.BasicUser.Id, []string{emailTo}, "http://testserver", nil) require.NoError(t, err) verifyMailbox(t) diff --git a/app/email_test.go b/app/email_test.go index 7e2abb5a24..ad3ab38426 100644 --- a/app/email_test.go +++ b/app/email_test.go @@ -34,7 +34,7 @@ func TestSendInviteEmailRateLimits(t *testing.T) { assert.Equal(t, "app.email.rate_limit_exceeded.app_error", err.Id) assert.Equal(t, http.StatusRequestEntityTooLarge, err.StatusCode) - _, err = th.App.InviteNewUsersToTeamGracefully(emailList, th.BasicTeam.Id, th.BasicUser.Id) + _, err = th.App.InviteNewUsersToTeamGracefully(emailList, th.BasicTeam.Id, th.BasicUser.Id, "") require.NotNil(t, err) assert.Equal(t, "app.email.rate_limit_exceeded.app_error", err.Id) assert.Equal(t, http.StatusRequestEntityTooLarge, err.StatusCode) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index c6bb954251..e68daa0817 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -10765,7 +10765,7 @@ func (a *OpenTracingAppLayer) InviteNewUsersToTeam(emailList []string, teamID st return resultVar0 } -func (a *OpenTracingAppLayer) InviteNewUsersToTeamGracefully(emailList []string, teamID string, senderId string) ([]*model.EmailInviteWithError, *model.AppError) { +func (a *OpenTracingAppLayer) InviteNewUsersToTeamGracefully(emailList []string, teamID string, senderId string, reminderInterval string) ([]*model.EmailInviteWithError, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.InviteNewUsersToTeamGracefully") @@ -10777,7 +10777,7 @@ func (a *OpenTracingAppLayer) InviteNewUsersToTeamGracefully(emailList []string, }() defer span.Finish() - resultVar0, resultVar1 := a.app.InviteNewUsersToTeamGracefully(emailList, teamID, senderId) + resultVar0, resultVar1 := a.app.InviteNewUsersToTeamGracefully(emailList, teamID, senderId, reminderInterval) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/app/team.go b/app/team.go index 7dea4f9586..3eed761f2a 100644 --- a/app/team.go +++ b/app/team.go @@ -1286,7 +1286,7 @@ func (a *App) GetErrorListForEmailsOverLimit(emailList []string, cloudUserLimit return emailList, invitesNotSent, nil } -func (a *App) InviteNewUsersToTeamGracefully(emailList []string, teamID, senderId string) ([]*model.EmailInviteWithError, *model.AppError) { +func (a *App) InviteNewUsersToTeamGracefully(emailList []string, teamID, senderId string, reminderInterval string) ([]*model.EmailInviteWithError, *model.AppError) { if !*a.Config().ServiceSettings.EnableEmailInvitations { return nil, model.NewAppError("InviteNewUsersToTeam", "api.team.invite_members.disabled.app_error", nil, "", http.StatusNotImplemented) } @@ -1315,9 +1315,14 @@ func (a *App) InviteNewUsersToTeamGracefully(emailList []string, teamID, senderI inviteListWithErrors = append(inviteListWithErrors, invite) } + var reminderData *model.TeamInviteReminderData + if reminderInterval != "" { + reminderData = &model.TeamInviteReminderData{Interval: reminderInterval} + } + if len(goodEmails) > 0 { nameFormat := *a.Config().TeamSettings.TeammateNameDisplay - eErr := a.Srv().EmailService.SendInviteEmails(team, user.GetDisplayName(nameFormat), user.Id, goodEmails, a.GetSiteURL()) + eErr := a.Srv().EmailService.SendInviteEmails(team, user.GetDisplayName(nameFormat), user.Id, goodEmails, a.GetSiteURL(), reminderData) if eErr != nil { switch { case errors.Is(eErr, email.NoRateLimiterError): @@ -1472,7 +1477,7 @@ func (a *App) InviteNewUsersToTeam(emailList []string, teamID, senderId string) } nameFormat := *a.Config().TeamSettings.TeammateNameDisplay - eErr := a.Srv().EmailService.SendInviteEmails(team, user.GetDisplayName(nameFormat), user.Id, emailList, a.GetSiteURL()) + eErr := a.Srv().EmailService.SendInviteEmails(team, user.GetDisplayName(nameFormat), user.Id, emailList, a.GetSiteURL(), nil) if eErr != nil { switch { case errors.Is(eErr, email.NoRateLimiterError): diff --git a/i18n/en.json b/i18n/en.json index 0644d5a254..d4f82286fc 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3267,6 +3267,10 @@ "id": "api.templates.invite_body.title", "translation": "{{ .SenderName }} invited you to join the {{ .TeamDisplayName }} team." }, + { + "id": "api.templates.invite_body.title.reminder", + "translation": "Reminder" + }, { "id": "api.templates.invite_body_footer.info", "translation": "Mattermost is a flexible, open source messaging platform that enables secure team collaboration." diff --git a/jobs/resend_invitation_email/worker.go b/jobs/resend_invitation_email/worker.go index bcb5092e07..457eed8c85 100644 --- a/jobs/resend_invitation_email/worker.go +++ b/jobs/resend_invitation_email/worker.go @@ -11,9 +11,12 @@ import ( "github.com/mattermost/mattermost-server/v6/app" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/shared/mlog" + "github.com/mattermost/mattermost-server/v6/store" ) const TwentyFourHoursInMillis int64 = 86400000 +const FourtyEightHoursInMillis int64 = 172800000 +const SeventyTwoHoursInMillis int64 = 259200000 type ResendInvitationEmailWorker struct { name string @@ -64,6 +67,77 @@ func (rseworker *ResendInvitationEmailWorker) JobChannel() chan<- model.Job { return rseworker.jobs } +func (rseworker *ResendInvitationEmailWorker) DoJob(job *model.Job) { + resendInviteEmailIntervalFlag := rseworker.App.Config().FeatureFlags.ResendInviteEmailInterval + + switch resendInviteEmailIntervalFlag { + case "48": + rseworker.DoJob_24_48(job) + case "72": + rseworker.DoJob_24_72(job) + default: + rseworker.DoJob_24(job) + } +} + +func (rseworker *ResendInvitationEmailWorker) DoJob_24(job *model.Job) { + elapsedTimeSinceSchedule, DurationInMillis_24, _, _ := rseworker.GetDurations(job) + if elapsedTimeSinceSchedule > DurationInMillis_24 { + rseworker.ResendEmails(job, "24") + rseworker.TearDown(job) + } +} + +func (rseworker *ResendInvitationEmailWorker) DoJob_24_48(job *model.Job) { + elapsedTimeSinceSchedule, DurationInMillis_24, DurationInMillis_48, _ := rseworker.GetDurations(job) + rseworker.Execute(job, elapsedTimeSinceSchedule, DurationInMillis_24, DurationInMillis_48) +} + +func (rseworker *ResendInvitationEmailWorker) DoJob_24_72(job *model.Job) { + elapsedTimeSinceSchedule, DurationInMillis_24, _, DurationInMillis_72 := rseworker.GetDurations(job) + rseworker.Execute(job, elapsedTimeSinceSchedule, DurationInMillis_24, DurationInMillis_72) +} + +func (rseworker *ResendInvitationEmailWorker) Execute(job *model.Job, elapsedTimeSinceSchedule, firstDuration, secondDuration int64) { + systemValue, sysValErr := rseworker.App.Srv().Store.System().GetByName(job.Id) + if sysValErr != nil { + if _, ok := sysValErr.(*store.ErrNotFound); !ok { + mlog.Error("An error occurred while getting NUMBER_OF_INVITE_EMAILS_SENT from system store", mlog.String("worker", rseworker.name), mlog.Err(sysValErr)) + // system value information is critical and if it was not set for this job at creation, we want to cancel the job all together. + rseworker.setJobCancelled(job) + return + } + } + + if (elapsedTimeSinceSchedule > firstDuration) && (systemValue.Value == "0") { + rseworker.ResendEmails(job, "48") + rseworker.setNumResendEmailSent(job, "1") + } else if elapsedTimeSinceSchedule > secondDuration { + rseworker.ResendEmails(job, "72") + rseworker.TearDown(job) + } +} + +func (rseworker *ResendInvitationEmailWorker) setJobSuccess(job *model.Job) { + if err := rseworker.App.Srv().Jobs.SetJobSuccess(job); err != nil { + mlog.Error("Worker: Failed to set success for job", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", err.Error())) + rseworker.setJobError(job, err) + } +} + +func (rseworker *ResendInvitationEmailWorker) setJobCancelled(job *model.Job) { + if err := rseworker.App.Srv().Jobs.SetJobCanceled(job); err != nil { + mlog.Error("Worker: Failed to cancel job", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", err.Error())) + rseworker.setJobError(job, err) + } +} + +func (rseworker *ResendInvitationEmailWorker) setJobError(job *model.Job, appError *model.AppError) { + if err := rseworker.App.Srv().Jobs.SetJobError(job, appError); err != nil { + mlog.Error("Worker: Failed to set job error", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", err.Error())) + } +} + func (rseworker *ResendInvitationEmailWorker) cleanEmailData(emailStringData string) ([]string, error) { // emailStringData looks like this ["user1@gmail.com","user2@gmail.com"] emails := []string{} @@ -95,54 +169,66 @@ func (rseworker *ResendInvitationEmailWorker) removeAlreadyJoined(teamID string, return notJoinedYet } -func (rseworker *ResendInvitationEmailWorker) DoJob(job *model.Job) { +func (rseworker *ResendInvitationEmailWorker) setNumResendEmailSent(job *model.Job, num string) { + sysVar := &model.System{Name: job.Id, Value: num} + if err := rseworker.App.Srv().Store.System().SaveOrUpdate(sysVar); err != nil { + mlog.Error("Unable to save NUMBER_OF_INVITE_EMAIL_SENT", mlog.String("worker", rseworker.name), mlog.Err(err)) + } +} + +func (rseworker *ResendInvitationEmailWorker) GetDurations(job *model.Job) (int64, int64, int64, int64) { scheduledAt, _ := strconv.ParseInt(job.Data["scheduledAt"], 10, 64) now := model.GetMillis() elapsedTimeSinceSchedule := now - scheduledAt - var DurationInMillis int64 - - duration := os.Getenv("MM_RESEND_INVITATION_EMAIL_JOB_DURATION") - - DurationInMillis, parseError := strconv.ParseInt(duration, 10, 64) + duration_24 := os.Getenv("MM_RESEND_INVITATION_EMAIL_JOB_DURATION") + DurationInMillis_24, parseError := strconv.ParseInt(duration_24, 10, 64) if parseError != nil { // default to 24 hours - DurationInMillis = TwentyFourHoursInMillis + DurationInMillis_24 = TwentyFourHoursInMillis } - if elapsedTimeSinceSchedule > DurationInMillis { - teamID := job.Data["teamID"] - emailListData := job.Data["emailList"] - - emailList, err := rseworker.cleanEmailData(emailListData) - if err != nil { - appErr := model.NewAppError("worker: "+rseworker.name, "job_id: "+job.Id, nil, err.Error(), http.StatusInternalServerError) - mlog.Error("Worker: Failed to clean emails string data", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", appErr.Error())) - rseworker.setJobError(job, appErr) - } - - emailList = rseworker.removeAlreadyJoined(teamID, emailList) - - _, appErr := rseworker.App.InviteNewUsersToTeamGracefully(emailList, teamID, job.Data["senderID"]) - if appErr != nil { - mlog.Error("Worker: Failed to send emails", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", appErr.Error())) - rseworker.setJobError(job, appErr) - } - rseworker.setJobSuccess(job) + duration_48 := os.Getenv("MM_RESEND_INVITATION_EMAIL_JOB_DURATION_48") + DurationInMillis_48, parseError := strconv.ParseInt(duration_48, 10, 64) + if parseError != nil { + // default to 48 hours + DurationInMillis_48 = FourtyEightHoursInMillis } + duration_72 := os.Getenv("MM_RESEND_INVITATION_EMAIL_JOB_DURATION_72") + DurationInMillis_72, parseError := strconv.ParseInt(duration_72, 10, 64) + if parseError != nil { + // default to 72 hours + DurationInMillis_72 = SeventyTwoHoursInMillis + } + + return elapsedTimeSinceSchedule, DurationInMillis_24, DurationInMillis_48, DurationInMillis_72 + } -func (rseworker *ResendInvitationEmailWorker) setJobSuccess(job *model.Job) { - if err := rseworker.App.Srv().Jobs.SetJobSuccess(job); err != nil { - mlog.Error("Worker: Failed to set success for job", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", err.Error())) - rseworker.setJobError(job, err) - } +func (rseworker *ResendInvitationEmailWorker) TearDown(job *model.Job) { + rseworker.App.Srv().Store.System().PermanentDeleteByName(job.Id) + rseworker.setJobSuccess(job) } -func (rseworker *ResendInvitationEmailWorker) setJobError(job *model.Job, appError *model.AppError) { - if err := rseworker.App.Srv().Jobs.SetJobError(job, appError); err != nil { - mlog.Error("Worker: Failed to set job error", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", err.Error())) +func (rseworker *ResendInvitationEmailWorker) ResendEmails(job *model.Job, interval string) { + teamID := job.Data["teamID"] + emailListData := job.Data["emailList"] + + emailList, err := rseworker.cleanEmailData(emailListData) + if err != nil { + appErr := model.NewAppError("worker: "+rseworker.name, "job_id: "+job.Id, nil, err.Error(), http.StatusInternalServerError) + mlog.Error("Worker: Failed to clean emails string data", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", appErr.Error())) + rseworker.setJobError(job, appErr) } + + emailList = rseworker.removeAlreadyJoined(teamID, emailList) + + _, appErr := rseworker.App.InviteNewUsersToTeamGracefully(emailList, teamID, job.Data["senderID"], interval) + if appErr != nil { + mlog.Error("Worker: Failed to send emails", mlog.String("worker", rseworker.name), mlog.String("job_id", job.Id), mlog.String("error", appErr.Error())) + rseworker.setJobError(job, appErr) + } + rseworker.App.Srv().GetTelemetryService().SendTelemetry("track_invite_email_resend", map[string]interface{}{interval: interval}) } diff --git a/model/feature_flags.go b/model/feature_flags.go index 4d412950d1..c1d6427f87 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -46,6 +46,7 @@ type FeatureFlags struct { // Determine whether when a user gets created, they'll have noisy notifications e.g. Send desktop notifications for all activity NewAccountNoisy bool + // Enable Boards Unfurl Preview BoardsUnfurl bool @@ -61,6 +62,9 @@ type FeatureFlags struct { // A/B test for the add members to channel button, possible values = ("top", "bottom") AddMembersToChannel string + // Determine after which duration in hours to send a second invitation to someone that didn't join after the initial invite, possible values = ("48", "72") + ResendInviteEmailInterval string + // A/B test for whether radio buttons or toggle button is more effective in in-screen invite to team modal ("none", "toggle") InviteToTeam string } @@ -84,6 +88,7 @@ func (f *FeatureFlags) SetDefaults() { f.AutoTour = "none" f.BoardsFeatureFlags = "" f.AddMembersToChannel = "top" + f.ResendInviteEmailInterval = "" f.InviteToTeam = "none" } diff --git a/model/team_member.go b/model/team_member.go index c0b3772c1a..cec0a6a6bd 100644 --- a/model/team_member.go +++ b/model/team_member.go @@ -69,6 +69,10 @@ type TeamMembersGetOptions struct { ViewRestrictions *ViewUsersRestrictions } +type TeamInviteReminderData struct { + Interval string +} + func EmailInviteWithErrorToEmails(o []*EmailInviteWithError) []string { var ret []string for _, o := range o {