[PLT-7362] Option to add user to channel if mentioned user is not currently in the channel (#7619)

* Option to add user to channel if mentioned user is not currently in the channel * instead of link from server, just add component on client side to add channel member * change implementation using post.props * do clean up and add test * sanitize post.props['add_channel_member'] on post creation * move sanitize to app.CreatePost and also apply to app.UpdatePost
2017-10-21 01:38:26 +08:00
--- a/api/post_test.go
+++ b/api/post_test.go
@@ -39,7 +39,7 @@ func TestCreatePost(t *testing.T) {
 	adminUser := th.CreateUser(th.SystemAdminClient)
 	th.LinkUserToTeam(adminUser, adminTeam)

-	post1 := &model.Post{ChannelId: channel1.Id, Message: "#hashtag a" + model.NewId() + "a"}
+	post1 := &model.Post{ChannelId: channel1.Id, Message: "#hashtag a" + model.NewId() + "a", Props: model.StringInterface{model.PROPS_ADD_CHANNEL_MEMBER: "no good"}}
 	rpost1, err := Client.CreatePost(post1)
 	if err != nil {
 		t.Fatal(err)
@@ -61,6 +61,10 @@ func TestCreatePost(t *testing.T) {
 		t.Fatal("Newly craeted post shouldn't have EditAt set")
 	}

+	if rpost1.Data.(*model.Post).Props[model.PROPS_ADD_CHANNEL_MEMBER] != nil {
+		t.Fatal("newly created post shouldn't have Props['add_channel_member'] set")
+	}
+
 	_, err = Client.CreatePost(&model.Post{ChannelId: channel1.Id, Message: "#hashtag a" + model.NewId() + "a", Type: model.POST_SYSTEM_GENERIC})
 	if err == nil {
 		t.Fatal("should have failed - bad post type")
@@ -433,6 +437,7 @@ func TestUpdatePost(t *testing.T) {

 	msg2 := "zz" + model.NewId() + " update post 1"
 	rpost2.Data.(*model.Post).Message = msg2
+	rpost2.Data.(*model.Post).Props[model.PROPS_ADD_CHANNEL_MEMBER] = "no good"
 	if rupost2, err := Client.UpdatePost(rpost2.Data.(*model.Post)); err != nil {
 		t.Fatal(err)
 	} else {
@@ -442,6 +447,9 @@ func TestUpdatePost(t *testing.T) {
 		if rupost2.Data.(*model.Post).EditAt == 0 {
 			t.Fatal("EditAt not updated for post")
 		}
+		if rupost2.Data.(*model.Post).Props[model.PROPS_ADD_CHANNEL_MEMBER] != nil {
+			t.Fatal("failed to sanitize Props['add_channel_member'], should be nil")
+		}
 	}

 	msg1 := "#hashtag a" + model.NewId() + " update post 2"
--- a/api4/post_test.go
+++ b/api4/post_test.go
@@ -24,7 +24,7 @@ func TestCreatePost(t *testing.T) {
 	defer th.TearDown()
 	Client := th.Client

-	post := &model.Post{ChannelId: th.BasicChannel.Id, Message: "#hashtag a" + model.NewId() + "a"}
+	post := &model.Post{ChannelId: th.BasicChannel.Id, Message: "#hashtag a" + model.NewId() + "a", Props: model.StringInterface{model.PROPS_ADD_CHANNEL_MEMBER: "no good"}}
 	rpost, resp := Client.CreatePost(post)
 	CheckNoError(t, resp)
 	CheckCreatedStatus(t, resp)
@@ -45,6 +45,10 @@ func TestCreatePost(t *testing.T) {
 		t.Fatal("newly created post shouldn't have EditAt set")
 	}

+	if rpost.Props[model.PROPS_ADD_CHANNEL_MEMBER] != nil {
+		t.Fatal("newly created post shouldn't have Props['add_channel_member'] set")
+	}
+
 	post.RootId = rpost.Id
 	post.ParentId = rpost.Id
 	_, resp = Client.CreatePost(post)
@@ -371,6 +375,73 @@ func TestCreatePostAll(t *testing.T) {
 	CheckForbiddenStatus(t, resp)
 }

+func TestCreatePostSendOutOfChannelMentions(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+	Client := th.Client
+
+	WebSocketClient, err := th.CreateWebSocketClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+	WebSocketClient.Listen()
+
+	inChannelUser := th.CreateUser()
+	th.LinkUserToTeam(inChannelUser, th.BasicTeam)
+	th.App.AddUserToChannel(inChannelUser, th.BasicChannel)
+
+	post1 := &model.Post{ChannelId: th.BasicChannel.Id, Message: "@" + inChannelUser.Username}
+	_, resp := Client.CreatePost(post1)
+	CheckNoError(t, resp)
+	CheckCreatedStatus(t, resp)
+
+	timeout := time.After(300 * time.Millisecond)
+	waiting := true
+	for waiting {
+		select {
+		case event := <-WebSocketClient.EventChannel:
+			if event.Event == model.WEBSOCKET_EVENT_EPHEMERAL_MESSAGE {
+				t.Fatal("should not have ephemeral message event")
+			}
+
+		case <-timeout:
+			waiting = false
+		}
+	}
+
+	outOfChannelUser := th.CreateUser()
+	th.LinkUserToTeam(outOfChannelUser, th.BasicTeam)
+
+	post2 := &model.Post{ChannelId: th.BasicChannel.Id, Message: "@" + outOfChannelUser.Username}
+	_, resp = Client.CreatePost(post2)
+	CheckNoError(t, resp)
+	CheckCreatedStatus(t, resp)
+
+	timeout = time.After(300 * time.Millisecond)
+	waiting = true
+	for waiting {
+		select {
+		case event := <-WebSocketClient.EventChannel:
+			if event.Event != model.WEBSOCKET_EVENT_EPHEMERAL_MESSAGE {
+				// Ignore any other events
+				continue
+			}
+
+			wpost := model.PostFromJson(strings.NewReader(event.Data["post"].(string)))
+			if acm, ok := wpost.Props[model.PROPS_ADD_CHANNEL_MEMBER].(map[string]interface{}); !ok {
+				t.Fatal("should have received ephemeral post with 'add_channel_member' in props")
+			} else {
+				if acm["post_id"] == nil || acm["user_ids"] == nil || acm["usernames"] == nil {
+					t.Fatal("should not be nil")
+				}
+			}
+			waiting = false
+		case <-timeout:
+			t.Fatal("timed out waiting for ephemeral message event")
+		}
+	}
+}
+
 func TestUpdatePost(t *testing.T) {
 	th := Setup().InitBasic().InitSystemAdmin()
 	defer th.TearDown()
@@ -421,6 +492,7 @@ func TestUpdatePost(t *testing.T) {

 	msg1 := "#hashtag a" + model.NewId() + " update post again"
 	rpost.Message = msg1
+	rpost.Props[model.PROPS_ADD_CHANNEL_MEMBER] = "no good"
 	rrupost, resp := Client.UpdatePost(rpost.Id, rpost)
 	CheckNoError(t, resp)

@@ -428,6 +500,10 @@ func TestUpdatePost(t *testing.T) {
 		t.Fatal("failed to updates")
 	}

+	if rrupost.Props[model.PROPS_ADD_CHANNEL_MEMBER] != nil {
+		t.Fatal("failed to sanitize Props['add_channel_member'], should be nil")
+	}
+
 	rpost2, err := th.App.CreatePost(&model.Post{ChannelId: channel.Id, Message: "zz" + model.NewId() + "a", Type: model.POST_JOIN_LEAVE, UserId: th.BasicUser.Id}, channel, false)
 	if err != nil {
 		t.Fatal(err)
--- a/app/notification.go
+++ b/app/notification.go
@@ -94,7 +94,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 				outOfChannelMentions := result.Data.([]*model.User)
 				if channel.Type != model.CHANNEL_GROUP {
 					a.Go(func() {
-						a.sendOutOfChannelMentions(sender, post, team.Id, outOfChannelMentions)
+						a.sendOutOfChannelMentions(sender, post, channel.Type, outOfChannelMentions)
 					})
 				}
 			}
@@ -723,7 +723,7 @@ func (a *App) getMobileAppSessions(userId string) ([]*model.Session, *model.AppE
 	}
 }

-func (a *App) sendOutOfChannelMentions(sender *model.User, post *model.Post, teamId string, users []*model.User) *model.AppError {
+func (a *App) sendOutOfChannelMentions(sender *model.User, post *model.Post, channelType string, users []*model.User) *model.AppError {
 	if len(users) == 0 {
 		return nil
 	}
@@ -734,26 +734,51 @@ func (a *App) sendOutOfChannelMentions(sender *model.User, post *model.Post, tea
 	}
 	sort.Strings(usernames)

+	var userIds []string
+	for _, user := range users {
+		userIds = append(userIds, user.Id)
+	}
+
 	T := utils.GetUserTranslations(sender.Locale)

+	var localePhrase string
+	if channelType == model.CHANNEL_OPEN {
+		localePhrase = T("api.post.check_for_out_of_channel_mentions.link.public")
+	} else if channelType == model.CHANNEL_PRIVATE {
+		localePhrase = T("api.post.check_for_out_of_channel_mentions.link.private")
+	}
+
+	ephemeralPostId := model.NewId()
 	var message string
-	if len(usernames) == 1 {
+	if len(users) == 1 {
 		message = T("api.post.check_for_out_of_channel_mentions.message.one", map[string]interface{}{
 			"Username": usernames[0],
+			"Phrase":   localePhrase,
 		})
 	} else {
 		message = T("api.post.check_for_out_of_channel_mentions.message.multiple", map[string]interface{}{
-			"Usernames":    strings.Join(usernames[:len(usernames)-1], ", "),
+			"Usernames":    strings.Join(usernames[:len(usernames)-1], ", @"),
 			"LastUsername": usernames[len(usernames)-1],
+			"Phrase":       localePhrase,
 		})
 	}

+	props := model.StringInterface{
+		model.PROPS_ADD_CHANNEL_MEMBER: model.StringInterface{
+			"post_id":   ephemeralPostId,
+			"usernames": usernames,
+			"user_ids":  userIds,
+		},
+	}
+
 	a.SendEphemeralPost(
 		post.UserId,
 		&model.Post{
+			Id:        ephemeralPostId,
 			ChannelId: post.ChannelId,
 			Message:   message,
 			CreateAt:  post.CreateAt + 1,
+			Props:     props,
 		},
 	)

--- a/app/post.go
+++ b/app/post.go
@@ -104,6 +104,8 @@ func (a *App) CreatePostMissingChannel(post *model.Post, triggerWebhooks bool) (
 }

 func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhooks bool) (*model.Post, *model.AppError) {
+	post.SanitizeProps()
+
 	var pchan store.StoreChannel
 	if len(post.RootId) > 0 {
 		pchan = a.Srv.Store.Post().Get(post.RootId)
@@ -273,6 +275,8 @@ func (a *App) SendEphemeralPost(userId string, post *model.Post) *model.Post {
 }

 func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) {
+	post.SanitizeProps()
+
 	var oldPost *model.Post
 	if result := <-a.Srv.Store.Post().Get(post.Id); result.Err != nil {
 		return nil, result.Err
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -1553,13 +1553,21 @@
    "id": "api.plugin.upload.no_file.app_error",
    "translation": "Missing file in multipart/form request"
  },
+  {
+    "id": "api.post.check_for_out_of_channel_mentions.link.public",
+    "translation": "add them to the channel"
+  },
+  {
+    "id": "api.post.check_for_out_of_channel_mentions.link.private",
+    "translation": "add them to this private channel"
+  },
  {
    "id": "api.post.check_for_out_of_channel_mentions.message.multiple",
-    "translation": "{{.Usernames}} and {{.LastUsername}} were mentioned, but they did not receive notifications because they do not belong to this channel."
+    "translation": "@{{.Usernames}} and @{{.LastUsername}} were mentioned but they are not in the channel. Would you like to {{.Phrase}}? They will have access to all message history."
  },
  {
    "id": "api.post.check_for_out_of_channel_mentions.message.one",
-    "translation": "{{.Username}} was mentioned, but they did not receive a notification because they do not belong to this channel."
+    "translation": "@{{.Username}} was mentioned but they are not in the channel. Would you like to {{.Phrase}}? They will have access to all message history."
  },
  {
    "id": "api.post.create_post.attach_files.error",
--- a/model/post.go
+++ b/model/post.go
@@ -33,6 +33,7 @@ const (
 	POST_MESSAGE_MAX_RUNES     = 4000
 	POST_PROPS_MAX_RUNES       = 8000
 	POST_CUSTOM_TYPE_PREFIX    = "custom_"
+	PROPS_ADD_CHANNEL_MEMBER   = "add_channel_member"
 )

 type Post struct {
@@ -188,6 +189,18 @@ func (o *Post) IsValid() *AppError {
 	return nil
 }

+func (o *Post) SanitizeProps() {
+	membersToSanitize := []string{
+		PROPS_ADD_CHANNEL_MEMBER,
+	}
+
+	for _, member := range membersToSanitize {
+		if _, ok := o.Props[member]; ok {
+			delete(o.Props, member)
+		}
+	}
+}
+
 func (o *Post) PreSave() {
 	if o.Id == "" {
 		o.Id = NewId()
--- a/model/post_test.go
+++ b/model/post_test.go
@@ -123,3 +123,46 @@ func TestPostIsSystemMessage(t *testing.T) {
 		t.Fatalf("TestPostIsSystemMessage failed, expected post2.IsSystemMessage() to be true")
 	}
 }
+
+func TestPostSanitizeProps(t *testing.T) {
+	post1 := &Post{
+		Message: "test",
+	}
+
+	post1.SanitizeProps()
+
+	if post1.Props[PROPS_ADD_CHANNEL_MEMBER] != nil {
+		t.Fatal("should be nil")
+	}
+
+	post2 := &Post{
+		Message: "test",
+		Props: StringInterface{
+			PROPS_ADD_CHANNEL_MEMBER: "test",
+		},
+	}
+
+	post2.SanitizeProps()
+
+	if post2.Props[PROPS_ADD_CHANNEL_MEMBER] != nil {
+		t.Fatal("should be nil")
+	}
+
+	post3 := &Post{
+		Message: "test",
+		Props: StringInterface{
+			PROPS_ADD_CHANNEL_MEMBER: "no good",
+			"attachments":            "good",
+		},
+	}
+
+	post3.SanitizeProps()
+
+	if post3.Props[PROPS_ADD_CHANNEL_MEMBER] != nil {
+		t.Fatal("should be nil")
+	}
+
+	if post3.Props["attachments"] == nil {
+		t.Fatal("should not be nil")
+	}
+}