* PLT-3647 Added config settings for email batching

* PLT-3647 Refactored generation of email notification

* PLT-3647 Added serverside code for email batching

* PLT-3647 Updated settings UI to enable email batching

* PLT-3647 Removed debug code

* PLT-3647 Fixed 0-padding of minutes in batched notification

* PLT-3647 Updated clientside UI for when email batching is disabled

* Go fmt

* PLT-3647 Changed email batching to be disabled by default

* Updated batched email message

* Added email batching toggle to system console

* Changed Email Notifications > Immediate setting to a 30 second batch interval

* Go fmt

* Fixed link to Mattermost icon in batched email notification

* Updated users to use 30 second email batching by default

* Fully disabled email batching when clustering is enabled

* Fixed email batching setting in the system console

* Fixed casing of 'Send Email notifications' -> 'Send email notifications'

* Updating UI Improvements for email batching (#3736)

* Updated text for notification settings and SiteURL.

* Prevented enabling email batching when SiteURL isn't set in the system console

* Re-added a couple debug messages

* Added warning text when clustering is enabled
Этот коммит содержится в:
Harrison Healey
2016-08-16 14:41:47 -04:00
коммит произвёл Christopher Speller
родитель dde158c57f
Коммит 8203fd16ce
25 изменённых файлов: 1123 добавлений и 212 удалений

Просмотреть файл

@@ -156,6 +156,10 @@ func reloadConfig(c *Context, w http.ResponseWriter, r *http.Request) {
}
utils.LoadConfig(utils.CfgFileName)
// start/restart email batching job if necessary
InitEmailBatching()
w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")
ReturnStatusOK(w)
}
@@ -204,6 +208,9 @@ func saveConfig(c *Context, w http.ResponseWriter, r *http.Request) {
// }
// }
// start/restart email batching job if necessary
InitEmailBatching()
rdata := map[string]string{}
rdata["status"] = "OK"
w.Write([]byte(model.MapToJson(rdata)))

Просмотреть файл

@@ -100,6 +100,8 @@ func InitApi() {
Srv.Router.Handle("/api/{anything:.*}", http.HandlerFunc(Handle404))
utils.InitHTML()
InitEmailBatching()
}
func HandleEtag(etag string, w http.ResponseWriter, r *http.Request) bool {

252
api/email_batching.go Обычный файл
Просмотреть файл

@@ -0,0 +1,252 @@
// Copyright (c) 2016 Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
package api
import (
"database/sql"
"fmt"
"html/template"
"strconv"
"time"
"github.com/mattermost/platform/model"
"github.com/mattermost/platform/utils"
l4g "github.com/alecthomas/log4go"
"github.com/nicksnyder/go-i18n/i18n"
)
const (
EMAIL_BATCHING_TASK_NAME = "Email Batching"
)
var emailBatchingJob *EmailBatchingJob
func InitEmailBatching() {
if *utils.Cfg.EmailSettings.EnableEmailBatching {
if emailBatchingJob == nil {
emailBatchingJob = MakeEmailBatchingJob(*utils.Cfg.EmailSettings.EmailBatchingBufferSize)
}
// note that we don't support changing EmailBatchingBufferSize without restarting the server
emailBatchingJob.Start()
}
}
func AddNotificationEmailToBatch(user *model.User, post *model.Post, team *model.Team) *model.AppError {
if !*utils.Cfg.EmailSettings.EnableEmailBatching {
return model.NewLocAppError("AddNotificationEmailToBatch", "api.email_batching.add_notification_email_to_batch.disabled.app_error", nil, "")
}
if !emailBatchingJob.Add(user, post, team) {
l4g.Error(utils.T("api.email_batching.add_notification_email_to_batch.channel_full.app_error"))
return model.NewLocAppError("AddNotificationEmailToBatch", "api.email_batching.add_notification_email_to_batch.channel_full.app_error", nil, "")
}
return nil
}
type batchedNotification struct {
userId string
post *model.Post
teamName string
}
type EmailBatchingJob struct {
newNotifications chan *batchedNotification
pendingNotifications map[string][]*batchedNotification
}
func MakeEmailBatchingJob(bufferSize int) *EmailBatchingJob {
return &EmailBatchingJob{
newNotifications: make(chan *batchedNotification, bufferSize),
pendingNotifications: make(map[string][]*batchedNotification),
}
}
func (job *EmailBatchingJob) Start() {
if task := model.GetTaskByName(EMAIL_BATCHING_TASK_NAME); task != nil {
task.Cancel()
}
l4g.Debug(utils.T("api.email_batching.start.starting"), *utils.Cfg.EmailSettings.EmailBatchingInterval)
model.CreateRecurringTask(EMAIL_BATCHING_TASK_NAME, job.CheckPendingEmails, time.Duration(*utils.Cfg.EmailSettings.EmailBatchingInterval)*time.Second)
}
func (job *EmailBatchingJob) Add(user *model.User, post *model.Post, team *model.Team) bool {
notification := &batchedNotification{
userId: user.Id,
post: post,
teamName: team.Name,
}
select {
case job.newNotifications <- notification:
return true
default:
// return false if we couldn't queue the email notification so that we can send an immediate email
return false
}
}
func (job *EmailBatchingJob) CheckPendingEmails() {
job.handleNewNotifications()
// it's a bit weird to pass the send email function through here, but it makes it so that we can test
// without actually sending emails
job.checkPendingNotifications(time.Now(), sendBatchedEmailNotification)
l4g.Debug(utils.T("api.email_batching.check_pending_emails.finished_running"), len(job.pendingNotifications))
}
func (job *EmailBatchingJob) handleNewNotifications() {
receiving := true
// read in new notifications to send
for receiving {
select {
case notification := <-job.newNotifications:
userId := notification.userId
if _, ok := job.pendingNotifications[userId]; !ok {
job.pendingNotifications[userId] = []*batchedNotification{notification}
} else {
job.pendingNotifications[userId] = append(job.pendingNotifications[userId], notification)
}
default:
receiving = false
}
}
}
func (job *EmailBatchingJob) checkPendingNotifications(now time.Time, handler func(string, []*batchedNotification)) {
// look for users who've acted since pending posts were received
for userId, notifications := range job.pendingNotifications {
schan := Srv.Store.Status().Get(userId)
pchan := Srv.Store.Preference().Get(userId, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL)
batchStartTime := notifications[0].post.CreateAt
// check if the user has been active and would've seen any new posts
if result := <-schan; result.Err != nil {
l4g.Error(utils.T("api.email_batching.check_pending_emails.status.app_error"), result.Err)
delete(job.pendingNotifications, userId)
continue
} else if status := result.Data.(*model.Status); status.LastActivityAt >= batchStartTime {
delete(job.pendingNotifications, userId)
continue
}
// get how long we need to wait to send notifications to the user
var interval int64
if result := <-pchan; result.Err != nil {
// default to 30 seconds to match the send "immediate" setting
interval, _ = strconv.ParseInt(model.PREFERENCE_DEFAULT_EMAIL_INTERVAL, 10, 64)
} else {
preference := result.Data.(model.Preference)
if value, err := strconv.ParseInt(preference.Value, 10, 64); err != nil {
interval, _ = strconv.ParseInt(model.PREFERENCE_DEFAULT_EMAIL_INTERVAL, 10, 64)
} else {
interval = value
}
}
// send the email notification if it's been long enough
if now.Sub(time.Unix(batchStartTime/1000, 0)) > time.Duration(interval)*time.Second {
go handler(userId, notifications)
delete(job.pendingNotifications, userId)
}
}
}
func sendBatchedEmailNotification(userId string, notifications []*batchedNotification) {
uchan := Srv.Store.User().Get(userId)
pchan := Srv.Store.Preference().Get(userId, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_DISPLAY_NAME_FORMAT)
var user *model.User
if result := <-uchan; result.Err != nil {
l4g.Warn("api.email_batching.send_batched_email_notification.user.app_error")
return
} else {
user = result.Data.(*model.User)
}
translateFunc := utils.GetUserTranslations(user.Locale)
var displayNameFormat string
if result := <-pchan; result.Err != nil && result.Err.DetailedError != sql.ErrNoRows.Error() {
l4g.Warn("api.email_batching.send_batched_email_notification.preferences.app_error")
return
} else if result.Err != nil {
// no display name format saved, so fall back to default
displayNameFormat = model.PREFERENCE_DEFAULT_DISPLAY_NAME_FORMAT
} else {
displayNameFormat = result.Data.(model.Preference).Value
}
var contents string
for _, notification := range notifications {
template := utils.NewHTMLTemplate("post_batched_post", user.Locale)
contents += renderBatchedPost(template, notification.post, notification.teamName, displayNameFormat, translateFunc)
}
tm := time.Unix(notifications[0].post.CreateAt/1000, 0)
subject := translateFunc("api.email_batching.send_batched_email_notification.subject", len(notifications), map[string]interface{}{
"SiteName": utils.Cfg.TeamSettings.SiteName,
"Year": tm.Year(),
"Month": translateFunc(tm.Month().String()),
"Day": tm.Day(),
})
body := utils.NewHTMLTemplate("post_batched_body", user.Locale)
body.Props["SiteURL"] = *utils.Cfg.ServiceSettings.SiteURL
body.Props["Posts"] = template.HTML(contents)
body.Props["BodyText"] = translateFunc("api.email_batching.send_batched_email_notification.body_text", len(notifications))
if err := utils.SendMail(user.Email, subject, body.Render()); err != nil {
l4g.Warn(utils.T("api.email_batchings.send_batched_email_notification.send.app_error"), user.Email, err)
}
}
func renderBatchedPost(template *utils.HTMLTemplate, post *model.Post, teamName string, displayNameFormat string, translateFunc i18n.TranslateFunc) string {
schan := Srv.Store.User().Get(post.UserId)
cchan := Srv.Store.Channel().Get(post.ChannelId)
template.Props["Button"] = translateFunc("api.email_batching.render_batched_post.go_to_post")
template.Props["PostMessage"] = getMessageForNotification(post, translateFunc)
template.Props["PostLink"] = *utils.Cfg.ServiceSettings.SiteURL + "/" + teamName + "/pl/" + post.Id
tm := time.Unix(post.CreateAt/1000, 0)
timezone, _ := tm.Zone()
template.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,
})
if result := <-schan; result.Err != nil {
l4g.Warn(utils.T("api.email_batching.render_batched_post.sender.app_error"))
return ""
} else {
template.Props["SenderName"] = result.Data.(*model.User).GetDisplayNameForPreference(displayNameFormat)
}
if result := <-cchan; result.Err != nil {
l4g.Warn(utils.T("api.email_batching.render_batched_post.channel.app_error"))
return ""
} else if channel := result.Data.(*model.Channel); channel.Type == model.CHANNEL_DIRECT {
template.Props["ChannelName"] = translateFunc("api.email_batching.render_batched_post.direct_message")
} else {
template.Props["ChannelName"] = channel.DisplayName
}
return template.Render()
}

193
api/email_batching_test.go Обычный файл
Просмотреть файл

@@ -0,0 +1,193 @@
// Copyright (c) 2016 Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
package api
import (
"testing"
"time"
"github.com/mattermost/platform/model"
"github.com/mattermost/platform/store"
)
func TestHandleNewNotifications(t *testing.T) {
Setup()
id1 := model.NewId()
id2 := model.NewId()
id3 := model.NewId()
// test queueing of received posts by user
job := MakeEmailBatchingJob(128)
job.handleNewNotifications()
if len(job.pendingNotifications) != 0 {
t.Fatal("shouldn't have added any pending notifications")
}
job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test"}, &model.Team{Name: "team"})
if len(job.pendingNotifications) != 0 {
t.Fatal("shouldn't have added any pending notifications")
}
job.handleNewNotifications()
if len(job.pendingNotifications) != 1 {
t.Fatal("should have received posts for 1 user")
} else if len(job.pendingNotifications[id1]) != 1 {
t.Fatal("should have received 1 post for user")
}
job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test"}, &model.Team{Name: "team"})
job.handleNewNotifications()
if len(job.pendingNotifications) != 1 {
t.Fatal("should have received posts for 1 user")
} else if len(job.pendingNotifications[id1]) != 2 {
t.Fatal("should have received 2 posts for user1", job.pendingNotifications[id1])
}
job.Add(&model.User{Id: id2}, &model.Post{UserId: id1, Message: "test"}, &model.Team{Name: "team"})
job.handleNewNotifications()
if len(job.pendingNotifications) != 2 {
t.Fatal("should have received posts for 2 users")
} else if len(job.pendingNotifications[id1]) != 2 {
t.Fatal("should have received 2 posts for user1")
} else if len(job.pendingNotifications[id2]) != 1 {
t.Fatal("should have received 1 post for user2")
}
job.Add(&model.User{Id: id2}, &model.Post{UserId: id2, Message: "test"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id1}, &model.Post{UserId: id3, Message: "test"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id3}, &model.Post{UserId: id3, Message: "test"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id2}, &model.Post{UserId: id2, Message: "test"}, &model.Team{Name: "team"})
job.handleNewNotifications()
if len(job.pendingNotifications) != 3 {
t.Fatal("should have received posts for 3 users")
} else if len(job.pendingNotifications[id1]) != 3 {
t.Fatal("should have received 3 posts for user1")
} else if len(job.pendingNotifications[id2]) != 3 {
t.Fatal("should have received 3 posts for user2")
} else if len(job.pendingNotifications[id3]) != 1 {
t.Fatal("should have received 1 post for user3")
}
// test ordering of received posts
job = MakeEmailBatchingJob(128)
job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test1"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test2"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id2}, &model.Post{UserId: id1, Message: "test3"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id1}, &model.Post{UserId: id1, Message: "test4"}, &model.Team{Name: "team"})
job.Add(&model.User{Id: id2}, &model.Post{UserId: id1, Message: "test5"}, &model.Team{Name: "team"})
job.handleNewNotifications()
if job.pendingNotifications[id1][0].post.Message != "test1" ||
job.pendingNotifications[id1][1].post.Message != "test2" ||
job.pendingNotifications[id1][2].post.Message != "test4" {
t.Fatal("incorrect order of received posts for user1")
} else if job.pendingNotifications[id2][0].post.Message != "test3" ||
job.pendingNotifications[id2][1].post.Message != "test5" {
t.Fatal("incorrect order of received posts for user2")
}
}
func TestCheckPendingNotifications(t *testing.T) {
Setup()
id1 := model.NewId()
job := MakeEmailBatchingJob(128)
job.pendingNotifications[id1] = []*batchedNotification{
{
post: &model.Post{
UserId: id1,
CreateAt: 10000000,
},
},
}
store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{
UserId: id1,
LastActivityAt: 9999000,
}))
store.Must(Srv.Store.Preference().Save(&model.Preferences{{
UserId: id1,
Category: model.PREFERENCE_CATEGORY_NOTIFICATIONS,
Name: model.PREFERENCE_NAME_EMAIL_INTERVAL,
Value: "60",
}}))
// test that notifications aren't sent before interval
job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {})
if job.pendingNotifications[id1] == nil || len(job.pendingNotifications[id1]) != 1 {
t.Fatal("should'nt have sent queued post")
}
// test that notifications are cleared if the user has acted
store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{
UserId: id1,
LastActivityAt: 10001000,
}))
job.checkPendingNotifications(time.Unix(10002, 0), func(string, []*batchedNotification) {})
if job.pendingNotifications[id1] != nil && len(job.pendingNotifications[id1]) != 0 {
t.Fatal("should've remove queued post since user acted")
}
// test that notifications are sent if enough time passes since the first message
job.pendingNotifications[id1] = []*batchedNotification{
{
post: &model.Post{
UserId: id1,
CreateAt: 10060000,
Message: "post1",
},
},
{
post: &model.Post{
UserId: id1,
CreateAt: 10090000,
Message: "post2",
},
},
}
received := make(chan *model.Post, 2)
timeout := make(chan bool)
job.checkPendingNotifications(time.Unix(10130, 0), func(s string, notifications []*batchedNotification) {
for _, notification := range notifications {
received <- notification.post
}
})
go func() {
// start a timeout to make sure that we don't get stuck here on a failed test
time.Sleep(5 * time.Second)
timeout <- true
}()
if job.pendingNotifications[id1] != nil && len(job.pendingNotifications[id1]) != 0 {
t.Fatal("should've remove queued posts when sending messages")
}
select {
case post := <-received:
if post.Message != "post1" {
t.Fatal("should've received post1 first")
}
case _ = <-timeout:
t.Fatal("timed out waiting for first post notification")
}
select {
case post := <-received:
if post.Message != "post2" {
t.Fatal("should've received post2 second")
}
case _ = <-timeout:
t.Fatal("timed out waiting for second post notification")
}
}

Просмотреть файл

@@ -23,6 +23,7 @@ import (
"github.com/mattermost/platform/model"
"github.com/mattermost/platform/store"
"github.com/mattermost/platform/utils"
"github.com/nicksnyder/go-i18n/i18n"
)
const (
@@ -770,6 +771,14 @@ func sendNotificationEmail(c *Context, post *model.Post, user *model.User, chann
return
}
if *utils.Cfg.EmailSettings.EnableEmailBatching {
if err := AddNotificationEmailToBatch(user, post, team); err == nil {
return
}
// fall back to sending a single email if we can't batch it for some reason
}
var channelName string
var bodyText string
var subjectText string
@@ -802,7 +811,7 @@ func sendNotificationEmail(c *Context, post *model.Post, user *model.User, chann
bodyPage := utils.NewHTMLTemplate("post_body", user.Locale)
bodyPage.Props["SiteURL"] = c.GetSiteURL()
bodyPage.Props["PostMessage"] = model.ClearMentionTags(post.Message)
bodyPage.Props["PostMessage"] = getMessageForNotification(post, userLocale)
bodyPage.Props["TeamLink"] = teamURL + "/pl/" + post.Id
bodyPage.Props["BodyText"] = bodyText
bodyPage.Props["Button"] = userLocale("api.templates.post_body.button")
@@ -811,42 +820,39 @@ func sendNotificationEmail(c *Context, post *model.Post, user *model.User, chann
"Hour": fmt.Sprintf("%02d", tm.Hour()), "Minute": fmt.Sprintf("%02d", tm.Minute()),
"TimeZone": zone, "Month": month, "Day": day}))
// attempt to fill in a message body if the post doesn't have any text
if len(strings.TrimSpace(bodyPage.Props["PostMessage"])) == 0 && len(post.Filenames) > 0 {
// extract the filenames from their paths and determine what type of files are attached
filenames := make([]string, len(post.Filenames))
onlyImages := true
for i, filename := range post.Filenames {
var err error
if filenames[i], err = url.QueryUnescape(filepath.Base(filename)); err != nil {
// this should never error since filepath was escaped using url.QueryEscape
filenames[i] = filepath.Base(filename)
}
ext := filepath.Ext(filename)
onlyImages = onlyImages && model.IsFileExtImage(ext)
}
filenamesString := strings.Join(filenames, ", ")
var attachmentPrefix string
if onlyImages {
attachmentPrefix = "Image"
} else {
attachmentPrefix = "File"
}
if len(post.Filenames) > 1 {
attachmentPrefix += "s"
}
bodyPage.Props["PostMessage"] = userLocale("api.post.send_notifications_and_forget.sent",
map[string]interface{}{"Prefix": attachmentPrefix, "Filenames": filenamesString})
}
if err := utils.SendMail(user.Email, subjectPage.Render(), bodyPage.Render()); err != nil {
l4g.Error(utils.T("api.post.send_notifications_and_forget.send.error"), user.Email, err)
}
}
func getMessageForNotification(post *model.Post, translateFunc i18n.TranslateFunc) string {
if len(strings.TrimSpace(post.Message)) != 0 || len(post.Filenames) == 0 {
return post.Message
}
// extract the filenames from their paths and determine what type of files are attached
filenames := make([]string, len(post.Filenames))
onlyImages := true
for i, filename := range post.Filenames {
var err error
if filenames[i], err = url.QueryUnescape(filepath.Base(filename)); err != nil {
// this should never error since filepath was escaped using url.QueryEscape
filenames[i] = filepath.Base(filename)
}
ext := filepath.Ext(filename)
onlyImages = onlyImages && model.IsFileExtImage(ext)
}
props := map[string]interface{}{"Filenames": strings.Join(filenames, ", ")}
if onlyImages {
return translateFunc("api.post.get_message_for_notification.images_sent", len(filenames), props)
} else {
return translateFunc("api.post.get_message_for_notification.files_sent", len(filenames), props)
}
}
func sendPushNotification(post *model.Post, user *model.User, channel *model.Channel, senderName string, wasMentioned bool) {
var sessions []*model.Session
if result := <-Srv.Store.Session().GetSessions(user.Id); result.Err != nil {

Просмотреть файл

@@ -1147,3 +1147,42 @@ func TestGetFlaggedPosts(t *testing.T) {
t.Fatal("should not have gotten a flagged post")
}
}
func TestGetMessageForNotification(t *testing.T) {
Setup()
translateFunc := utils.GetUserTranslations("en")
post := &model.Post{
Message: "test",
Filenames: model.StringArray{},
}
if getMessageForNotification(post, translateFunc) != "test" {
t.Fatal("should've returned message text")
}
post.Filenames = model.StringArray{"test1.png"}
if getMessageForNotification(post, translateFunc) != "test" {
t.Fatal("should've returned message text, even with attachments")
}
post.Message = ""
if message := getMessageForNotification(post, translateFunc); message != "1 image sent: test1.png" {
t.Fatal("should've returned number of images:", message)
}
post.Filenames = model.StringArray{"test1.png", "test2.jpg"}
if message := getMessageForNotification(post, translateFunc); message != "2 images sent: test1.png, test2.jpg" {
t.Fatal("should've returned number of images:", message)
}
post.Filenames = model.StringArray{"test1.go"}
if message := getMessageForNotification(post, translateFunc); message != "1 file sent: test1.go" {
t.Fatal("should've returned number of files:", message)
}
post.Filenames = model.StringArray{"test1.go", "test2.jpg"}
if message := getMessageForNotification(post, translateFunc); message != "2 files sent: test1.go, test2.jpg" {
t.Fatal("should've returned number of mixed files:", message)
}
}