Reworked the code for sending notifications and checking for out of channel mentions to share some data

api/post.go

@@ -232,6 +232,8 @@ func handlePostEventsAndForget(c *Context, post *model.Post, triggerWebhooks boo
 		tchan := Srv.Store.Team().Get(c.Session.TeamId)
 		cchan := Srv.Store.Channel().Get(post.ChannelId)
 		uchan := Srv.Store.User().Get(post.UserId)
+		pchan := Srv.Store.User().GetProfiles(c.Session.TeamId)
+		mchan := Srv.Store.Channel().GetMembers(post.ChannelId)
 
 		var team *model.Team
 		if result := <-tchan; result.Err != nil {
@@ -249,8 +251,24 @@ func handlePostEventsAndForget(c *Context, post *model.Post, triggerWebhooks boo
 			channel = result.Data.(*model.Channel)
 		}
 
-		sendNotificationsAndForget(c, post, team, channel)
+		var profiles map[string]*model.User
-		go checkForOutOfChannelMentions(c, post, channel)
+		if result := <-pchan; result.Err != nil {
+			l4g.Error(utils.T("api.post.handle_post_events_and_forget.profiles.error"), c.Session.TeamId, result.Err)
+			return
+		} else {
+			profiles = result.Data.(map[string]*model.User)
+		}
+
+		var members []model.ChannelMember
+		if result := <-mchan; result.Err != nil {
+			l4g.Error(utils.T("api.post.handle_post_events_and_forget.members.error"), post.ChannelId, result.Err)
+			return
+		} else {
+			members = result.Data.([]model.ChannelMember)
+		}
+
+		sendNotificationsAndForget(c, post, team, channel, profiles, members)
+		go checkForOutOfChannelMentions(c, post, channel, profiles, members)
 
 		var user *model.User
 		if result := <-uchan; result.Err != nil {
@@ -415,25 +433,15 @@ func handleWebhookEventsAndForget(c *Context, post *model.Post, team *model.Team
 
 }
 
-func sendNotificationsAndForget(c *Context, post *model.Post, team *model.Team, channel *model.Channel) {
+func sendNotificationsAndForget(c *Context, post *model.Post, team *model.Team, channel *model.Channel, profileMap map[string]*model.User, members []model.ChannelMember) {
 
 	go func() {
-		// Get a list of user names (to be used as keywords) and ids for the given team
-		uchan := Srv.Store.User().GetProfiles(c.Session.TeamId)
-		echan := Srv.Store.Channel().GetMembers(post.ChannelId)
-
 		var channelName string
 		var bodyText string
 		var subjectText string
 
 		var mentionedUsers []string
 
-		if result := <-uchan; result.Err != nil {
-			l4g.Error(utils.T("api.post.send_notifications_and_forget.retrive_profiles.error"), c.Session.TeamId, result.Err)
-			return
-		} else {
-			profileMap := result.Data.(map[string]*model.User)
-
 		if _, ok := profileMap[post.UserId]; !ok {
 			l4g.Error(utils.T("api.post.send_notifications_and_forget.user_id.error"), post.UserId)
 			return
@@ -463,20 +471,13 @@ func sendNotificationsAndForget(c *Context, post *model.Post, team *model.Team,
 			}
 
 		} else {
-
 			// Find out who is a member of the channel, only keep those profiles
-				if eResult := <-echan; eResult.Err != nil {
-					l4g.Error(utils.T("api.post.send_notifications_and_forget.members.error"), post.ChannelId, eResult.Err.Message)
-					return
-				} else {
 			tempProfileMap := make(map[string]*model.User)
-					members := eResult.Data.([]model.ChannelMember)
 			for _, member := range members {
 				tempProfileMap[member.UserId] = profileMap[member.UserId]
 			}
 
 			profileMap = tempProfileMap
-				}
 
 			// Build map for keywords
 			keywordMap := make(map[string][]string)
@@ -696,7 +697,6 @@ func sendNotificationsAndForget(c *Context, post *model.Post, team *model.Team,
 				}
 			}
 		}
-		}
 
 		message := model.NewMessage(c.Session.TeamId, post.ChannelId, post.UserId, model.ACTION_POSTED)
 		message.Add("post", post.ToJson())
@@ -730,13 +730,13 @@ func updateMentionCountAndForget(channelId, userId string) {
 	}()
 }
 
-func checkForOutOfChannelMentions(c *Context, post *model.Post, channel *model.Channel) {
+func checkForOutOfChannelMentions(c *Context, post *model.Post, channel *model.Channel, allProfiles map[string]*model.User, members []model.ChannelMember) {
 	// don't check for out of channel mentions in direct channels
 	if channel.Type == model.CHANNEL_DIRECT {
 		return
 	}
 
-	mentioned := getOutOfChannelMentions(post, channel.TeamId)
+	mentioned := getOutOfChannelMentions(post, allProfiles, members)
 	if len(mentioned) == 0 {
 		return
 	}
@@ -779,29 +779,17 @@ func checkForOutOfChannelMentions(c *Context, post *model.Post, channel *model.C
 }
 
 // Gets a list of users that were mentioned in a given post that aren't in the channel that the post was made in
-func getOutOfChannelMentions(post *model.Post, teamId string) []*model.User {
+func getOutOfChannelMentions(post *model.Post, allProfiles map[string]*model.User, members []model.ChannelMember) []*model.User {
-	pchan := Srv.Store.User().GetProfiles(teamId)
+	// copy the profiles map since we'll be removing items from it
-	mchan := Srv.Store.Channel().GetMembers(post.ChannelId)
+	profiles := make(map[string]*model.User)
-
+	for id, profile := range allProfiles {
-	var profiles map[string]*model.User
+		profiles[id] = profile
-	if result := <-pchan; result.Err != nil {
-		l4g.Error(utils.T("api.post.get_out_of_channel_mentions.retrieve_profiles.error"), teamId, result.Err)
-		return []*model.User{}
-	} else {
-		profiles = result.Data.(map[string]*model.User)
 	}
 
 	// only keep profiles which aren't in the current channel
-	if result := <-mchan; result.Err != nil {
-		l4g.Error(utils.T("api.post.get_out_of_channel_mentions.retrieve_members.error"), post.ChannelId, result.Err)
-		return []*model.User{}
-	} else {
-		members := result.Data.([]model.ChannelMember)
-
 	for _, member := range members {
 		delete(profiles, member.UserId)
 	}
-	}
 
 	var mentioned []*model.User
 

api/post_test.go

@@ -882,27 +882,41 @@ func TestGetOutOfChannelMentions(t *testing.T) {
 	channel1 := &model.Channel{DisplayName: "Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team1.Id}
 	channel1 = Client.Must(Client.CreateChannel(channel1)).Data.(*model.Channel)
 
+	var allProfiles map[string]*model.User
+	if result := <-Srv.Store.User().GetProfiles(team1.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		allProfiles = result.Data.(map[string]*model.User)
+	}
+
+	var members []model.ChannelMember
+	if result := <-Srv.Store.Channel().GetMembers(channel1.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		members = result.Data.([]model.ChannelMember)
+	}
+
 	// test a post that doesn't @mention anybody
 	post1 := &model.Post{ChannelId: channel1.Id, Message: "user1 user2 user3"}
-	if mentioned := getOutOfChannelMentions(post1, team1.Id); len(mentioned) != 0 {
+	if mentioned := getOutOfChannelMentions(post1, allProfiles, members); len(mentioned) != 0 {
 		t.Fatalf("getOutOfChannelMentions returned %v when no users were mentioned", mentioned)
 	}
 
 	// test a post that @mentions someone in the channel
 	post2 := &model.Post{ChannelId: channel1.Id, Message: "@user1 is user1"}
-	if mentioned := getOutOfChannelMentions(post2, team1.Id); len(mentioned) != 0 {
+	if mentioned := getOutOfChannelMentions(post2, allProfiles, members); len(mentioned) != 0 {
 		t.Fatalf("getOutOfChannelMentions returned %v when only users in the channel were mentioned", mentioned)
 	}
 
 	// test a post that @mentions someone not in the channel
 	post3 := &model.Post{ChannelId: channel1.Id, Message: "@user2 and @user3 aren't in the channel"}
-	if mentioned := getOutOfChannelMentions(post3, team1.Id); len(mentioned) != 2 || (mentioned[0].Id != user2.Id && mentioned[0].Id != user3.Id) || (mentioned[1].Id != user2.Id && mentioned[1].Id != user3.Id) {
+	if mentioned := getOutOfChannelMentions(post3, allProfiles, members); len(mentioned) != 2 || (mentioned[0].Id != user2.Id && mentioned[0].Id != user3.Id) || (mentioned[1].Id != user2.Id && mentioned[1].Id != user3.Id) {
 		t.Fatalf("getOutOfChannelMentions returned %v when two users outside the channel were mentioned", mentioned)
 	}
 
 	// test a post that @mentions someone not in the channel as well as someone in the channel
 	post4 := &model.Post{ChannelId: channel1.Id, Message: "@user2 and @user1 might be in the channel"}
-	if mentioned := getOutOfChannelMentions(post4, team1.Id); len(mentioned) != 1 || mentioned[0].Id != user2.Id {
+	if mentioned := getOutOfChannelMentions(post4, allProfiles, members); len(mentioned) != 1 || mentioned[0].Id != user2.Id {
 		t.Fatalf("getOutOfChannelMentions returned %v when someone in the channel and someone  outside the channel were mentioned", mentioned)
 	}
 
@@ -920,9 +934,21 @@ func TestGetOutOfChannelMentions(t *testing.T) {
 	channel2 := &model.Channel{DisplayName: "Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team2.Id}
 	channel2 = Client.Must(Client.CreateChannel(channel2)).Data.(*model.Channel)
 
+	if result := <-Srv.Store.User().GetProfiles(team2.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		allProfiles = result.Data.(map[string]*model.User)
+	}
+
+	if result := <-Srv.Store.Channel().GetMembers(channel2.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		members = result.Data.([]model.ChannelMember)
+	}
+
 	// test a post that @mentions someone on a different team
 	post5 := &model.Post{ChannelId: channel2.Id, Message: "@user2 and @user3 might be in the channel"}
-	if mentioned := getOutOfChannelMentions(post5, team2.Id); len(mentioned) != 0 {
+	if mentioned := getOutOfChannelMentions(post5, allProfiles, members); len(mentioned) != 0 {
 		t.Fatalf("getOutOfChannelMentions returned %v when two users on a different team were mentioned", mentioned)
 	}
 }

i18n/en.json

@@ -691,14 +691,6 @@
     "id": "api.post.delete_post.permissions.app_error",
     "translation": "You do not have the appropriate permissions"
   },
-  {
-    "id": "api.post.get_out_of_channel_mentions.retrieve_members.error",
-    "translation": "Failed to get channel members channel_id=%v err=%v"
-  },
-  {
-    "id": "api.post.get_out_of_channel_mentions.retrieve_profiles.error",
-    "translation": "Failed to retrieve user profiles team_id=%v, err=%v"
-  },
   {
     "id": "api.post.get_out_of_channel_mentions.regex.error",
     "translation": "Failed to compile @mention regex user_id=%v, err=%v"
@@ -711,6 +703,14 @@
     "id": "api.post.handle_post_events_and_forget.channel.error",
     "translation": "Encountered error getting channel, channel_id=%s, err=%v"
   },
+  {
+    "id": "api.post.handle_post_events_and_forget.members.error",
+    "translation": "Failed to get channel members channel_id=%v err=%v"
+  },
+  {
+    "id": "api.post.handle_post_events_and_forget.profiles.error",
+    "translation": "Failed to retrieve user profiles team_id=%v, err=%v"
+  },
   {
     "id": "api.post.handle_post_events_and_forget.team.error",
     "translation": "Encountered error getting team, team_id=%s, err=%v"
@@ -751,10 +751,6 @@
     "id": "api.post.make_direct_channel_visible.update_pref.error",
     "translation": "Failed to update direct channel preference user_id=%v other_user_id=%v err=%v"
   },
-  {
-    "id": "api.post.send_notifications_and_forget.members.error",
-    "translation": "Failed to get channel members channel_id=%v err=%v"
-  },
   {
     "id": "api.post.send_notifications_and_forget.mention_body",
     "translation": "You have one new mention."
@@ -787,10 +783,6 @@
     "id": "api.post.send_notifications_and_forget.push_notification.error",
     "translation": "Failed to send push notificationid=%v, err=%v"
   },
-  {
-    "id": "api.post.send_notifications_and_forget.retrive_profiles.error",
-    "translation": "Failed to retrieve user profiles team_id=%v, err=%v"
-  },
   {
     "id": "api.post.send_notifications_and_forget.send.error",
     "translation": "Failed to send mention email successfully email=%v err=%v"

i18n/es.json

@@ -683,14 +683,6 @@
     "id": "api.post.delete_post.permissions.app_error",
     "translation": "No tienes los permisos apropiados"
   },
-  {
-    "id": "api.post.get_out_of_channel_mentions.retrieve_members.error",
-    "translation": "Falla al obtener los miembros del canal channel_id=%v err=%v"
-  },
-  {
-    "id": "api.post.get_out_of_channel_mentions.retrieve_profiles.error",
-    "translation": "Falla al recuperar los perfiles de usuario team_id=%v, err=%v"
-  },
   {
     "id": "api.post.get_post.permissions.app_error",
     "translation": "No tienes los permisos apropiados"
@@ -699,6 +691,14 @@
     "id": "api.post.handle_post_events_and_forget.channel.error",
     "translation": "Se encontró un error obteniendo el canal, channel_id=%s, err=%v"
   },
+  {
+    "id": "api.post.handle_post_events_and_forget.members.error",
+    "translation": "Falla al obtener los miembros del canal channel_id=%v err=%v"
+  },
+  {
+    "id": "api.post.handle_post_events_and_forget.profiles.error",
+    "translation": "Falla al recuperar los perfiles de usuario team_id=%v, err=%v"
+  },
   {
     "id": "api.post.handle_post_events_and_forget.team.error",
     "translation": "Se encontró un error obteniendo el equipo, team_id=%s, err=%v"
@@ -739,10 +739,6 @@
     "id": "api.post.make_direct_channel_visible.update_pref.error",
     "translation": "Falla al actualizar las preferencias del canal directo user_id=%v other_user_id=%v err=%v"
   },
-  {
-    "id": "api.post.send_notifications_and_forget.members.error",
-    "translation": "Falla al obtener los miembros del canal channel_id=%v err=%v"
-  },
   {
     "id": "api.post.send_notifications_and_forget.mention_body",
     "translation": "Tienes una mención nueva."
@@ -775,10 +771,6 @@
     "id": "api.post.send_notifications_and_forget.push_notification.error",
     "translation": "Falló el envio de la notificación push notificationid=%v, err=%v"
   },
-  {
-    "id": "api.post.send_notifications_and_forget.retrive_profiles.error",
-    "translation": "Falla al recuperar los perfiles de usuario team_id=%v, err=%v"
-  },
   {
     "id": "api.post.send_notifications_and_forget.send.error",
     "translation": "Falla al enviar el correo con la mención satisfactoriamente email=%v err=%v"