Skip to content

Commit

Permalink
[mattermostGH-15906][MM-22844] Redesign message notification emails. (m…
Browse files Browse the repository at this point in the history 
…attermost#17184)

* Redesign message notification emails.

* Fix tests and linter.

* Fix tests

* Fix tests

* gofmt.

* Fix date separator

* Update html files

* Remove date for message notification email.

* Modify subtitle for mentions and direct and group messages.

* Fix lint error

* Fix DM subtitle.

* Fixing translations

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Jesús Espino <jespinog@gmail.com>
(cherry picked from commit d97daaa)
  • Loading branch information
@jp0707 @mattermost-build
jp0707 authored and mattermost-build committed May 10, 2021
1 parent 0f018dc commit 0051ea1
Show file tree
Hide file tree
Showing 22 changed files with 1,255 additions and 656 deletions.
146 changes: 74 additions & 72 deletions app/email_batching.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
package app

import (
"bytes"
"context"
"fmt"
"html/template"
"io"
"net/http"
"strconv"
"sync"
Expand Down Expand Up @@ -201,33 +203,61 @@ func (es *EmailService) sendBatchedEmailNotification(userID string, notification

translateFunc := i18n.GetUserTranslations(user.Locale)
displayNameFormat := *es.srv.Config().TeamSettings.TeammateNameDisplay
siteURL := *es.srv.Config().ServiceSettings.SiteURL

var contents string
for _, notification := range notifications {
sender, err := es.srv.Store.User().Get(context.Background(), notification.post.UserId)
if err != nil {
mlog.Warn("Unable to find sender of post for batched email notification")
continue
}
postsData := make([]*postData, 0 /* len */, len(notifications) /* cap */)
embeddedFiles := make(map[string]io.Reader)

channel, errCh := es.srv.Store.Channel().Get(notification.post.ChannelId, true)
if errCh != nil {
mlog.Warn("Unable to find channel of post for batched email notification")
continue
}
emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL
if license := es.srv.License(); license != nil && *license.Features.EmailNotificationContents {
emailNotificationContentsType = *es.srv.Config().EmailSettings.EmailNotificationContentsType
}

emailNotificationContentsType := model.EMAIL_NOTIFICATION_CONTENTS_FULL
if license := es.srv.License(); license != nil && *license.Features.EmailNotificationContents {
emailNotificationContentsType = *es.srv.Config().EmailSettings.EmailNotificationContentsType
}
if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL {
for i, notification := range notifications {
sender, errSender := es.srv.Store.User().Get(context.Background(), notification.post.UserId)
if errSender != nil {
mlog.Warn("Unable to find sender of post for batched email notification")
}

postContent, err := es.renderBatchedPost(notification, channel, sender, *es.srv.Config().ServiceSettings.SiteURL, displayNameFormat, translateFunc, user.Locale, emailNotificationContentsType)
if err != nil {
mlog.Warn("Unable to render post for batched email notification template", mlog.Err(err))
continue
}
channel, errCh := es.srv.Store.Channel().Get(notification.post.ChannelId, true)
if errCh != nil {
mlog.Warn("Unable to find channel of post for batched email notification")
}

contents += postContent
senderProfileImage, _, errProfileImage := es.srv.GetProfileImage(sender)
if errProfileImage != nil {
mlog.Warn("Unable to get the sender user profile image.", mlog.String("user_id", sender.Id), mlog.Err(errProfileImage))
}

senderPhoto := fmt.Sprintf("user-avatar-%d.png", i)
if senderProfileImage != nil {
embeddedFiles[senderPhoto] = bytes.NewReader(senderProfileImage)
}

tm := time.Unix(notification.post.CreateAt/1000, 0)
timezone, _ := tm.Zone()

t := translateFunc("api.email_batching.send_batched_email_notification.time", map[string]interface{}{
"Hour": tm.Hour(),
"Minute": fmt.Sprintf("%02d", tm.Minute()),
"Month": translateFunc(tm.Month().String()),
"Day": tm.Day(),
"Year": tm.Year(),
"Timezone": timezone,
})

MessageURL := siteURL + "/" + notification.teamName + "/pl/" + notification.post.Id

postsData = append(postsData, &postData{
SenderPhoto: senderPhoto,
SenderName: sender.GetDisplayName(displayNameFormat),
Time: t,
ChannelName: channel.DisplayName,
Message: template.HTML(es.srv.GetMessageForNotification(notification.post, translateFunc)),
MessageURL: MessageURL,
})
}
}

tm := time.Unix(notifications[0].post.CreateAt/1000, 0)
Expand All @@ -239,59 +269,31 @@ func (es *EmailService) sendBatchedEmailNotification(userID string, notification
"Day": tm.Day(),
})

data := es.newEmailTemplateData(user.Locale)
data.Props["SiteURL"] = *es.srv.Config().ServiceSettings.SiteURL
data.Props["Posts"] = template.HTML(contents)
data.Props["BodyText"] = translateFunc("api.email_batching.send_batched_email_notification.body_text", len(notifications))

body, err2 := es.srv.TemplatesContainer().RenderToString("post_batched_body", data)
if err2 != nil {
mlog.Warn("Unable build the batched email notification template", mlog.Err(err2))
return
}

if nErr := es.sendNotificationMail(user.Email, subject, body); nErr != nil {
mlog.Warn("Unable to send batched email notification", mlog.String("email", user.Email), mlog.Err(nErr))
}
}

func (es *EmailService) renderBatchedPost(notification *batchedNotification, channel *model.Channel, sender *model.User, siteURL string, displayNameFormat string, translateFunc i18n.TranslateFunc, userLocale string, emailNotificationContentsType string) (string, error) {
// don't include message contents if email notification contents type is set to generic
var templateName = "post_batched_post_generic"
if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL {
templateName = "post_batched_post_full"
firstSender, err := es.srv.Store.User().Get(context.Background(), notifications[0].post.UserId)
if err != nil {
mlog.Warn("Unable to find sender of post for batched email notification")
}

data := es.newEmailTemplateData(userLocale)
data.Props["Button"] = translateFunc("api.email_batching.render_batched_post.go_to_post")
data.Props["PostMessage"] = es.srv.GetMessageForNotification(notification.post, translateFunc)
data.Props["PostLink"] = siteURL + "/" + notification.teamName + "/pl/" + notification.post.Id
data.Props["SenderName"] = sender.GetDisplayName(displayNameFormat)

tm := time.Unix(notification.post.CreateAt/1000, 0)
timezone, _ := tm.Zone()

data.Props["Date"] = translateFunc("api.email_batching.render_batched_post.date", map[string]interface{}{
"Year": tm.Year(),
"Month": translateFunc(tm.Month().String()),
"Day": tm.Day(),
"Hour": tm.Hour(),
"Minute": fmt.Sprintf("%02d", tm.Minute()),
"Timezone": timezone,
data := es.newEmailTemplateData(user.Locale)
data.Props["SiteURL"] = siteURL
data.Props["Title"] = translateFunc("api.email_batching.send_batched_email_notification.title", len(notifications)-1, map[string]interface{}{
"SenderName": firstSender.GetDisplayName(displayNameFormat),
})

if channel.Type == model.CHANNEL_DIRECT {
data.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.direct_message")
} else if channel.Type == model.CHANNEL_GROUP {
data.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.group_message")
} else {
// don't include channel name if email notification contents type is set to generic
if emailNotificationContentsType == model.EMAIL_NOTIFICATION_CONTENTS_FULL {
data.Props["ChannelName"] = channel.DisplayName
} else {
data.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.notification")
}
data.Props["SubTitle"] = translateFunc("api.email_batching.send_batched_email_notification.subTitle")
data.Props["Button"] = translateFunc("api.email_batching.send_batched_email_notification.button")
data.Props["ButtonURL"] = siteURL
data.Props["Posts"] = postsData
data.Props["MessageButton"] = translateFunc("api.email_batching.send_batched_email_notification.messageButton")
data.Props["NotificationFooterTitle"] = translateFunc("app.notification.footer.title")
data.Props["NotificationFooterInfoLogin"] = translateFunc("app.notification.footer.infoLogin")
data.Props["NotificationFooterInfo"] = translateFunc("app.notification.footer.info")

renderedPage, renderErr := es.srv.TemplatesContainer().RenderToString("messages_notification", data)
if renderErr != nil {
mlog.Error("Unable to render email", mlog.Err(renderErr))
}

return es.srv.TemplatesContainer().RenderToString(templateName, data)
if nErr := es.sendNotificationMail(user.Email, subject, renderedPage); nErr != nil {
mlog.Warn("Unable to send batched email notification", mlog.String("email", user.Email), mlog.Err(nErr))
}
}
52 changes: 0 additions & 52 deletions app/email_batching_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,55 +283,3 @@ func TestCheckPendingNotificationsCantParseInterval(t *testing.T) {

require.Nil(t, job.pendingNotifications[th.BasicUser.Id], "should have sent queued post")
}

/*
* Ensures that post contents are not included in notification email when email notification content type is set to generic
*/
func TestRenderBatchedPostGeneric(t *testing.T) {
th := SetupWithStoreMock(t)
defer th.TearDown()

var post = &model.Post{}
post.Message = "This is the message"
var notification = &batchedNotification{}
notification.post = post
var channel = &model.Channel{}
channel.DisplayName = "Some Test Channel"
var sender = &model.User{}
sender.Email = "sender@test.com"

translateFunc := func(translationID string, args ...interface{}) string {
// mock translateFunc just returns the translation id - this is good enough for our purposes
return translationID
}

rendered, err := th.Server.EmailService.renderBatchedPost(notification, channel, sender, "http://localhost:8065", "", translateFunc, "en", model.EMAIL_NOTIFICATION_CONTENTS_GENERIC)
require.NoError(t, err)
require.NotContains(t, rendered, post.Message, "Rendered email should not contain post contents when email notification contents type is set to Generic.")
}

/*
* Ensures that post contents included in notification email when email notification content type is set to full
*/
func TestRenderBatchedPostFull(t *testing.T) {
th := SetupWithStoreMock(t)
defer th.TearDown()

var post = &model.Post{}
post.Message = "This is the message"
var notification = &batchedNotification{}
notification.post = post
var channel = &model.Channel{}
channel.DisplayName = "Some Test Channel"
var sender = &model.User{}
sender.Email = "sender@test.com"

translateFunc := func(translationID string, args ...interface{}) string {
// mock translateFunc just returns the translation id - this is good enough for our purposes
return translationID
}

rendered, err := th.Server.EmailService.renderBatchedPost(notification, channel, sender, "http://localhost:8065", "", translateFunc, "en", model.EMAIL_NOTIFICATION_CONTENTS_FULL)
require.NoError(t, err)
require.Contains(t, rendered, post.Message, "Rendered email should contain post contents when email notification contents type is set to Full.")
}
21 changes: 2 additions & 19 deletions app/file.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,7 @@ func (a *App) TestFileStoreConnectionWithConfig(cfg *model.FileSettings) *model.
}

func (a *App) ReadFile(path string) ([]byte, *model.AppError) {
backend, err := a.FileBackend()
if err != nil {
return nil, err
}
result, nErr := backend.ReadFile(path)
if nErr != nil {
return nil, model.NewAppError("ReadFile", "api.file.read_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}
return result, nil
return a.srv.ReadFile(path)
}

// Caller must close the first return value
Expand Down Expand Up @@ -202,16 +194,7 @@ func (a *App) MoveFile(oldPath, newPath string) *model.AppError {
}

func (a *App) WriteFile(fr io.Reader, path string) (int64, *model.AppError) {
backend, err := a.FileBackend()
if err != nil {
return 0, err
}

result, nErr := backend.WriteFile(fr, path)
if nErr != nil {
return result, model.NewAppError("WriteFile", "api.file.write_file.app_error", nil, nErr.Error(), http.StatusInternalServerError)
}
return result, nil
return a.srv.WriteFile(fr, path)
}

func (a *App) AppendFile(fr io.Reader, path string) (int64, *model.AppError) {
Expand Down
5 changes: 4 additions & 1 deletion app/notification.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,11 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
}

if a.userAllowsEmail(profileMap[id], channelMemberNotifyPropsMap[id], post) {
err := a.sendNotificationEmail(notification, profileMap[id], team)
senderProfileImage, _, err := a.GetProfileImage(sender)
if err != nil {
a.Log().Warn("Unable to get the sender user profile image.", mlog.String("user_id", sender.Id), mlog.Err(err))
}
if err := a.sendNotificationEmail(notification, profileMap[id], team, senderProfileImage); err != nil {
mlog.Warn("Unable to send notification email.", mlog.Err(err))
}
}
Expand Down

0 comments on commit 0051ea1

Please sign in to comment.