summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJesse Hallam <jesse.hallam@gmail.com>2018-06-26 16:46:58 -0400
committerJoram Wilander <jwawilander@gmail.com>2018-06-26 16:46:57 -0400
commit2d7cd02abcd62ffd60fe3c6e16e5189169de349e (patch)
tree93fde76e72e39577ffb70a6ff0922e5a81ffc973
parent164e030d33b03cab347ddcdf064615cb9e144317 (diff)
downloadchat-2d7cd02abcd62ffd60fe3c6e16e5189169de349e.tar.gz
chat-2d7cd02abcd62ffd60fe3c6e16e5189169de349e.tar.bz2
chat-2d7cd02abcd62ffd60fe3c6e16e5189169de349e.zip
MM-10833: send down computed channel props (#8953)
* MM-10833: send down computed channel props This allows channel headers to reference channel mentions for a client that doesn't already know about the channels in question. We intentionally don't send down the props for the autocomplete and search endpoints since they aren't used in that context, and would add unnecessary overhead. * update channel props on patch * revert to treating channel purpose as plaintext
-rw-r--r--api4/channel.go107
-rw-r--r--app/channel.go64
-rw-r--r--app/channel_test.go206
-rw-r--r--model/channel.go43
-rw-r--r--model/channel_mentions.go28
-rw-r--r--model/post.go17
6 files changed, 411 insertions, 54 deletions
diff --git a/api4/channel.go b/api4/channel.go
index b2c920ddb..cb9112677 100644
--- a/api4/channel.go
+++ b/api4/channel.go
@@ -209,13 +209,20 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
- if rchannel, err := c.App.PatchChannel(oldChannel, patch, c.Session.UserId); err != nil {
+ rchannel, err := c.App.PatchChannel(oldChannel, patch, c.Session.UserId)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
+ err = c.App.FillInChannelProps(rchannel)
+ if err != nil {
c.Err = err
return
- } else {
- c.LogAudit("")
- w.Write([]byte(rchannel.ToJson()))
}
+
+ c.LogAudit("")
+ w.Write([]byte(rchannel.ToJson()))
}
func restoreChannel(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -361,6 +368,12 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) {
}
}
+ err = c.App.FillInChannelProps(channel)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
w.Write([]byte(channel.ToJson()))
}
@@ -444,13 +457,19 @@ func getPublicChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request
return
}
- if channels, err := c.App.GetPublicChannelsForTeam(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage); err != nil {
+ channels, err := c.App.GetPublicChannelsForTeam(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage)
+ if err != nil {
c.Err = err
return
- } else {
- w.Write([]byte(channels.ToJson()))
+ }
+
+ err = c.App.FillInChannelsProps(channels)
+ if err != nil {
+ c.Err = err
return
}
+
+ w.Write([]byte(channels.ToJson()))
}
func getDeletedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -464,13 +483,19 @@ func getDeletedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Reques
return
}
- if channels, err := c.App.GetDeletedChannels(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage); err != nil {
+ channels, err := c.App.GetDeletedChannels(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage)
+ if err != nil {
c.Err = err
return
- } else {
- w.Write([]byte(channels.ToJson()))
+ }
+
+ err = c.App.FillInChannelsProps(channels)
+ if err != nil {
+ c.Err = err
return
}
+
+ w.Write([]byte(channels.ToJson()))
}
func getPublicChannelsByIdsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -497,12 +522,19 @@ func getPublicChannelsByIdsForTeam(c *Context, w http.ResponseWriter, r *http.Re
return
}
- if channels, err := c.App.GetPublicChannelsByIdsForTeam(c.Params.TeamId, channelIds); err != nil {
+ channels, err := c.App.GetPublicChannelsByIdsForTeam(c.Params.TeamId, channelIds)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
+ err = c.App.FillInChannelsProps(channels)
+ if err != nil {
c.Err = err
return
- } else {
- w.Write([]byte(channels.ToJson()))
}
+
+ w.Write([]byte(channels.ToJson()))
}
func getChannelsForTeamForUser(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -521,15 +553,24 @@ func getChannelsForTeamForUser(c *Context, w http.ResponseWriter, r *http.Reques
return
}
- if channels, err := c.App.GetChannelsForUser(c.Params.TeamId, c.Params.UserId); err != nil {
+ channels, err := c.App.GetChannelsForUser(c.Params.TeamId, c.Params.UserId)
+ if err != nil {
c.Err = err
return
- } else if c.HandleEtag(channels.Etag(), "Get Channels", w, r) {
+ }
+
+ if c.HandleEtag(channels.Etag(), "Get Channels", w, r) {
return
- } else {
- w.Header().Set(model.HEADER_ETAG_SERVER, channels.Etag())
- w.Write([]byte(channels.ToJson()))
}
+
+ err = c.App.FillInChannelsProps(channels)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
+ w.Header().Set(model.HEADER_ETAG_SERVER, channels.Etag())
+ w.Write([]byte(channels.ToJson()))
}
func autocompleteChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -545,12 +586,15 @@ func autocompleteChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Requ
name := r.URL.Query().Get("name")
- if channels, err := c.App.AutocompleteChannels(c.Params.TeamId, name); err != nil {
+ channels, err := c.App.AutocompleteChannels(c.Params.TeamId, name)
+ if err != nil {
c.Err = err
return
- } else {
- w.Write([]byte(channels.ToJson()))
}
+
+ // Don't fill in channels props, since unused by client and potentially expensive.
+
+ w.Write([]byte(channels.ToJson()))
}
func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -570,12 +614,15 @@ func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
- if channels, err := c.App.SearchChannels(c.Params.TeamId, props.Term); err != nil {
+ channels, err := c.App.SearchChannels(c.Params.TeamId, props.Term)
+ if err != nil {
c.Err = err
return
- } else {
- w.Write([]byte(channels.ToJson()))
}
+
+ // Don't fill in channels props, since unused by client and potentially expensive.
+
+ w.Write([]byte(channels.ToJson()))
}
func deleteChannel(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -638,6 +685,12 @@ func getChannelByName(c *Context, w http.ResponseWriter, r *http.Request) {
}
}
+ err = c.App.FillInChannelProps(channel)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
w.Write([]byte(channel.ToJson()))
}
@@ -660,6 +713,12 @@ func getChannelByNameForTeamName(c *Context, w http.ResponseWriter, r *http.Requ
return
}
+ err = c.App.FillInChannelProps(channel)
+ if err != nil {
+ c.Err = err
+ return
+ }
+
w.Write([]byte(channel.ToJson()))
}
diff --git a/app/channel.go b/app/channel.go
index 7394d813b..eee27a6de 100644
--- a/app/channel.go
+++ b/app/channel.go
@@ -1591,3 +1591,67 @@ func (a *App) ToggleMuteChannel(channelId string, userId string) *model.ChannelM
a.Srv.Store.Channel().UpdateMember(member)
return member
}
+
+func (a *App) FillInChannelProps(channel *model.Channel) *model.AppError {
+ return a.FillInChannelsProps(&model.ChannelList{channel})
+}
+
+func (a *App) FillInChannelsProps(channelList *model.ChannelList) *model.AppError {
+ // Group the channels by team and call GetChannelsByNames just once per team.
+ channelsByTeam := make(map[string]model.ChannelList)
+ for _, channel := range *channelList {
+ channelsByTeam[channel.TeamId] = append(channelsByTeam[channel.TeamId], channel)
+ }
+
+ for teamId, channelList := range channelsByTeam {
+ allChannelMentions := make(map[string]bool)
+ channelMentions := make(map[*model.Channel][]string, len(channelList))
+
+ // Collect mentions across the channels so as to query just once for this team.
+ for _, channel := range channelList {
+ channelMentions[channel] = model.ChannelMentions(channel.Header)
+
+ for _, channelMention := range channelMentions[channel] {
+ allChannelMentions[channelMention] = true
+ }
+ }
+
+ allChannelMentionNames := make([]string, 0, len(allChannelMentions))
+ for channelName := range allChannelMentions {
+ allChannelMentionNames = append(allChannelMentionNames, channelName)
+ }
+
+ if len(allChannelMentionNames) > 0 {
+ mentionedChannels, err := a.GetChannelsByNames(allChannelMentionNames, teamId)
+ if err != nil {
+ return err
+ }
+
+ mentionedChannelsByName := make(map[string]*model.Channel)
+ for _, channel := range mentionedChannels {
+ mentionedChannelsByName[channel.Name] = channel
+ }
+
+ for _, channel := range channelList {
+ channelMentionsProp := make(map[string]interface{}, len(channelMentions[channel]))
+ for _, channelMention := range channelMentions[channel] {
+ if mentioned, ok := mentionedChannelsByName[channelMention]; ok {
+ if mentioned.Type == model.CHANNEL_OPEN {
+ channelMentionsProp[mentioned.Name] = map[string]interface{}{
+ "display_name": mentioned.DisplayName,
+ }
+ }
+ }
+ }
+
+ if len(channelMentionsProp) > 0 {
+ channel.AddProp("channel_mentions", channelMentionsProp)
+ } else if channel.Props != nil {
+ delete(channel.Props, "channel_mentions")
+ }
+ }
+ }
+ }
+
+ return nil
+}
diff --git a/app/channel_test.go b/app/channel_test.go
index 336d9b25b..4e6aaaf52 100644
--- a/app/channel_test.go
+++ b/app/channel_test.go
@@ -9,6 +9,7 @@ import (
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/store"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestPermanentDeleteChannel(t *testing.T) {
@@ -399,3 +400,208 @@ func TestAppUpdateChannelScheme(t *testing.T) {
t.Fatal("Wrong Channel SchemeId")
}
}
+
+func TestFillInChannelProps(t *testing.T) {
+ th := Setup().InitBasic()
+ defer th.TearDown()
+
+ channelPublic1, err := th.App.CreateChannel(&model.Channel{DisplayName: "Public 1", Name: "public1", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false)
+ require.Nil(t, err)
+ defer th.App.PermanentDeleteChannel(channelPublic1)
+
+ channelPublic2, err := th.App.CreateChannel(&model.Channel{DisplayName: "Public 2", Name: "public2", Type: model.CHANNEL_OPEN, TeamId: th.BasicTeam.Id}, false)
+ require.Nil(t, err)
+ defer th.App.PermanentDeleteChannel(channelPublic2)
+
+ channelPrivate, err := th.App.CreateChannel(&model.Channel{DisplayName: "Private", Name: "private", Type: model.CHANNEL_PRIVATE, TeamId: th.BasicTeam.Id}, false)
+ require.Nil(t, err)
+ defer th.App.PermanentDeleteChannel(channelPrivate)
+
+ otherTeamId := model.NewId()
+ otherTeam := &model.Team{
+ DisplayName: "dn_" + otherTeamId,
+ Name: "name" + otherTeamId,
+ Email: "success+" + otherTeamId + "@simulator.amazonses.com",
+ Type: model.TEAM_OPEN,
+ }
+ otherTeam, err = th.App.CreateTeam(otherTeam)
+ require.Nil(t, err)
+ defer th.App.PermanentDeleteTeam(otherTeam)
+
+ channelOtherTeam, err := th.App.CreateChannel(&model.Channel{DisplayName: "Other Team Channel", Name: "other-team", Type: model.CHANNEL_OPEN, TeamId: otherTeam.Id}, false)
+ require.Nil(t, err)
+ defer th.App.PermanentDeleteChannel(channelOtherTeam)
+
+ // Note that purpose is intentionally plaintext below.
+
+ t.Run("single channels", func(t *testing.T) {
+ testCases := []struct {
+ Description string
+ Channel *model.Channel
+ ExpectedChannelProps map[string]interface{}
+ }{
+ {
+ "channel on basic team without references",
+ &model.Channel{
+ TeamId: th.BasicTeam.Id,
+ Header: "No references",
+ Purpose: "No references",
+ },
+ nil,
+ },
+ {
+ "channel on basic team",
+ &model.Channel{
+ TeamId: th.BasicTeam.Id,
+ Header: "~public1, ~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ map[string]interface{}{
+ "channel_mentions": map[string]interface{}{
+ "public1": map[string]interface{}{
+ "display_name": "Public 1",
+ },
+ },
+ },
+ },
+ {
+ "channel on other team",
+ &model.Channel{
+ TeamId: otherTeam.Id,
+ Header: "~public1, ~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ map[string]interface{}{
+ "channel_mentions": map[string]interface{}{
+ "other-team": map[string]interface{}{
+ "display_name": "Other Team Channel",
+ },
+ },
+ },
+ },
+ }
+
+ for _, testCase := range testCases {
+ t.Run(testCase.Description, func(t *testing.T) {
+ err = th.App.FillInChannelProps(testCase.Channel)
+ require.Nil(t, err)
+
+ assert.Equal(t, testCase.ExpectedChannelProps, testCase.Channel.Props)
+ })
+ }
+ })
+
+ t.Run("multiple channels", func(t *testing.T) {
+ testCases := []struct {
+ Description string
+ Channels *model.ChannelList
+ ExpectedChannelProps map[string]interface{}
+ }{
+ {
+ "single channel on basic team",
+ &model.ChannelList{
+ {
+ Name: "test",
+ TeamId: th.BasicTeam.Id,
+ Header: "~public1, ~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ },
+ map[string]interface{}{
+ "test": map[string]interface{}{
+ "channel_mentions": map[string]interface{}{
+ "public1": map[string]interface{}{
+ "display_name": "Public 1",
+ },
+ },
+ },
+ },
+ },
+ {
+ "multiple channels on basic team",
+ &model.ChannelList{
+ {
+ Name: "test",
+ TeamId: th.BasicTeam.Id,
+ Header: "~public1, ~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ {
+ Name: "test2",
+ TeamId: th.BasicTeam.Id,
+ Header: "~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ {
+ Name: "test3",
+ TeamId: th.BasicTeam.Id,
+ Header: "No references",
+ Purpose: "No references",
+ },
+ },
+ map[string]interface{}{
+ "test": map[string]interface{}{
+ "channel_mentions": map[string]interface{}{
+ "public1": map[string]interface{}{
+ "display_name": "Public 1",
+ },
+ },
+ },
+ "test2": map[string]interface{}(nil),
+ "test3": map[string]interface{}(nil),
+ },
+ },
+ {
+ "multiple channels across teams",
+ &model.ChannelList{
+ {
+ Name: "test",
+ TeamId: th.BasicTeam.Id,
+ Header: "~public1, ~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ {
+ Name: "test2",
+ TeamId: otherTeam.Id,
+ Header: "~private, ~other-team",
+ Purpose: "~public2, ~private, ~other-team",
+ },
+ {
+ Name: "test3",
+ TeamId: th.BasicTeam.Id,
+ Header: "No references",
+ Purpose: "No references",
+ },
+ },
+ map[string]interface{}{
+ "test": map[string]interface{}{
+ "channel_mentions": map[string]interface{}{
+ "public1": map[string]interface{}{
+ "display_name": "Public 1",
+ },
+ },
+ },
+ "test2": map[string]interface{}{
+ "channel_mentions": map[string]interface{}{
+ "other-team": map[string]interface{}{
+ "display_name": "Other Team Channel",
+ },
+ },
+ },
+ "test3": map[string]interface{}(nil),
+ },
+ },
+ }
+
+ for _, testCase := range testCases {
+ t.Run(testCase.Description, func(t *testing.T) {
+ err = th.App.FillInChannelsProps(testCase.Channels)
+ require.Nil(t, err)
+
+ for _, channel := range *testCase.Channels {
+ assert.Equal(t, testCase.ExpectedChannelProps[channel.Name], channel.Props)
+ }
+ })
+ }
+ })
+}
diff --git a/model/channel.go b/model/channel.go
index 5617240e6..7a57496ae 100644
--- a/model/channel.go
+++ b/model/channel.go
@@ -32,21 +32,22 @@ const (
)
type Channel struct {
- Id string `json:"id"`
- CreateAt int64 `json:"create_at"`
- UpdateAt int64 `json:"update_at"`
- DeleteAt int64 `json:"delete_at"`
- TeamId string `json:"team_id"`
- Type string `json:"type"`
- DisplayName string `json:"display_name"`
- Name string `json:"name"`
- Header string `json:"header"`
- Purpose string `json:"purpose"`
- LastPostAt int64 `json:"last_post_at"`
- TotalMsgCount int64 `json:"total_msg_count"`
- ExtraUpdateAt int64 `json:"extra_update_at"`
- CreatorId string `json:"creator_id"`
- SchemeId *string `json:"scheme_id"`
+ Id string `json:"id"`
+ CreateAt int64 `json:"create_at"`
+ UpdateAt int64 `json:"update_at"`
+ DeleteAt int64 `json:"delete_at"`
+ TeamId string `json:"team_id"`
+ Type string `json:"type"`
+ DisplayName string `json:"display_name"`
+ Name string `json:"name"`
+ Header string `json:"header"`
+ Purpose string `json:"purpose"`
+ LastPostAt int64 `json:"last_post_at"`
+ TotalMsgCount int64 `json:"total_msg_count"`
+ ExtraUpdateAt int64 `json:"extra_update_at"`
+ CreatorId string `json:"creator_id"`
+ SchemeId *string `json:"scheme_id"`
+ Props map[string]interface{} `json:"props" db:"-"`
}
type ChannelPatch struct {
@@ -163,6 +164,18 @@ func (o *Channel) Patch(patch *ChannelPatch) {
}
}
+func (o *Channel) MakeNonNil() {
+ if o.Props == nil {
+ o.Props = make(map[string]interface{})
+ }
+}
+
+func (o *Channel) AddProp(key string, value interface{}) {
+ o.MakeNonNil()
+
+ o.Props[key] = value
+}
+
func GetDMNameFromIds(userId1, userId2 string) string {
if userId1 > userId2 {
return userId2 + "__" + userId1
diff --git a/model/channel_mentions.go b/model/channel_mentions.go
new file mode 100644
index 000000000..795ec379c
--- /dev/null
+++ b/model/channel_mentions.go
@@ -0,0 +1,28 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package model
+
+import (
+ "regexp"
+ "strings"
+)
+
+var channelMentionRegexp = regexp.MustCompile(`\B~[a-zA-Z0-9\-_]+`)
+
+func ChannelMentions(message string) []string {
+ var names []string
+
+ if strings.Contains(message, "~") {
+ alreadyMentioned := make(map[string]bool)
+ for _, match := range channelMentionRegexp.FindAllString(message, -1) {
+ name := match[1:]
+ if !alreadyMentioned[name] {
+ names = append(names, name)
+ alreadyMentioned[name] = true
+ }
+ }
+ }
+
+ return names
+}
diff --git a/model/post.go b/model/post.go
index 3d7a31ab5..1dd0a4db6 100644
--- a/model/post.go
+++ b/model/post.go
@@ -7,7 +7,6 @@ import (
"encoding/json"
"io"
"net/http"
- "regexp"
"sort"
"strings"
"unicode/utf8"
@@ -343,20 +342,8 @@ func PostPatchFromJson(data io.Reader) *PostPatch {
return &post
}
-var channelMentionRegexp = regexp.MustCompile(`\B~[a-zA-Z0-9\-_]+`)
-
-func (o *Post) ChannelMentions() (names []string) {
- if strings.Contains(o.Message, "~") {
- alreadyMentioned := make(map[string]bool)
- for _, match := range channelMentionRegexp.FindAllString(o.Message, -1) {
- name := match[1:]
- if !alreadyMentioned[name] {
- names = append(names, name)
- alreadyMentioned[name] = true
- }
- }
- }
- return
+func (o *Post) ChannelMentions() []string {
+ return ChannelMentions(o.Message)
}
func (r *PostActionIntegrationRequest) ToJson() string {