MM-17071 Remove auto responder logic from App.SendNotifications (#11789)

* MM-17071 Remove auto responder logic from App.SendNotifications

* Log errors returned by SendAutoResponseIfNecessary

app/auto_responder.go

@@ -8,28 +8,48 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
-func (a *App) SendAutoResponse(channel *model.Channel, receiver *model.User) {
+func (a *App) SendAutoResponseIfNecessary(channel *model.Channel, sender *model.User) (bool, *model.AppError) {
+	if channel.Type != model.CHANNEL_DIRECT {
+		return false, nil
+	}
+
+	receiverId := channel.GetOtherUserIdForDM(sender.Id)
+
+	receiver, err := a.GetUser(receiverId)
+	if err != nil {
+		return false, err
+	}
+
+	return a.SendAutoResponse(channel, receiver)
+}
+
+func (a *App) SendAutoResponse(channel *model.Channel, receiver *model.User) (bool, *model.AppError) {
 	if receiver == nil || receiver.NotifyProps == nil {
-		return
+		return false, nil
 	}
 
 	active := receiver.NotifyProps[model.AUTO_RESPONDER_ACTIVE_NOTIFY_PROP] == "true"
 	message := receiver.NotifyProps[model.AUTO_RESPONDER_MESSAGE_NOTIFY_PROP]
 
-	if active && message != "" {
-		autoResponderPost := &model.Post{
-			ChannelId: channel.Id,
-			Message:   message,
-			RootId:    "",
-			ParentId:  "",
-			Type:      model.POST_AUTO_RESPONDER,
-			UserId:    receiver.Id,
-		}
-
-		if _, err := a.CreatePost(autoResponderPost, channel, false); err != nil {
-			mlog.Error(err.Error())
-		}
+	if !active || message == "" {
+		return false, nil
 	}
+
+	autoResponderPost := &model.Post{
+		ChannelId: channel.Id,
+		Message:   message,
+		RootId:    "",
+		ParentId:  "",
+		Type:      model.POST_AUTO_RESPONDER,
+		UserId:    receiver.Id,
+	}
+
+	if _, err := a.CreatePost(autoResponderPost, channel, false); err != nil {
+		mlog.Error(err.Error())
+		return false, err
+	}
+
+	return true, nil
 }
 
 func (a *App) SetAutoResponderStatus(user *model.User, oldNotifyProps model.StringMap) {

app/auto_responder_test.go

@@ -79,6 +79,64 @@ func TestDisableAutoResponder(t *testing.T) {
 	assert.Equal(t, userUpdated2.NotifyProps["auto_responder_active"], "false")
 }
 
+func TestSendAutoResponseIfNecessary(t *testing.T) {
+	t.Run("should send auto response when enabled", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		receiver := th.CreateUser()
+
+		patch := &model.UserPatch{
+			NotifyProps: map[string]string{
+				"auto_responder_active":  "true",
+				"auto_responder_message": "Hello, I'm unavailable today.",
+			},
+		}
+		receiver, err := th.App.PatchUser(receiver.Id, patch, true)
+		require.Nil(t, err)
+
+		channel := th.CreateDmChannel(receiver)
+
+		sent, err := th.App.SendAutoResponseIfNecessary(channel, th.BasicUser)
+
+		assert.Nil(t, err)
+		assert.True(t, sent)
+	})
+
+	t.Run("should not send auto response when disabled", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		receiver := th.CreateUser()
+
+		patch := &model.UserPatch{
+			NotifyProps: map[string]string{
+				"auto_responder_active":  "false",
+				"auto_responder_message": "Hello, I'm unavailable today.",
+			},
+		}
+		receiver, err := th.App.PatchUser(receiver.Id, patch, true)
+		require.Nil(t, err)
+
+		channel := th.CreateDmChannel(receiver)
+
+		sent, err := th.App.SendAutoResponseIfNecessary(channel, th.BasicUser)
+
+		assert.Nil(t, err)
+		assert.False(t, sent)
+	})
+
+	t.Run("should not send auto response for non-DM channel", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		sent, err := th.App.SendAutoResponseIfNecessary(th.BasicChannel, th.BasicUser)
+
+		assert.Nil(t, err)
+		assert.False(t, sent)
+	})
+}
+
 func TestSendAutoResponseSuccess(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
@@ -101,7 +159,10 @@ func TestSendAutoResponseSuccess(t *testing.T) {
 		th.BasicChannel,
 		false)
 
-	th.App.SendAutoResponse(th.BasicChannel, userUpdated1)
+	sent, err := th.App.SendAutoResponse(th.BasicChannel, userUpdated1)
+
+	assert.Nil(t, err)
+	assert.True(t, sent)
 
 	if list, err := th.App.GetPosts(th.BasicChannel.Id, 0, 1); err != nil {
 		require.Nil(t, err)
@@ -138,7 +199,10 @@ func TestSendAutoResponseFailure(t *testing.T) {
 		th.BasicChannel,
 		false)
 
-	th.App.SendAutoResponse(th.BasicChannel, userUpdated1)
+	sent, err := th.App.SendAutoResponse(th.BasicChannel, userUpdated1)
+
+	assert.Nil(t, err)
+	assert.False(t, sent)
 
 	if list, err := th.App.GetPosts(th.BasicChannel.Id, 0, 1); err != nil {
 		require.Nil(t, err)

app/notification.go

@@ -73,19 +73,9 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 	updateMentionChans := []chan *model.AppError{}
 
 	if channel.Type == model.CHANNEL_DIRECT {
-		var otherUserId string
+		otherUserId := channel.GetOtherUserIdForDM(post.UserId)
 
-		userIds := strings.Split(channel.Name, "__")
-
-		if userIds[0] != userIds[1] {
-			if userIds[0] == post.UserId {
-				otherUserId = userIds[1]
-			} else {
-				otherUserId = userIds[0]
-			}
-		}
-
-		otherUser, ok := profileMap[otherUserId]
+		_, ok := profileMap[otherUserId]
 		if ok {
 			mentionedUserIds[otherUserId] = true
 		}
@@ -93,13 +83,6 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 		if post.Props["from_webhook"] == "true" {
 			mentionedUserIds[post.UserId] = true
 		}
-
-		if post.Type != model.POST_AUTO_RESPONDER {
-			a.Srv.Go(func() {
-				a.SendAutoResponse(channel, otherUser)
-			})
-		}
-
 	} else {
 		keywords := a.getMentionKeywordsInChannel(profileMap, post.Type != model.POST_HEADER_CHANGE && post.Type != model.POST_PURPOSE_CHANGE, channelMemberNotifyPropsMap)
 

app/post.go

@@ -393,6 +393,13 @@ func (a *App) handlePostEvents(post *model.Post, user *model.User, channel *mode
 		return err
 	}
 
+	a.Srv.Go(func() {
+		_, err := a.SendAutoResponseIfNecessary(channel, user)
+		if err != nil {
+			mlog.Error("Failed to send auto response", mlog.String("user_id", user.Id), mlog.String("post_id", post.Id), mlog.Err(err))
+		}
+	})
+
 	if triggerWebhooks {
 		a.Srv.Go(func() {
 			if err := a.handleWebhookEvents(post, team, channel, user); err != nil {

model/channel.go

@@ -239,6 +239,26 @@ func (o *Channel) IsGroupConstrained() bool {
 	return o.GroupConstrained != nil && *o.GroupConstrained
 }
 
+func (o *Channel) GetOtherUserIdForDM(userId string) string {
+	if o.Type != CHANNEL_DIRECT {
+		return ""
+	}
+
+	userIds := strings.Split(o.Name, "__")
+
+	var otherUserId string
+
+	if userIds[0] != userIds[1] {
+		if userIds[0] == userId {
+			otherUserId = userIds[1]
+		} else {
+			otherUserId = userIds[0]
+		}
+	}
+
+	return otherUserId
+}
+
 func GetDMNameFromIds(userId1, userId2 string) string {
 	if userId1 > userId2 {
 		return userId2 + "__" + userId1