MM42267: Add member count in the browse channel modal (#23800)

* add base for calling the endpoint

* add endpoint and handler

* update store and layers

* call the endpoint

* align types

* update app layers

* generate mocks

* complete handler

* finish store query

* add todos

* add ui for member count

* add selector

* add a todo

* add cache layer

* optimize calls in FE

* handle invalidation of the cache

* fix go style

* fix test

* use existing channel layer count

* fix import error

* delete unnecessary code

* write tests for channel cache layer

* fix testname

* fix mocks

* fix cache layer test

* fix a test

* really fix the test

* write more tests for server

* address PR comments

* remove comment

* rename more_channels to browse_channels

* fix style

* update snapshot

* add translations

* Revert "add translations"

This reverts commit 56476a5dabe357703ef02be9a38b3e88f5c8b1e7.

* add only related translations

* address PR review points

* add test

* fix test

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Sinan Sonmez (Chaush)
2023-07-19 08:15:27 +02:00
коммит произвёл GitHub
родитель 4803889158
Коммит 628273d98d
37 изменённых файлов: 591 добавлений и 87 удалений

Просмотреть файл

@@ -25,6 +25,7 @@ func (api *API) InitChannel() {
api.BaseRoutes.Channels.Handle("/group", api.APISessionRequired(createGroupChannel)).Methods("POST")
api.BaseRoutes.Channels.Handle("/members/{user_id:[A-Za-z0-9]+}/view", api.APISessionRequired(viewChannel)).Methods("POST")
api.BaseRoutes.Channels.Handle("/{channel_id:[A-Za-z0-9]+}/scheme", api.APISessionRequired(updateChannelScheme)).Methods("PUT")
api.BaseRoutes.Channels.Handle("/stats/member_count", api.APISessionRequired(getChannelsMemberCount)).Methods("POST")
api.BaseRoutes.ChannelsForTeam.Handle("", api.APISessionRequired(getPublicChannelsForTeam)).Methods("GET")
api.BaseRoutes.ChannelsForTeam.Handle("/deleted", api.APISessionRequired(getDeletedChannelsForTeam)).Methods("GET")
@@ -681,6 +682,29 @@ func getChannelStats(c *Context, w http.ResponseWriter, r *http.Request) {
}
}
func getChannelsMemberCount(c *Context, w http.ResponseWriter, r *http.Request) {
if c.Err != nil {
return
}
channelIDs := model.ArrayFromJSON(r.Body)
if !c.App.SessionHasPermissionToChannels(c.AppContext, *c.AppContext.Session(), channelIDs, model.PermissionReadChannel) {
c.SetPermissionError(model.PermissionReadChannel)
return
}
channelsMemberCount, err := c.App.GetChannelsMemberCount(c.AppContext, channelIDs)
if err != nil {
c.Err = err
return
}
if err := json.NewEncoder(w).Encode(channelsMemberCount); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
}
}
func getPinnedPosts(c *Context, w http.ResponseWriter, r *http.Request) {
c.RequireChannelId()
if c.Err != nil {

Просмотреть файл

@@ -4469,6 +4469,74 @@ func TestGetChannelMemberCountsByGroup(t *testing.T) {
})
}
func TestGetChannelsMemberCount(t *testing.T) {
// Setup
th := Setup(t).InitBasic()
defer th.TearDown()
client := th.Client
channel1 := th.CreatePublicChannel()
channel2 := th.CreatePublicChannel()
user1 := th.CreateUser()
user2 := th.CreateUser()
user3 := th.CreateUser()
th.LinkUserToTeam(user1, th.BasicTeam)
th.LinkUserToTeam(user2, th.BasicTeam)
th.LinkUserToTeam(user3, th.BasicTeam)
th.AddUserToChannel(user1, channel1)
th.AddUserToChannel(user2, channel1)
th.AddUserToChannel(user3, channel1)
th.AddUserToChannel(user2, channel2)
t.Run("Should return correct member count", func(t *testing.T) {
// Create a request with channel IDs
channelIDs := []string{channel1.Id, channel2.Id}
channelsMemberCount, _, err := client.GetChannelsMemberCount(context.Background(), channelIDs)
require.NoError(t, err)
// Verify the member counts
require.Contains(t, channelsMemberCount, channel1.Id)
require.Contains(t, channelsMemberCount, channel2.Id)
require.Equal(t, int64(4), channelsMemberCount[channel1.Id])
require.Equal(t, int64(2), channelsMemberCount[channel2.Id])
})
t.Run("Should return empty object when empty array is passed", func(t *testing.T) {
channelsMemberCount, _, err := client.GetChannelsMemberCount(context.Background(), []string{})
require.NoError(t, err)
require.Equal(t, 0, len(channelsMemberCount))
})
t.Run("Should fail due to permissions", func(t *testing.T) {
_, resp, err := client.GetChannelsMemberCount(context.Background(), []string{"junk"})
require.Error(t, err)
CheckForbiddenStatus(t, resp)
CheckErrorID(t, err, "api.context.permissions.app_error")
})
t.Run("Should fail due to expired session when logged out", func(t *testing.T) {
client.Logout(context.Background())
channelIDs := []string{channel1.Id, channel2.Id}
_, resp, err := client.GetChannelsMemberCount(context.Background(), channelIDs)
require.Error(t, err)
CheckUnauthorizedStatus(t, resp)
CheckErrorID(t, err, "api.context.session_expired.app_error")
})
t.Run("Should fail due to expired session when logged out", func(t *testing.T) {
th.LoginBasic2()
channelIDs := []string{channel1.Id, channel2.Id}
_, resp, err := client.GetChannelsMemberCount(context.Background(), channelIDs)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
CheckErrorID(t, err, "api.context.permissions.app_error")
})
}
func TestMoveChannel(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()

Просмотреть файл

@@ -625,6 +625,7 @@ type AppIface interface {
GetChannelsForTeamForUser(c request.CTX, teamID string, userID string, opts *model.ChannelSearchOpts) (model.ChannelList, *model.AppError)
GetChannelsForTeamForUserWithCursor(c request.CTX, teamID string, userID string, opts *model.ChannelSearchOpts, afterChannelID string) (model.ChannelList, *model.AppError)
GetChannelsForUser(c request.CTX, userID string, includeDeleted bool, lastDeleteAt, pageSize int, fromChannelID string) (model.ChannelList, *model.AppError)
GetChannelsMemberCount(c request.CTX, channelIDs []string) (map[string]int64, *model.AppError)
GetChannelsUserNotIn(c request.CTX, teamID string, userID string, offset int, limit int) (model.ChannelList, *model.AppError)
GetCloudSession(token string) (*model.Session, *model.AppError)
GetClusterId() string

Просмотреть файл

@@ -1834,6 +1834,20 @@ func (a *App) GetChannels(c request.CTX, channelIDs []string) ([]*model.Channel,
return channels, nil
}
func (a *App) GetChannelsMemberCount(c request.CTX, channelIDs []string) (map[string]int64, *model.AppError) {
channelsCount, err := a.Srv().Store().Channel().GetChannelsMemberCount(channelIDs)
if err != nil {
var nfErr *store.ErrNotFound
switch {
case errors.As(err, &nfErr):
return nil, model.NewAppError("GetChannelsMemberCount", "app.channel.get_channels_member_count.existing.app_error", nil, "", http.StatusNotFound).Wrap(err)
default:
return nil, model.NewAppError("GetChannelsMemberCount", "app.channel.get_channels_member_count.find.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
}
return channelsCount, nil
}
func (a *App) GetChannelByName(c request.CTX, channelName, teamID string, includeDeleted bool) (*model.Channel, *model.AppError) {
var channel *model.Channel
var err error

Просмотреть файл

@@ -2195,6 +2195,24 @@ func TestGetMemberCountsByGroup(t *testing.T) {
require.ElementsMatch(t, cmc, resp)
}
func TestGetChannelsMemberCount(t *testing.T) {
th := SetupWithStoreMock(t)
defer th.TearDown()
mockStore := th.App.Srv().Store().(*mocks.Store)
mockChannelStore := mocks.ChannelStore{}
channelsMemberCount := map[string]int64{
"channel1": int64(10),
"channel2": int64(20),
}
mockChannelStore.On("GetChannelsMemberCount", []string{"channel1", "channel2"}).Return(channelsMemberCount, nil)
mockStore.On("Channel").Return(&mockChannelStore)
mockStore.On("GetDBSchemaVersion").Return(1, nil)
resp, err := th.App.GetChannelsMemberCount(th.Context, []string{"channel1", "channel2"})
require.Nil(t, err)
require.Equal(t, channelsMemberCount, resp)
}
func TestViewChannelCollapsedThreadsTurnedOff(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()

Просмотреть файл

@@ -5612,6 +5612,28 @@ func (a *OpenTracingAppLayer) GetChannelsForUser(c request.CTX, userID string, i
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetChannelsMemberCount(c request.CTX, channelIDs []string) (map[string]int64, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetChannelsMemberCount")
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.GetChannelsMemberCount(c, channelIDs)
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))
ext.Error.Set(span, true)
}
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetChannelsUserNotIn(c request.CTX, teamID string, userID string, offset int, limit int) (model.ChannelList, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetChannelsUserNotIn")

Просмотреть файл

@@ -225,6 +225,35 @@ func (s LocalCacheChannelStore) SaveMultipleMembers(members []*model.ChannelMemb
return members, nil
}
func (s LocalCacheChannelStore) GetChannelsMemberCount(channelIDs []string) (_ map[string]int64, err error) {
counts := make(map[string]int64)
remainingChannels := make([]string, 0)
for _, channelID := range channelIDs {
var cacheItem int64
err := s.rootStore.doStandardReadCache(s.rootStore.channelMemberCountsCache, channelID, &cacheItem)
if err == nil {
counts[channelID] = cacheItem
} else {
remainingChannels = append(remainingChannels, channelID)
}
}
if len(remainingChannels) > 0 {
remainingChannels, err := s.ChannelStore.GetChannelsMemberCount(remainingChannels)
if err != nil {
return nil, err
}
for id, count := range remainingChannels {
s.rootStore.doStandardAddToCache(s.rootStore.channelMemberCountsCache, id, count)
counts[id] = count
}
}
return counts, nil
}
func (s LocalCacheChannelStore) UpdateMember(member *model.ChannelMember) (*model.ChannelMember, error) {
member, err := s.ChannelStore.UpdateMember(member)
if err != nil {

Просмотреть файл

@@ -104,6 +104,43 @@ func TestChannelStoreChannelMemberCountsCache(t *testing.T) {
})
}
func TestChannelStoreChannelsMemberCountCache(t *testing.T) {
channelsCountResult := map[string]int64{
"channel1": 10,
"channel2": 20,
}
t.Run("first call not cached, second cached and returning same data", func(t *testing.T) {
mockStore := getMockStore()
mockCacheProvider := getMockCacheProvider()
cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
require.NoError(t, err)
channelsCount, err := cachedStore.Channel().GetChannelsMemberCount([]string{"channel1", "channel2"})
require.NoError(t, err)
assert.Equal(t, channelsCount, channelsCountResult)
mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetChannelsMemberCount", 1)
channelsCount, err = cachedStore.Channel().GetChannelsMemberCount([]string{"channel1", "channel2"})
require.NoError(t, err)
assert.Equal(t, channelsCount, channelsCountResult)
mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetChannelsMemberCount", 1)
})
t.Run("first call not cached, invalidate cache, second call not cached", func(t *testing.T) {
mockStore := getMockStore()
mockCacheProvider := getMockCacheProvider()
cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
require.NoError(t, err)
cachedStore.Channel().GetChannelsMemberCount([]string{"channel1", "channel2"})
mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetChannelsMemberCount", 1)
cachedStore.Channel().InvalidateMemberCount("channel1")
cachedStore.Channel().InvalidateMemberCount("channel2")
cachedStore.Channel().GetChannelsMemberCount([]string{"channel1", "channel2"})
mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetChannelsMemberCount", 2)
})
}
func TestChannelStoreChannelPinnedPostsCountsCache(t *testing.T) {
countResult := int64(10)

Просмотреть файл

@@ -100,6 +100,12 @@ func getMockStore() *mocks.Store {
mockChannelStore.On("Get", channelId, false).Return(&fakeChannelId, nil)
mockStore.On("Channel").Return(&mockChannelStore)
mockChannelsMemberCount := map[string]int64{
"channel1": 10,
"channel2": 20,
}
mockChannelStore.On("GetChannelsMemberCount", []string{"channel1", "channel2"}).Return(mockChannelsMemberCount, nil)
mockPinnedPostsCount := int64(10)
mockChannelStore.On("GetPinnedPostCount", "id", true).Return(mockPinnedPostsCount, nil)
mockChannelStore.On("GetPinnedPostCount", "id", false).Return(mockPinnedPostsCount, nil)

Просмотреть файл

@@ -1269,6 +1269,24 @@ func (s *OpenTracingLayerChannelStore) GetChannelsByUser(userID string, includeD
return result, err
}
func (s *OpenTracingLayerChannelStore) GetChannelsMemberCount(channelIDs []string) (map[string]int64, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetChannelsMemberCount")
s.Root.Store.SetContext(newCtx)
defer func() {
s.Root.Store.SetContext(origCtx)
}()
defer span.Finish()
result, err := s.ChannelStore.GetChannelsMemberCount(channelIDs)
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)
}
return result, err
}
func (s *OpenTracingLayerChannelStore) GetChannelsWithCursor(teamId string, userId string, opts *model.ChannelSearchOpts, afterChannelID string) (model.ChannelList, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ChannelStore.GetChannelsWithCursor")

Просмотреть файл

@@ -1406,6 +1406,27 @@ func (s *RetryLayerChannelStore) GetChannelsByUser(userID string, includeDeleted
}
func (s *RetryLayerChannelStore) GetChannelsMemberCount(channelIDs []string) (map[string]int64, error) {
tries := 0
for {
result, err := s.ChannelStore.GetChannelsMemberCount(channelIDs)
if err == nil {
return result, nil
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
timepkg.Sleep(100 * timepkg.Millisecond)
}
}
func (s *RetryLayerChannelStore) GetChannelsWithCursor(teamId string, userId string, opts *model.ChannelSearchOpts, afterChannelID string) (model.ChannelList, error) {
tries := 0

Просмотреть файл

@@ -2207,6 +2207,47 @@ func (s SqlChannelStore) GetAllChannelMembersForUser(userId string, allowFromCac
return ids, nil
}
func (s SqlChannelStore) GetChannelsMemberCount(channelIDs []string) (_ map[string]int64, err error) {
query := s.getQueryBuilder().
Select("ChannelMembers.ChannelId,COUNT(*) AS Count").
From("ChannelMembers").
InnerJoin("Users ON ChannelMembers.UserId = Users.Id").
Where(sq.And{
sq.Eq{"ChannelMembers.ChannelId": channelIDs},
sq.Eq{"Users.DeleteAt": 0},
}).
GroupBy("ChannelMembers.ChannelId")
queryString, args, err := query.ToSql()
if err != nil {
return nil, errors.Wrap(err, "channels_member_count_tosql")
}
rows, err := s.GetReplicaX().DB.Query(queryString, args...)
if err != nil {
return nil, errors.Wrap(err, "failed to fetch member counts")
}
defer rows.Close()
memberCounts := make(map[string]int64)
for rows.Next() {
var channelID string
var count int64
errScan := rows.Scan(&channelID, &count)
if errScan != nil {
return nil, errors.Wrap(err, "failed to scan row")
}
memberCounts[channelID] = count
}
if err = rows.Err(); err != nil {
return nil, errors.Wrap(err, "error while iterating rows")
}
return memberCounts, nil
}
func (s SqlChannelStore) InvalidateCacheForChannelMembersNotifyProps(channelId string) {
allChannelMembersNotifyPropsForChannelCache.Remove(channelId)
if s.metrics != nil {

Просмотреть файл

@@ -222,6 +222,7 @@ type ChannelStore interface {
GetMember(ctx context.Context, channelID string, userID string) (*model.ChannelMember, error)
GetChannelMembersTimezones(channelID string) ([]model.StringMap, error)
GetAllChannelMembersForUser(userID string, allowFromCache bool, includeDeleted bool) (map[string]string, error)
GetChannelsMemberCount(channelIDs []string) (map[string]int64, error)
InvalidateAllChannelMembersForUser(userID string)
IsUserInChannelUseCache(userID string, channelID string) bool
GetAllChannelMembersNotifyPropsForChannel(channelID string, allowFromCache bool) (map[string]model.StringMap, error)

Просмотреть файл

@@ -910,6 +910,32 @@ func (_m *ChannelStore) GetChannelsByUser(userID string, includeDeleted bool, la
return r0, r1
}
// GetChannelsMemberCount provides a mock function with given fields: channelIDs
func (_m *ChannelStore) GetChannelsMemberCount(channelIDs []string) (map[string]int64, error) {
ret := _m.Called(channelIDs)
var r0 map[string]int64
var r1 error
if rf, ok := ret.Get(0).(func([]string) (map[string]int64, error)); ok {
return rf(channelIDs)
}
if rf, ok := ret.Get(0).(func([]string) map[string]int64); ok {
r0 = rf(channelIDs)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(map[string]int64)
}
}
if rf, ok := ret.Get(1).(func([]string) error); ok {
r1 = rf(channelIDs)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// GetChannelsWithCursor provides a mock function with given fields: teamId, userId, opts, afterChannelID
func (_m *ChannelStore) GetChannelsWithCursor(teamId string, userId string, opts *model.ChannelSearchOpts, afterChannelID string) (model.ChannelList, error) {
ret := _m.Called(teamId, userId, opts, afterChannelID)

Просмотреть файл

@@ -1184,6 +1184,22 @@ func (s *TimerLayerChannelStore) GetChannelsByUser(userID string, includeDeleted
return result, err
}
func (s *TimerLayerChannelStore) GetChannelsMemberCount(channelIDs []string) (map[string]int64, error) {
start := time.Now()
result, err := s.ChannelStore.GetChannelsMemberCount(channelIDs)
elapsed := float64(time.Since(start)) / float64(time.Second)
if s.Root.Metrics != nil {
success := "false"
if err == nil {
success = "true"
}
s.Root.Metrics.ObserveStoreMethodDuration("ChannelStore.GetChannelsMemberCount", success, elapsed)
}
return result, err
}
func (s *TimerLayerChannelStore) GetChannelsWithCursor(teamId string, userId string, opts *model.ChannelSearchOpts, afterChannelID string) (model.ChannelList, error) {
start := time.Now()

Просмотреть файл

@@ -4739,6 +4739,14 @@
"id": "app.channel.get_channels_by_ids.not_found.app_error",
"translation": "No channel found."
},
{
"id": "app.channel.get_channels_member_count.existing.app_error",
"translation": "Unable to find member count for given channels."
},
{
"id": "app.channel.get_channels_member_count.find.app_error",
"translation": "Unable to find member count."
},
{
"id": "app.channel.get_deleted.existing.app_error",
"translation": "Unable to find the existing deleted channel."

Просмотреть файл

@@ -3047,6 +3047,21 @@ func (c *Client4) GetChannelStats(ctx context.Context, channelId string, etag st
return &stats, BuildResponse(r), nil
}
// GetChannelsMemberCount get channel member count for a given array of channel ids
func (c *Client4) GetChannelsMemberCount(ctx context.Context, channelIDs []string) (map[string]int64, *Response, error) {
route := c.channelsRoute() + "/stats/member_count"
r, err := c.DoAPIPost(ctx, route, ArrayToJSON(channelIDs))
if err != nil {
return nil, BuildResponse(r), err
}
defer closeBody(r)
var counts map[string]int64
if err := json.NewDecoder(r.Body).Decode(&counts); err != nil {
return nil, nil, NewAppError("GetChannelsMemberCount", "api.unmarshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
return counts, BuildResponse(r), nil
}
// GetChannelMembersTimezones gets a list of timezones for a channel.
func (c *Client4) GetChannelMembersTimezones(ctx context.Context, channelId string) ([]string, *Response, error) {
r, err := c.DoAPIGet(ctx, c.channelRoute(channelId)+"/timezones", "")

Просмотреть файл

@@ -29,6 +29,7 @@ const (
ClusterEventInvalidateCacheForChannelFileCount ClusterEvent = "inv_channel_file_count"
ClusterEventInvalidateCacheForChannelPinnedpostsCounts ClusterEvent = "inv_channel_pinnedposts_counts"
ClusterEventInvalidateCacheForChannelMemberCounts ClusterEvent = "inv_channel_member_counts"
ClusterEventInvalidateCacheForChannelsMemberCount ClusterEvent = "inv_channels_member_count"
ClusterEventInvalidateCacheForLastPosts ClusterEvent = "inv_last_posts"
ClusterEventInvalidateCacheForLastPostTime ClusterEvent = "inv_last_post_time"
ClusterEventInvalidateCacheForPostsUsage ClusterEvent = "inv_posts_usage"