Fix msg command so that it doesn't try to pull all user profiles at once (#4936)

api/command_msg.go

@@ -39,63 +39,56 @@ func (me *msgProvider) DoCommand(c *Context, args *model.CommandArgs, message st
 	splitMessage := strings.SplitN(message, " ", 2)
 
 	parsedMessage := ""
-	targetUser := ""
+	targetUsername := ""
 
 	if len(splitMessage) > 1 {
 		parsedMessage = strings.SplitN(message, " ", 2)[1]
 	}
-	targetUser = strings.SplitN(message, " ", 2)[0]
+	targetUsername = strings.SplitN(message, " ", 2)[0]
-	targetUser = strings.TrimPrefix(targetUser, "@")
+	targetUsername = strings.TrimPrefix(targetUsername, "@")
 
-	// FIX ME
+	var userProfile *model.User
-	// Why isn't this selecting by username since we have that?
+	if result := <-Srv.Store.User().GetByUsername(targetUsername); result.Err != nil {
-	if profileList := <-Srv.Store.User().GetAll(); profileList.Err != nil {
+		c.Err = result.Err
-		c.Err = profileList.Err
+		return &model.CommandResponse{Text: c.T("api.command_msg.missing.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-		return &model.CommandResponse{Text: c.T("api.command_msg.list.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
 	} else {
-		profileUsers := profileList.Data.([]*model.User)
+		userProfile = result.Data.(*model.User)
-		for _, userProfile := range profileUsers {
+	}
-			// Don't let users open DMs with themselves. It probably won't work out well.
+
-			if userProfile.Id == c.Session.UserId {
+	if userProfile.Id == c.Session.UserId {
-				continue
+		return &model.CommandResponse{Text: c.T("api.command_msg.missing.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+	}
+
+	// Find the channel based on this user
+	channelName := model.GetDMNameFromIds(c.Session.UserId, userProfile.Id)
+
+	targetChannelId := ""
+	if channel := <-Srv.Store.Channel().GetByName(c.TeamId, channelName); channel.Err != nil {
+		if channel.Err.Id == "store.sql_channel.get_by_name.missing.app_error" {
+			if directChannel, err := CreateDirectChannel(c.Session.UserId, userProfile.Id); err != nil {
+				c.Err = err
+				return &model.CommandResponse{Text: c.T("api.command_msg.dm_fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+			} else {
+				targetChannelId = directChannel.Id
 			}
-			if userProfile.Username == targetUser {
+		} else {
-				targetChannelId := ""
+			c.Err = channel.Err
+			return &model.CommandResponse{Text: c.T("api.command_msg.dm_fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+		}
+	} else {
+		targetChannelId = channel.Data.(*model.Channel).Id
+	}
 
-				// Find the channel based on this user
+	makeDirectChannelVisible(targetChannelId)
-				channelName := model.GetDMNameFromIds(c.Session.UserId, userProfile.Id)
+	if len(parsedMessage) > 0 {
-
+		post := &model.Post{}
-				if channel := <-Srv.Store.Channel().GetByName(c.TeamId, channelName); channel.Err != nil {
+		post.Message = parsedMessage
-					if channel.Err.Id == "store.sql_channel.get_by_name.missing.app_error" {
+		post.ChannelId = targetChannelId
-						if directChannel, err := CreateDirectChannel(c.Session.UserId, userProfile.Id); err != nil {
+		post.UserId = c.Session.UserId
-							c.Err = err
+		if _, err := CreatePost(c, post, true); err != nil {
-							return &model.CommandResponse{Text: c.T("api.command_msg.dm_fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+			return &model.CommandResponse{Text: c.T("api.command_msg.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-						} else {
-							targetChannelId = directChannel.Id
-						}
-					} else {
-						c.Err = channel.Err
-						return &model.CommandResponse{Text: c.T("api.command_msg.dm_fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-					}
-				} else {
-					targetChannelId = channel.Data.(*model.Channel).Id
-				}
-
-				makeDirectChannelVisible(targetChannelId)
-				if len(parsedMessage) > 0 {
-					post := &model.Post{}
-					post.Message = parsedMessage
-					post.ChannelId = targetChannelId
-					post.UserId = c.Session.UserId
-					if _, err := CreatePost(c, post, true); err != nil {
-						return &model.CommandResponse{Text: c.T("api.command_msg.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-					}
-				}
-
-				return &model.CommandResponse{GotoLocation: c.GetTeamURL() + "/channels/" + channelName, Text: "", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-			}
 		}
 	}
 
-	return &model.CommandResponse{Text: c.T("api.command_msg.missing.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+	return &model.CommandResponse{GotoLocation: c.GetTeamURL() + "/channels/" + channelName, Text: "", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
 }

api/command_msg_test.go

@@ -38,4 +38,7 @@ func TestMsgCommands(t *testing.T) {
 	if !strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user2.Id) && !strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+user2.Id+"__"+user1.Id) {
 		t.Fatal("failed to go back to existing direct channel")
 	}
+
+	Client.Must(Client.Command("", "/msg "+th.BasicUser.Username+" foobar"))
+	Client.Must(Client.Command("", "/msg junk foobar"))
 }