Merge branch 'master' into mark-as-unread

2019-10-08 15:54:53 -04:00
--- a/app/diagnostics.go
+++ b/app/diagnostics.go
@@ -296,7 +296,6 @@ func (a *App) trackConfig() {
 		"experimental_strict_csrf_enforcement":                    *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement,
 		"enable_email_invitations":                                *cfg.ServiceSettings.EnableEmailInvitations,
 		"experimental_channel_organization":                       *cfg.ServiceSettings.ExperimentalChannelOrganization,
-		"experimental_ldap_group_sync":                            *cfg.ServiceSettings.ExperimentalLdapGroupSync,
 		"disable_bots_when_owner_is_deactivated":                  *cfg.ServiceSettings.DisableBotsWhenOwnerIsDeactivated,
 		"enable_bot_account_creation":                             *cfg.ServiceSettings.EnableBotAccountCreation,
 		"enable_svgs":                                             *cfg.ServiceSettings.EnableSVGs,
--- a/app/notification.go
+++ b/app/notification.go
@@ -152,42 +152,13 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 				continue
 			}

-			userAllowsEmails := profileMap[id].NotifyProps[model.EMAIL_NOTIFY_PROP] != "false"
-			if channelEmail, ok := channelMemberNotifyPropsMap[id][model.EMAIL_NOTIFY_PROP]; ok {
-				if channelEmail != model.CHANNEL_NOTIFY_DEFAULT {
-					userAllowsEmails = channelEmail != "false"
-				}
-			}
-
-			// Remove the user as recipient when the user has muted the channel.
-			if channelMuted, ok := channelMemberNotifyPropsMap[id][model.MARK_UNREAD_NOTIFY_PROP]; ok {
-				if channelMuted == model.CHANNEL_MARK_UNREAD_MENTION {
-					mlog.Debug("Channel muted for user", mlog.String("user_id", id), mlog.String("channel_mute", channelMuted))
-					userAllowsEmails = false
-				}
-			}
-
 			//If email verification is required and user email is not verified don't send email.
 			if *a.Config().EmailSettings.RequireEmailVerification && !profileMap[id].EmailVerified {
 				mlog.Error("Skipped sending notification email, address not verified.", mlog.String("user_email", profileMap[id].Email), mlog.String("user_id", id))
 				continue
 			}

-			var status *model.Status
-			var err *model.AppError
-			if status, err = a.GetStatus(id); err != nil {
-				status = &model.Status{
-					UserId:         id,
-					Status:         model.STATUS_OFFLINE,
-					Manual:         false,
-					LastActivityAt: 0,
-					ActiveChannel:  "",
-				}
-			}
-
-			autoResponderRelated := status.Status == model.STATUS_OUT_OF_OFFICE || post.Type == model.POST_AUTO_RESPONDER
-
-			if userAllowsEmails && status.Status != model.STATUS_ONLINE && profileMap[id].DeleteAt == 0 && !autoResponderRelated {
+			if a.userAllowsEmail(profileMap[id], channelMemberNotifyPropsMap[id], post) {
 				a.sendNotificationEmail(notification, profileMap[id], team)
 			}
 		}
@@ -368,6 +339,40 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	return mentionedUsersList, nil
 }

+func (a *App) userAllowsEmail(user *model.User, channelMemberNotificationProps model.StringMap, post *model.Post) bool {
+	userAllowsEmails := user.NotifyProps[model.EMAIL_NOTIFY_PROP] != "false"
+	if channelEmail, ok := channelMemberNotificationProps[model.EMAIL_NOTIFY_PROP]; ok {
+		if channelEmail != model.CHANNEL_NOTIFY_DEFAULT {
+			userAllowsEmails = channelEmail != "false"
+		}
+	}
+
+	// Remove the user as recipient when the user has muted the channel.
+	if channelMuted, ok := channelMemberNotificationProps[model.MARK_UNREAD_NOTIFY_PROP]; ok {
+		if channelMuted == model.CHANNEL_MARK_UNREAD_MENTION {
+			mlog.Debug("Channel muted for user", mlog.String("user_id", user.Id), mlog.String("channel_mute", channelMuted))
+			userAllowsEmails = false
+		}
+	}
+
+	var status *model.Status
+	var err *model.AppError
+	if status, err = a.GetStatus(user.Id); err != nil {
+		status = &model.Status{
+			UserId:         user.Id,
+			Status:         model.STATUS_OFFLINE,
+			Manual:         false,
+			LastActivityAt: 0,
+			ActiveChannel:  "",
+		}
+	}
+
+	autoResponderRelated := status.Status == model.STATUS_OUT_OF_OFFICE || post.Type == model.POST_AUTO_RESPONDER
+	emailNotificationsAllowedForStatus := status.Status != model.STATUS_ONLINE && status.Status != model.STATUS_DND
+
+	return userAllowsEmails && emailNotificationsAllowedForStatus && user.DeleteAt == 0 && !autoResponderRelated
+}
+
 // sendOutOfChannelMentions sends an ephemeral post to the sender of a post if any of the given potential mentions
 // are outside of the post's channel. Returns whether or not an ephemeral post was sent.
 func (a *App) sendOutOfChannelMentions(sender *model.User, post *model.Post, channel *model.Channel, potentialMentions []string) (bool, error) {
--- a/app/notification_test.go
+++ b/app/notification_test.go
@@ -2007,3 +2007,100 @@ func TestGetNotificationNameFormat(t *testing.T) {
 		assert.Equal(t, model.SHOW_USERNAME, th.App.GetNotificationNameFormat(th.BasicUser))
 	})
 }
+
+func TestUserAllowsEmail(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	t.Run("should return true", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusOffline(user.Id, true)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       model.CHANNEL_NOTIFY_DEFAULT,
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_ALL,
+		}
+
+		assert.True(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: "some-post-type"}))
+	})
+
+	t.Run("should return false in case the status is ONLINE", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusOnline(user.Id, true)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       model.CHANNEL_NOTIFY_DEFAULT,
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_ALL,
+		}
+
+		assert.False(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: "some-post-type"}))
+	})
+
+	t.Run("should return false in case the EMAIL_NOTIFY_PROP is false", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusOffline(user.Id, true)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       "false",
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_ALL,
+		}
+
+		assert.False(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: "some-post-type"}))
+	})
+
+	t.Run("should return false in case the MARK_UNREAD_NOTIFY_PROP is CHANNEL_MARK_UNREAD_MENTION", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusOffline(user.Id, true)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       model.CHANNEL_NOTIFY_DEFAULT,
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_MENTION,
+		}
+
+		assert.False(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: "some-post-type"}))
+	})
+
+	t.Run("should return false in case the Post type is POST_AUTO_RESPONDER", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusOffline(user.Id, true)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       model.CHANNEL_NOTIFY_DEFAULT,
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_ALL,
+		}
+
+		assert.False(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: model.POST_AUTO_RESPONDER}))
+	})
+
+	t.Run("should return false in case the status is STATUS_OUT_OF_OFFICE", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusOutOfOffice(user.Id)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       model.CHANNEL_NOTIFY_DEFAULT,
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_ALL,
+		}
+
+		assert.False(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: model.POST_AUTO_RESPONDER}))
+	})
+
+	t.Run("should return false in case the status is STATUS_ONLINE", func(t *testing.T) {
+		user := th.CreateUser()
+
+		th.App.SetStatusDoNotDisturb(user.Id)
+
+		channelMemberNotificationProps := model.StringMap{
+			model.EMAIL_NOTIFY_PROP:       model.CHANNEL_NOTIFY_DEFAULT,
+			model.MARK_UNREAD_NOTIFY_PROP: model.CHANNEL_MARK_UNREAD_ALL,
+		}
+
+		assert.False(t, th.App.userAllowsEmail(user, channelMemberNotificationProps, &model.Post{Type: model.POST_AUTO_RESPONDER}))
+	})
+
+}
--- a/app/session.go
+++ b/app/session.go
@@ -4,7 +4,6 @@
 package app

 import (
-	"fmt"
 	"net/http"

 	"github.com/mattermost/mattermost-server/mlog"
@@ -96,7 +95,7 @@ func (a *App) GetSessions(userId string) ([]*model.Session, *model.AppError) {
 func (a *App) UpdateSessionsIsGuest(userId string, isGuest bool) {
 	sessions, err := a.Srv.Store.Session().GetSessions(userId)
 	if err != nil {
-		mlog.Error(fmt.Sprintf("Unable to get user sessions: userId=%s err=%s", userId, err.Error()))
+		mlog.Error("Unable to get user sessions", mlog.String("user_id", userId), mlog.Err(err))
 	}

 	for _, session := range sessions {
@@ -107,7 +106,7 @@ func (a *App) UpdateSessionsIsGuest(userId string, isGuest bool) {
 		}
 		err := a.Srv.Store.Session().UpdateProps(session)
 		if err != nil {
-			mlog.Error(fmt.Sprintf("Unable to update isGuest session: %s", err.Error()))
+			mlog.Error("Unable to update isGuest session", mlog.Err(err))
 			continue
 		}
 		a.AddSessionToCache(session)
@@ -214,7 +213,7 @@ func (a *App) RevokeSessionsForDeviceId(userId string, deviceId string, currentS
 	}
 	for _, session := range sessions {
 		if session.DeviceId == deviceId && session.Id != currentSessionId {
-			mlog.Debug(fmt.Sprintf("Revoking sessionId=%v for userId=%v re-login with same device Id", session.Id, userId), mlog.String("user_id", userId))
+			mlog.Debug("Revoking sessionId for userId. Re-login with the same device Id", mlog.String("session_id", session.Id), mlog.String("user_id", userId))
 			if err := a.RevokeSession(session); err != nil {
 				// Soft error so we still remove the other sessions
 				mlog.Error(err.Error())
@@ -279,7 +278,7 @@ func (a *App) UpdateLastActivityAtIfNeeded(session model.Session) {
 	}

 	if err := a.Srv.Store.Session().UpdateLastActivityAt(session.Id, now); err != nil {
-		mlog.Error(fmt.Sprintf("Failed to update LastActivityAt for user_id=%v and session_id=%v, err=%v", session.UserId, session.Id, err), mlog.String("user_id", session.UserId))
+		mlog.Error("Failed to update LastActivityAt", mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), mlog.Err(err))
 	}

 	session.LastActivityAt = now