From af8b914c6c64b26494fa3b7c09f7076553127cd2 Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Mon, 6 Jul 2020 12:34:29 +0530 Subject: [PATCH] MM-23596: Ability to list private channels for team (#14925) Summary: store, app, api and go driver support for listing private channels Ticket Link: https://mattermost.atlassian.net/browse/MM-23596 --- api4/channel.go | 27 ++++++++ api4/channel_local.go | 1 + api4/channel_test.go | 36 +++++++++++ app/app_iface.go | 1 + app/channel.go | 4 ++ app/channel_test.go | 33 ++++++++++ app/opentracing_layer.go | 22 +++++++ i18n/en.json | 4 ++ model/client4.go | 11 ++++ store/opentracing_layer.go | 18 ++++++ store/sqlstore/channel_store.go | 23 +++++++ store/store.go | 1 + store/storetest/channel_store.go | 93 +++++++++++++++++++++++++++ store/storetest/mocks/ChannelStore.go | 25 +++++++ store/timer_layer.go | 16 +++++ 15 files changed, 315 insertions(+) diff --git a/api4/channel.go b/api4/channel.go index 32c4efad9f..e662698959 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -27,6 +27,7 @@ func (api *API) InitChannel() { api.BaseRoutes.ChannelsForTeam.Handle("", api.ApiSessionRequired(getPublicChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelsForTeam.Handle("/deleted", api.ApiSessionRequired(getDeletedChannelsForTeam)).Methods("GET") + api.BaseRoutes.ChannelsForTeam.Handle("/private", api.ApiSessionRequired(getPrivateChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelsForTeam.Handle("/ids", api.ApiSessionRequired(getPublicChannelsByIdsForTeam)).Methods("POST") api.BaseRoutes.ChannelsForTeam.Handle("/search", api.ApiSessionRequiredDisableWhenBusy(searchChannelsForTeam)).Methods("POST") api.BaseRoutes.ChannelsForTeam.Handle("/search_archived", api.ApiSessionRequiredDisableWhenBusy(searchArchivedChannelsForTeam)).Methods("POST") @@ -763,6 +764,32 @@ func getDeletedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Reques w.Write([]byte(channels.ToJson())) } +func getPrivateChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireTeamId() + if c.Err != nil { + return + } + + if !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) { + c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) + return + } + + channels, err := c.App.GetPrivateChannelsForTeam(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage) + if err != nil { + c.Err = err + return + } + + 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) { c.RequireTeamId() if c.Err != nil { diff --git a/api4/channel_local.go b/api4/channel_local.go index 13a66b4826..d454527603 100644 --- a/api4/channel_local.go +++ b/api4/channel_local.go @@ -24,6 +24,7 @@ func (api *API) InitChannelLocal() { api.BaseRoutes.ChannelsForTeam.Handle("", api.ApiLocal(getPublicChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelsForTeam.Handle("/deleted", api.ApiLocal(getDeletedChannelsForTeam)).Methods("GET") + api.BaseRoutes.ChannelsForTeam.Handle("/private", api.ApiLocal(getPrivateChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelByName.Handle("", api.ApiLocal(getChannelByName)).Methods("GET") api.BaseRoutes.ChannelByNameForTeamName.Handle("", api.ApiLocal(getChannelByNameForTeamName)).Methods("GET") diff --git a/api4/channel_test.go b/api4/channel_test.go index d5baa680d3..b6ce78f0f4 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -735,6 +735,42 @@ func TestGetDeletedChannelsForTeam(t *testing.T) { require.Len(t, channels, 1, "should be one channel per page") } +func TestGetPrivateChannelsForTeam(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + team := th.BasicTeam + + // normal user + _, resp := th.Client.GetPrivateChannelsForTeam(team.Id, 0, 100, "") + CheckForbiddenStatus(t, resp) + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, c *model.Client4) { + channels, resp := c.GetPrivateChannelsForTeam(team.Id, 0, 100, "") + CheckNoError(t, resp) + // th.BasicPrivateChannel and th.BasicPrivateChannel2 + require.Len(t, channels, 2, "wrong number of private channels") + for _, c := range channels { + // check all channels included are private + require.Equal(t, model.CHANNEL_PRIVATE, c.Type, "should include private channels only") + } + + channels, resp = c.GetPrivateChannelsForTeam(team.Id, 0, 1, "") + CheckNoError(t, resp) + require.Len(t, channels, 1, "should be one channel per page") + + channels, resp = c.GetPrivateChannelsForTeam(team.Id, 1, 1, "") + CheckNoError(t, resp) + require.Len(t, channels, 1, "should be one channel per page") + + channels, resp = c.GetPrivateChannelsForTeam(team.Id, 10000, 100, "") + CheckNoError(t, resp) + require.Empty(t, channels, "should be no channel") + + _, resp = c.GetPrivateChannelsForTeam("junk", 0, 100, "") + CheckBadRequestStatus(t, resp) + }) +} + func TestGetPublicChannelsForTeam(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/app/app_iface.go b/app/app_iface.go index 14b293501e..aaeaf5d989 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -611,6 +611,7 @@ type AppIface interface { GetPreferenceByCategoryForUser(userId string, category string) (model.Preferences, *model.AppError) GetPreferencesForUser(userId string) (model.Preferences, *model.AppError) GetPrevPostIdFromPostList(postList *model.PostList) string + GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) (*model.ChannelList, *model.AppError) GetPublicChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) diff --git a/app/channel.go b/app/channel.go index cad4b44c71..712278d990 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1635,6 +1635,10 @@ func (a *App) GetPublicChannelsForTeam(teamId string, offset int, limit int) (*m return a.Srv().Store.Channel().GetPublicChannelsForTeam(teamId, offset, limit) } +func (a *App) GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + return a.Srv().Store.Channel().GetPrivateChannelsForTeam(teamId, offset, limit) +} + func (a *App) GetChannelMember(channelId string, userId string) (*model.ChannelMember, *model.AppError) { return a.Srv().Store.Channel().GetMember(channelId, userId) } diff --git a/app/channel_test.go b/app/channel_test.go index f09be78251..c262ae6961 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -924,6 +924,39 @@ func TestGetPublicChannelsForTeam(t *testing.T) { assert.ElementsMatch(t, expectedChannels, channels) } +func TestGetPrivateChannelsForTeam(t *testing.T) { + th := Setup(t) + team := th.CreateTeam() + defer th.TearDown() + + var expectedChannels []*model.Channel + for i := 0; i < 8; i++ { + channel := model.Channel{ + DisplayName: fmt.Sprintf("Private %v", i), + Name: fmt.Sprintf("private_%v", i), + Type: model.CHANNEL_PRIVATE, + TeamId: team.Id, + } + var rchannel *model.Channel + rchannel, err := th.App.CreateChannel(&channel, false) + require.Nil(t, err) + require.NotNil(t, rchannel) + defer th.App.PermanentDeleteChannel(rchannel) + + // Store the user ids for comparison later + expectedChannels = append(expectedChannels, rchannel) + } + + // Fetch private channels multipile times + channelList, err := th.App.GetPrivateChannelsForTeam(team.Id, 0, 5) + require.Nil(t, err) + channelList2, err := th.App.GetPrivateChannelsForTeam(team.Id, 5, 5) + require.Nil(t, err) + + channels := append(*channelList, *channelList2...) + assert.ElementsMatch(t, expectedChannels, channels) +} + func TestUpdateChannelMemberRolesChangingGuest(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/app/opentracing_layer.go b/app/opentracing_layer.go index f63c908d69..db9e0ae825 100644 --- a/app/opentracing_layer.go +++ b/app/opentracing_layer.go @@ -6987,6 +6987,28 @@ func (a *OpenTracingAppLayer) GetPrevPostIdFromPostList(postList *model.PostList return resultVar0 } +func (a *OpenTracingAppLayer) GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetPrivateChannelsForTeam") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetPrivateChannelsForTeam(teamId, offset, limit) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetProfileImage") diff --git a/i18n/en.json b/i18n/en.json index b492ad69bf..1dbda73d70 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6322,6 +6322,10 @@ "id": "store.sql_channel.get_pinnedpost_count.app_error", "translation": "Unable to get the channel pinned post count." }, + { + "id": "store.sql_channel.get_private_channels.get.app_error", + "translation": "Unable to get private channels." + }, { "id": "store.sql_channel.get_public_channels.get.app_error", "translation": "Unable to get public channels." diff --git a/model/client4.go b/model/client4.go index a2491c1799..a2b84a8c56 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2385,6 +2385,17 @@ func (c *Client4) GetPinnedPosts(channelId string, etag string) (*PostList, *Res return PostListFromJson(r.Body), BuildResponse(r) } +// GetPrivateChannelsForTeam returns a list of private channels based on the provided team id string. +func (c *Client4) GetPrivateChannelsForTeam(teamId string, page int, perPage int, etag string) ([]*Channel, *Response) { + query := fmt.Sprintf("/private?page=%v&per_page=%v", page, perPage) + r, err := c.DoApiGet(c.GetChannelsForTeamRoute(teamId)+query, etag) + if err != nil { + return nil, BuildErrorResponse(r, err) + } + defer closeBody(r) + return ChannelSliceFromJson(r.Body), BuildResponse(r) +} + // GetPublicChannelsForTeam returns a list of public channels based on the provided team id string. func (c *Client4) GetPublicChannelsForTeam(teamId string, page int, perPage int, etag string) ([]*Channel, *Response) { query := fmt.Sprintf("?page=%v&per_page=%v", page, perPage) diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index c70a6179f1..afb860a72e 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -1273,6 +1273,24 @@ func (s *OpenTracingLayerChannelStore) GetPinnedPosts(channelId string) (*model. return resultVar0, resultVar1 } +func (s *OpenTracingLayerChannelStore) GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetPrivateChannelsForTeam") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + resultVar0, resultVar1 := s.ChannelStore.GetPrivateChannelsForTeam(teamId, offset, limit) + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (s *OpenTracingLayerChannelStore) GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) (*model.ChannelList, *model.AppError) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetPublicChannelsByIdsForTeam") diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index f546ec8e8a..d841ad1e44 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1046,6 +1046,29 @@ func (s SqlChannelStore) GetMoreChannels(teamId string, userId string, offset in return channels, nil } +func (s SqlChannelStore) GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + channels := &model.ChannelList{} + + query := s.getQueryBuilder(). + Select("*"). + From("Channels"). + Where(sq.Eq{"Type": model.CHANNEL_PRIVATE, "TeamId": teamId, "DeleteAt": 0}). + OrderBy("DisplayName"). + Limit(uint64(limit)). + Offset(uint64(offset)) + + sql, args, err := query.ToSql() + if err != nil { + return nil, model.NewAppError("SqlChannelStore.GetPrivateChannelsForTeam", "store.sql_channel.get_private_channels.get.app_error", nil, "teamId="+teamId+", err="+err.Error(), http.StatusInternalServerError) + } + + _, err = s.GetReplica().Select(channels, sql, args...) + if err != nil { + return nil, model.NewAppError("SqlChannelStore.GetPrivateChannelsForTeam", "store.sql_channel.get_private_channels.get.app_error", nil, "teamId="+teamId+", err="+err.Error(), http.StatusInternalServerError) + } + return channels, nil +} + func (s SqlChannelStore) GetPublicChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { channels := &model.ChannelList{} _, err := s.GetReplica().Select(channels, ` diff --git a/store/store.go b/store/store.go index 92644d603d..de5070a353 100644 --- a/store/store.go +++ b/store/store.go @@ -156,6 +156,7 @@ type ChannelStore interface { GetAllChannels(page, perPage int, opts ChannelSearchOpts) (*model.ChannelListWithTeamData, error) GetAllChannelsCount(opts ChannelSearchOpts) (int64, error) GetMoreChannels(teamId string, userId string, offset int, limit int) (*model.ChannelList, error) + GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) GetPublicChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) (*model.ChannelList, *model.AppError) GetChannelCounts(teamId string, userId string) (*model.ChannelCounts, *model.AppError) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 1fec095077..665fdd4632 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -61,6 +61,7 @@ func TestChannelStore(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("GetChannels", func(t *testing.T) { testChannelStoreGetChannels(t, ss) }) t.Run("GetAllChannels", func(t *testing.T) { testChannelStoreGetAllChannels(t, ss, s) }) t.Run("GetMoreChannels", func(t *testing.T) { testChannelStoreGetMoreChannels(t, ss) }) + t.Run("GetPrivateChannelsForTeam", func(t *testing.T) { testChannelStoreGetPrivateChannelsForTeam(t, ss) }) t.Run("GetPublicChannelsForTeam", func(t *testing.T) { testChannelStoreGetPublicChannelsForTeam(t, ss) }) t.Run("GetPublicChannelsByIdsForTeam", func(t *testing.T) { testChannelStoreGetPublicChannelsByIdsForTeam(t, ss) }) t.Run("GetChannelCounts", func(t *testing.T) { testChannelStoreGetChannelCounts(t, ss) }) @@ -3444,6 +3445,98 @@ func testChannelStoreGetMoreChannels(t *testing.T, ss store.Store) { }) } +func testChannelStoreGetPrivateChannelsForTeam(t *testing.T, ss store.Store) { + teamId := model.NewId() + + // p1 is a private channel on the team + p1 := model.Channel{ + TeamId: teamId, + DisplayName: "PrivateChannel1Team1", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_PRIVATE, + } + _, nErr := ss.Channel().Save(&p1, -1) + require.Nil(t, nErr) + + // p2 is a private channel on another team + p2 := model.Channel{ + TeamId: model.NewId(), + DisplayName: "PrivateChannel1Team2", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_PRIVATE, + } + _, nErr = ss.Channel().Save(&p2, -1) + require.Nil(t, nErr) + + // o1 is a public channel on the team + o1 := model.Channel{ + TeamId: teamId, + DisplayName: "OpenChannel1Team1", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + } + _, nErr = ss.Channel().Save(&o1, -1) + require.Nil(t, nErr) + + t.Run("only p1 initially listed in private channels", func(t *testing.T) { + list, channelErr := ss.Channel().GetPrivateChannelsForTeam(teamId, 0, 100) + require.Nil(t, channelErr) + require.Equal(t, &model.ChannelList{&p1}, list) + }) + + // p3 is another private channel on the team + p3 := model.Channel{ + TeamId: teamId, + DisplayName: "PrivateChannel2Team1", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_PRIVATE, + } + _, nErr = ss.Channel().Save(&p3, -1) + require.Nil(t, nErr) + + // p4 is another private, but deleted channel on the team + p4 := model.Channel{ + TeamId: teamId, + DisplayName: "PrivateChannel3Team1", + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_PRIVATE, + } + _, nErr = ss.Channel().Save(&p4, -1) + require.Nil(t, nErr) + err := ss.Channel().Delete(p4.Id, model.GetMillis()) + require.Nil(t, err, "channel should have been deleted") + + t.Run("both p1 and p3 listed in private channels", func(t *testing.T) { + list, err := ss.Channel().GetPrivateChannelsForTeam(teamId, 0, 100) + require.Nil(t, err) + require.Equal(t, &model.ChannelList{&p1, &p3}, list) + }) + + t.Run("only p1 listed in private channels with offset 0, limit 1", func(t *testing.T) { + list, err := ss.Channel().GetPrivateChannelsForTeam(teamId, 0, 1) + require.Nil(t, err) + require.Equal(t, &model.ChannelList{&p1}, list) + }) + + t.Run("only p3 listed in private channels with offset 1, limit 1", func(t *testing.T) { + list, err := ss.Channel().GetPrivateChannelsForTeam(teamId, 1, 1) + require.Nil(t, err) + require.Equal(t, &model.ChannelList{&p3}, list) + }) + + t.Run("verify analytics for private channels", func(t *testing.T) { + count, err := ss.Channel().AnalyticsTypeCount(teamId, model.CHANNEL_PRIVATE) + require.Nil(t, err) + require.EqualValues(t, 3, count) + }) + + t.Run("verify analytics for open open channels", func(t *testing.T) { + count, err := ss.Channel().AnalyticsTypeCount(teamId, model.CHANNEL_OPEN) + require.Nil(t, err) + require.EqualValues(t, 1, count) + }) +} + func testChannelStoreGetPublicChannelsForTeam(t *testing.T, ss store.Store) { teamId := model.NewId() diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index aeffdb9c7b..4f448d545b 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -1051,6 +1051,31 @@ func (_m *ChannelStore) GetPinnedPosts(channelId string) (*model.PostList, *mode return r0, r1 } +// GetPrivateChannelsForTeam provides a mock function with given fields: teamId, offset, limit +func (_m *ChannelStore) GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + ret := _m.Called(teamId, offset, limit) + + var r0 *model.ChannelList + if rf, ok := ret.Get(0).(func(string, int, int) *model.ChannelList); ok { + r0 = rf(teamId, offset, limit) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.ChannelList) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, int, int) *model.AppError); ok { + r1 = rf(teamId, offset, limit) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetPublicChannelsByIdsForTeam provides a mock function with given fields: teamId, channelIds func (_m *ChannelStore) GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) (*model.ChannelList, *model.AppError) { ret := _m.Called(teamId, channelIds) diff --git a/store/timer_layer.go b/store/timer_layer.go index ca5814e7b6..93eaa41a89 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -1176,6 +1176,22 @@ func (s *TimerLayerChannelStore) GetPinnedPosts(channelId string) (*model.PostLi return resultVar0, resultVar1 } +func (s *TimerLayerChannelStore) GetPrivateChannelsForTeam(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { + start := timemodule.Now() + + resultVar0, resultVar1 := s.ChannelStore.GetPrivateChannelsForTeam(teamId, offset, limit) + + elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) + if s.Root.Metrics != nil { + success := "false" + if resultVar1 == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.GetPrivateChannelsForTeam", success, elapsed) + } + return resultVar0, resultVar1 +} + func (s *TimerLayerChannelStore) GetPublicChannelsByIdsForTeam(teamId string, channelIds []string) (*model.ChannelList, *model.AppError) { start := timemodule.Now()