From 18ee37586027e5672446a6f23c05a8ccb2b35896 Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Sat, 21 Oct 2017 01:38:26 +0800 Subject: [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 --- api/post_test.go | 10 ++++++- api4/post_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++- app/notification.go | 33 ++++++++++++++++++++--- app/post.go | 4 +++ i18n/en.json | 12 +++++++-- model/post.go | 13 +++++++++ model/post_test.go | 43 +++++++++++++++++++++++++++++ 7 files changed, 185 insertions(+), 8 deletions(-) diff --git a/api/post_test.go b/api/post_test.go index 8b5cbd454..901a7c6d0 100644 --- 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" diff --git a/api4/post_test.go b/api4/post_test.go index 3a410f2b4..05c5c3d97 100644 --- 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) diff --git a/app/notification.go b/app/notification.go index e11218faa..bbd305b05 100644 --- 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, }, ) diff --git a/app/post.go b/app/post.go index f12866217..4d91e8122 100644 --- 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 diff --git a/i18n/en.json b/i18n/en.json index 53cbc6d99..7a8c8f4c1 100644 --- 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", diff --git a/model/post.go b/model/post.go index 523a97c73..a5ae0b82a 100644 --- 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() diff --git a/model/post_test.go b/model/post_test.go index ced84f26b..846c8c775 100644 --- 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") + } +} -- cgit v1.2.3-1-g7c22