diff options
-rw-r--r-- | app/email_batching.go | 40 | ||||
-rw-r--r-- | app/email_batching_test.go | 87 | ||||
-rw-r--r-- | i18n/en.json | 4 |
3 files changed, 74 insertions, 57 deletions
diff --git a/app/email_batching.go b/app/email_batching.go index 2e643d2b1..4b557c678 100644 --- a/app/email_batching.go +++ b/app/email_batching.go @@ -122,24 +122,42 @@ func (job *EmailBatchingJob) handleNewNotifications() { } 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 + inspectedTeamNames := make(map[string]string) + for _, notification := range notifications { + // at most, we'll do one check for each team that notifications were sent for + if inspectedTeamNames[notification.teamName] != "" { + continue + } + tchan := Srv.Store.Team().GetByName(notifications[0].teamName) + if result := <-tchan; result.Err != nil { + l4g.Error("Unable to find Team id for notification", result.Err) + continue + } else if team, ok := result.Data.(*model.Team); ok { + inspectedTeamNames[notification.teamName] = team.Id + } - // 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 + // if the user has viewed any channels in this team since the notification was queued, delete + // all queued notifications + mchan := Srv.Store.Channel().GetMembersForUser(inspectedTeamNames[notification.teamName], userId) + if result := <-mchan; result.Err != nil { + l4g.Error("Unable to find ChannelMembers for user", result.Err) + continue + } else if channelMembers, ok := result.Data.(*model.ChannelMembers); ok { + for _, channelMember := range *channelMembers { + if channelMember.LastViewedAt >= batchStartTime { + l4g.Info("Deleted notifications for user %s", userId) + delete(job.pendingNotifications, userId) + break + } + } + } } // get how long we need to wait to send notifications to the user var interval int64 + pchan := Srv.Store.Preference().Get(userId, model.PREFERENCE_CATEGORY_NOTIFICATIONS, model.PREFERENCE_NAME_EMAIL_INTERVAL) if result := <-pchan; result.Err != nil { // use the default batching interval if an error ocurrs while fetching user preferences interval, _ = strconv.ParseInt(model.PREFERENCE_EMAIL_INTERVAL_BATCHING_SECONDS, 10, 64) diff --git a/app/email_batching_test.go b/app/email_batching_test.go index 24acc8a65..829bc11af 100644 --- a/app/email_batching_test.go +++ b/app/email_batching_test.go @@ -93,26 +93,26 @@ func TestHandleNewNotifications(t *testing.T) { } func TestCheckPendingNotifications(t *testing.T) { - Setup() - - id1 := model.NewId() + th := Setup().InitBasic() job := MakeEmailBatchingJob(128) - job.pendingNotifications[id1] = []*batchedNotification{ + job.pendingNotifications[th.BasicUser.Id] = []*batchedNotification{ { post: &model.Post{ - UserId: id1, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, CreateAt: 10000000, }, + teamName: th.BasicTeam.Name, }, } - store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{ - UserId: id1, - LastActivityAt: 9999000, - })) + channelMember := store.Must(Srv.Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id)).(*model.ChannelMember) + channelMember.LastViewedAt = 9999999 + store.Must(Srv.Store.Channel().UpdateMember(channelMember)) + store.Must(Srv.Store.Preference().Save(&model.Preferences{{ - UserId: id1, + UserId: th.BasicUser.Id, Category: model.PREFERENCE_CATEGORY_NOTIFICATIONS, Name: model.PREFERENCE_NAME_EMAIL_INTERVAL, Value: "60", @@ -121,37 +121,40 @@ func TestCheckPendingNotifications(t *testing.T) { // 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") + if job.pendingNotifications[th.BasicUser.Id] == nil || len(job.pendingNotifications[th.BasicUser.Id]) != 1 { + t.Fatal("shouldn't 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, - })) + channelMember = store.Must(Srv.Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id)).(*model.ChannelMember) + channelMember.LastViewedAt = 10001000 + store.Must(Srv.Store.Channel().UpdateMember(channelMember)) job.checkPendingNotifications(time.Unix(10002, 0), func(string, []*batchedNotification) {}) - if job.pendingNotifications[id1] != nil && len(job.pendingNotifications[id1]) != 0 { + if job.pendingNotifications[th.BasicUser.Id] != nil && len(job.pendingNotifications[th.BasicUser.Id]) != 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{ + job.pendingNotifications[th.BasicUser.Id] = []*batchedNotification{ { post: &model.Post{ - UserId: id1, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, CreateAt: 10060000, Message: "post1", }, + teamName: th.BasicTeam.Name, }, { post: &model.Post{ - UserId: id1, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, CreateAt: 10090000, Message: "post2", }, + teamName: th.BasicTeam.Name, }, } @@ -170,7 +173,7 @@ func TestCheckPendingNotifications(t *testing.T) { timeout <- true }() - if job.pendingNotifications[id1] != nil && len(job.pendingNotifications[id1]) != 0 { + if job.pendingNotifications[th.BasicUser.Id] != nil && len(job.pendingNotifications[th.BasicUser.Id]) != 0 { t.Fatal("should've remove queued posts when sending messages") } @@ -197,34 +200,34 @@ func TestCheckPendingNotifications(t *testing.T) { * Ensures that email batch interval defaults to 15 minutes for users that haven't explicitly set this preference */ func TestCheckPendingNotificationsDefaultInterval(t *testing.T) { - Setup() - id1 := model.NewId() + th := Setup().InitBasic() job := MakeEmailBatchingJob(128) // bypasses recent user activity check - store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{ - UserId: id1, - LastActivityAt: 9999000, - })) + channelMember := store.Must(Srv.Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id)).(*model.ChannelMember) + channelMember.LastViewedAt = 9999000 + store.Must(Srv.Store.Channel().UpdateMember(channelMember)) - job.pendingNotifications[id1] = []*batchedNotification{ + job.pendingNotifications[th.BasicUser.Id] = []*batchedNotification{ { post: &model.Post{ - UserId: id1, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, CreateAt: 10000000, }, + teamName: th.BasicTeam.Name, }, } // notifications should not be sent 1s after post was created, because default batch interval is 15mins job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {}) - if job.pendingNotifications[id1] == nil || len(job.pendingNotifications[id1]) != 1 { + if job.pendingNotifications[th.BasicUser.Id] == nil || len(job.pendingNotifications[th.BasicUser.Id]) != 1 { t.Fatal("shouldn't have sent queued post") } // notifications should be sent 901s after post was created, because default batch interval is 15mins job.checkPendingNotifications(time.Unix(10901, 0), func(string, []*batchedNotification) {}) - if job.pendingNotifications[id1] != nil || len(job.pendingNotifications[id1]) != 0 { + if job.pendingNotifications[th.BasicUser.Id] != nil || len(job.pendingNotifications[th.BasicUser.Id]) != 0 { t.Fatal("should have sent queued post") } } @@ -233,42 +236,42 @@ func TestCheckPendingNotificationsDefaultInterval(t *testing.T) { * Ensures that email batch interval defaults to 15 minutes if user preference is invalid */ func TestCheckPendingNotificationsCantParseInterval(t *testing.T) { - Setup() - id1 := model.NewId() + th := Setup().InitBasic() job := MakeEmailBatchingJob(128) // bypasses recent user activity check - store.Must(Srv.Store.Status().SaveOrUpdate(&model.Status{ - UserId: id1, - LastActivityAt: 9999000, - })) + channelMember := store.Must(Srv.Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id)).(*model.ChannelMember) + channelMember.LastViewedAt = 9999000 + store.Must(Srv.Store.Channel().UpdateMember(channelMember)) // preference value is not an integer, so we'll fall back to the default 15min value store.Must(Srv.Store.Preference().Save(&model.Preferences{{ - UserId: id1, + UserId: th.BasicUser.Id, Category: model.PREFERENCE_CATEGORY_NOTIFICATIONS, Name: model.PREFERENCE_NAME_EMAIL_INTERVAL, Value: "notAnIntegerValue", }})) - job.pendingNotifications[id1] = []*batchedNotification{ + job.pendingNotifications[th.BasicUser.Id] = []*batchedNotification{ { post: &model.Post{ - UserId: id1, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, CreateAt: 10000000, }, + teamName: th.BasicTeam.Name, }, } // notifications should not be sent 1s after post was created, because default batch interval is 15mins job.checkPendingNotifications(time.Unix(10001, 0), func(string, []*batchedNotification) {}) - if job.pendingNotifications[id1] == nil || len(job.pendingNotifications[id1]) != 1 { + if job.pendingNotifications[th.BasicUser.Id] == nil || len(job.pendingNotifications[th.BasicUser.Id]) != 1 { t.Fatal("shouldn't have sent queued post") } // notifications should be sent 901s after post was created, because default batch interval is 15mins job.checkPendingNotifications(time.Unix(10901, 0), func(string, []*batchedNotification) {}) - if job.pendingNotifications[id1] != nil || len(job.pendingNotifications[id1]) != 0 { + if job.pendingNotifications[th.BasicUser.Id] != nil || len(job.pendingNotifications[th.BasicUser.Id]) != 0 { t.Fatal("should have sent queued post") } } diff --git a/i18n/en.json b/i18n/en.json index 8ada9ce6c..eb7f5e1cc 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -972,10 +972,6 @@ "translation": "Email batching job ran. %v user(s) still have notifications pending." }, { - "id": "api.email_batching.check_pending_emails.status.app_error", - "translation": "Unable to find status of recipient for batched email notification" - }, - { "id": "api.email_batching.render_batched_post.channel.app_error", "translation": "Unable to find channel of post for batched email notification" }, |