From 76f7872a50b755c734bf29247838e18b4a819b03 Mon Sep 17 00:00:00 2001 From: Ben Cooke Date: Tue, 22 Nov 2022 11:31:04 -0500 Subject: [PATCH] [MM-46692] Channel group member count (#21270) * tools updates * Revert "tools updates" This reverts commit 6293297b55803c5a263e200ebd80192899666ae9. * adding channel member count to groups request * fixing models * adding a new test * removing unused var Co-authored-by: Benjamin Cooke Co-authored-by: Benjamin Cooke Co-authored-by: Benjamin Cooke Co-authored-by: Benjamin Cooke Co-authored-by: Mattermod --- api4/group.go | 39 +++++++++++-- api4/group_test.go | 22 +++++++ model/client4.go | 4 +- model/group.go | 29 ++++++---- store/sqlstore/group_store.go | 104 +++++++++++++++++++++++++--------- web/params.go | 2 + 6 files changed, 153 insertions(+), 47 deletions(-) diff --git a/api4/group.go b/api4/group.go index 343db5ee7e..78c606a448 100644 --- a/api4/group.go +++ b/api4/group.go @@ -950,12 +950,13 @@ func getGroupsAssociatedToChannelsByTeam(c *Context, w http.ResponseWriter, r *h } func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { + var teamID, NotAssociatedToChannelID, ChannelIDForMemberCount string + permissionErr := requireLicense(c) if permissionErr != nil { c.Err = permissionErr return } - var teamID, channelID string source := c.Params.GroupSource @@ -964,7 +965,11 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { } if id := c.Params.NotAssociatedToChannel; model.IsValidId(id) { - channelID = id + NotAssociatedToChannelID = id + } + + if id := c.Params.IncludeChannelMemberCount; model.IsValidId(id) { + ChannelIDForMemberCount = id } // If they specify the group_source as custom when the feature is disabled, throw an error @@ -979,6 +984,8 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { source = model.GroupSourceLdap } + includeTimezones := r.URL.Query().Get("include_timezones") == "true" + opts := model.GroupSearchOpts{ Q: c.Params.Q, IncludeMemberCount: c.Params.IncludeMemberCount, @@ -986,6 +993,7 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { FilterParentTeamPermitted: c.Params.FilterParentTeamPermitted, Source: source, FilterHasMember: c.Params.FilterHasMember, + IncludeTimezones: includeTimezones, } if teamID != "" { @@ -998,8 +1006,8 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { opts.NotAssociatedToTeam = teamID } - if channelID != "" { - channel, appErr := c.App.GetChannel(c.AppContext, channelID) + if NotAssociatedToChannelID != "" { + channel, appErr := c.App.GetChannel(c.AppContext, NotAssociatedToChannelID) if appErr != nil { c.Err = appErr return @@ -1010,11 +1018,30 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { } else { permission = model.PermissionManagePublicChannelMembers } - if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channelID, permission) { + if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), NotAssociatedToChannelID, permission) { c.SetPermissionError(permission) return } - opts.NotAssociatedToChannel = channelID + opts.NotAssociatedToChannel = NotAssociatedToChannelID + } + + if ChannelIDForMemberCount != "" { + channel, appErr := c.App.GetChannel(c.AppContext, ChannelIDForMemberCount) + if appErr != nil { + c.Err = appErr + return + } + var permission *model.Permission + if channel.Type == model.ChannelTypePrivate { + permission = model.PermissionManagePrivateChannelMembers + } else { + permission = model.PermissionManagePublicChannelMembers + } + if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), ChannelIDForMemberCount, permission) { + c.SetPermissionError(permission) + return + } + opts.IncludeChannelMemberCount = ChannelIDForMemberCount } sinceString := r.URL.Query().Get("since") diff --git a/api4/group_test.go b/api4/group_test.go index 468e043d4e..a612291d8c 100644 --- a/api4/group_test.go +++ b/api4/group_test.go @@ -1291,6 +1291,28 @@ func TestGetGroups(t *testing.T) { assert.Len(t, groups, 1) assert.Equal(t, groups[0].Id, group2.Id) + // Test IncludeChannelMemberCount url param is working + opts.IncludeChannelMemberCount = th.BasicChannel.Id + opts.IncludeTimezones = true + opts.Q = "-fOo" + opts.IncludeMemberCount = true + + groups, _, _ = th.SystemAdminClient.GetGroups(opts) + assert.Equal(t, *groups[0].MemberCount, int(0)) + assert.Equal(t, *groups[0].ChannelMemberCount, int(0)) + + _, appErr = th.App.UpsertGroupMember(group2.Id, th.BasicUser.Id) + assert.Nil(t, appErr) + + groups, _, _ = th.SystemAdminClient.GetGroups(opts) + assert.NotNil(t, groups[0].MemberCount) + assert.Equal(t, *groups[0].ChannelMemberCount, int(1)) + + opts.IncludeChannelMemberCount = "" + opts.IncludeTimezones = false + opts.Q = "" + opts.IncludeMemberCount = false + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableCustomGroups = false }) diff --git a/model/client4.go b/model/client4.go index 24ffc859ca..d195bec920 100644 --- a/model/client4.go +++ b/model/client4.go @@ -5515,7 +5515,7 @@ func (c *Client4) GetGroupsAssociatedToChannelsByTeam(teamId string, opts GroupS // GetGroups retrieves Mattermost Groups func (c *Client4) GetGroups(opts GroupSearchOpts) ([]*Group, *Response, error) { path := fmt.Sprintf( - "%s?include_member_count=%v¬_associated_to_team=%v¬_associated_to_channel=%v&filter_allow_reference=%v&q=%v&filter_parent_team_permitted=%v&group_source=%v", + "%s?include_member_count=%v¬_associated_to_team=%v¬_associated_to_channel=%v&filter_allow_reference=%v&q=%v&filter_parent_team_permitted=%v&group_source=%v&include_channel_member_count=%v&include_timezones=%v", c.groupsRoute(), opts.IncludeMemberCount, opts.NotAssociatedToTeam, @@ -5524,6 +5524,8 @@ func (c *Client4) GetGroups(opts GroupSearchOpts) ([]*Group, *Response, error) { opts.Q, opts.FilterParentTeamPermitted, opts.Source, + opts.IncludeChannelMemberCount, + opts.IncludeTimezones, ) if opts.Since > 0 { path = fmt.Sprintf("%s&since=%v", path, opts.Since) diff --git a/model/group.go b/model/group.go index 20d9533401..530eab88f7 100644 --- a/model/group.go +++ b/model/group.go @@ -31,18 +31,20 @@ var groupSourcesRequiringRemoteID = []GroupSource{ } type Group struct { - Id string `json:"id"` - Name *string `json:"name,omitempty"` - DisplayName string `json:"display_name"` - Description string `json:"description"` - Source GroupSource `json:"source"` - RemoteId *string `json:"remote_id"` - CreateAt int64 `json:"create_at"` - UpdateAt int64 `json:"update_at"` - DeleteAt int64 `json:"delete_at"` - HasSyncables bool `db:"-" json:"has_syncables"` - MemberCount *int `db:"-" json:"member_count,omitempty"` - AllowReference bool `json:"allow_reference"` + Id string `json:"id"` + Name *string `json:"name,omitempty"` + DisplayName string `json:"display_name"` + Description string `json:"description"` + Source GroupSource `json:"source"` + RemoteId *string `json:"remote_id"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + DeleteAt int64 `json:"delete_at"` + HasSyncables bool `db:"-" json:"has_syncables"` + MemberCount *int `db:"-" json:"member_count,omitempty"` + AllowReference bool `json:"allow_reference"` + ChannelMemberCount *int `db:"-" json:"channel_member_count,omitempty"` + ChannelMemberTimezonesCount *int `db:"-" json:"channel_member_timezones_count,omitempty"` } func (group *Group) Auditable() map[string]interface{} { @@ -113,6 +115,9 @@ type GroupSearchOpts struct { // FilterHasMember filters the groups to the intersect of the // set returned by the query and those that have the given user as a member. FilterHasMember string + + IncludeChannelMemberCount string + IncludeTimezones bool } type GetGroupOpts struct { diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 40866d2a7a..64737840b8 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -1056,34 +1056,38 @@ func (s *SqlGroupStore) CountGroupsByChannel(channelId string, opts model.GroupS } type group struct { - Id string - Name *string - DisplayName string - Description string - Source model.GroupSource - RemoteId *string - CreateAt int64 - UpdateAt int64 - DeleteAt int64 - HasSyncables bool - MemberCount *int - AllowReference bool + Id string + Name *string + DisplayName string + Description string + Source model.GroupSource + RemoteId *string + CreateAt int64 + UpdateAt int64 + DeleteAt int64 + HasSyncables bool + MemberCount *int + AllowReference bool + ChannelMemberCount *int + ChannelMemberTimezonesCount *int } func (g group) ToModel() *model.Group { return &model.Group{ - Id: g.Id, - Name: g.Name, - DisplayName: g.DisplayName, - Description: g.Description, - Source: g.Source, - RemoteId: g.RemoteId, - CreateAt: g.CreateAt, - UpdateAt: g.UpdateAt, - DeleteAt: g.DeleteAt, - HasSyncables: g.HasSyncables, - AllowReference: g.AllowReference, - MemberCount: g.MemberCount, + Id: g.Id, + Name: g.Name, + DisplayName: g.DisplayName, + Description: g.Description, + Source: g.Source, + RemoteId: g.RemoteId, + CreateAt: g.CreateAt, + UpdateAt: g.UpdateAt, + DeleteAt: g.DeleteAt, + HasSyncables: g.HasSyncables, + AllowReference: g.AllowReference, + MemberCount: g.MemberCount, + ChannelMemberCount: g.ChannelMemberCount, + ChannelMemberTimezonesCount: g.ChannelMemberTimezonesCount, } } @@ -1416,7 +1420,20 @@ func (s *SqlGroupStore) GetGroupsAssociatedToChannelsByTeam(teamId string, opts func (s *SqlGroupStore) GetGroups(page, perPage int, opts model.GroupSearchOpts, viewRestrictions *model.ViewUsersRestrictions) ([]*model.Group, error) { groupsVar := groups{} - groupsQuery := s.getQueryBuilder().Select("g.*") + selectQuery := []string{"g.*"} + + if opts.IncludeMemberCount { + selectQuery = append(selectQuery, "coalesce(Members.MemberCount, 0) AS MemberCount") + } + + if opts.IncludeChannelMemberCount != "" { + selectQuery = append(selectQuery, "coalesce(ChannelMembers.ChannelMemberCount, 0) AS ChannelMemberCount") + if opts.IncludeTimezones { + selectQuery = append(selectQuery, "coalesce(ChannelMembers.ChannelMemberTimezonesCount, 0) AS ChannelMemberTimezonesCount") + } + } + + groupsQuery := s.getQueryBuilder().Select(strings.Join(selectQuery, ", ")) if opts.IncludeMemberCount { countQuery := s.getQueryBuilder(). @@ -1433,12 +1450,43 @@ func (s *SqlGroupStore) GetGroups(page, perPage int, opts model.GroupSearchOpts, if err != nil { return nil, errors.Wrap(err, "get_groups_tosql") } - - groupsQuery = s.getQueryBuilder(). - Select("g.*, coalesce(Members.MemberCount, 0) AS MemberCount"). + groupsQuery = groupsQuery. LeftJoin("("+countString+") AS Members ON Members.GroupId = g.Id", params...) } + if opts.IncludeChannelMemberCount != "" { + selectStr := "GroupMembers.GroupId, COUNT(ChannelMembers.UserId) AS ChannelMemberCount" + joinStr := "" + + if opts.IncludeTimezones { + if s.DriverName() == model.DatabaseDriverMysql { + selectStr += `, + COUNT(DISTINCT + ( + CASE WHEN JSON_EXTRACT(Timezone, '$.useAutomaticTimezone') = 'true' AND LENGTH(JSON_UNQUOTE(JSON_EXTRACT(Timezone, '$.automaticTimezone'))) > 0 + THEN JSON_EXTRACT(Timezone, '$.automaticTimezone') + WHEN JSON_EXTRACT(Timezone, '$.useAutomaticTimezone') = 'false' AND LENGTH(JSON_UNQUOTE(JSON_EXTRACT(Timezone, '$.manualTimezone'))) > 0 + THEN JSON_EXTRACT(Timezone, '$.manualTimezone') + END + )) AS ChannelMemberTimezonesCount` + } else if s.DriverName() == model.DatabaseDriverPostgres { + selectStr += `, + COUNT(DISTINCT + ( + CASE WHEN Timezone->>'useAutomaticTimezone' = 'true' AND length(Timezone->>'automaticTimezone') > 0 + THEN Timezone->>'automaticTimezone' + WHEN Timezone->>'useAutomaticTimezone' = 'false' AND length(Timezone->>'manualTimezone') > 0 + THEN Timezone->>'manualTimezone' + END + )) AS ChannelMemberTimezonesCount` + } + joinStr = "LEFT JOIN Users ON Users.Id = GroupMembers.UserId" + } + + groupsQuery = groupsQuery. + LeftJoin("(SELECT "+selectStr+" FROM ChannelMembers LEFT JOIN GroupMembers ON GroupMembers.UserId = ChannelMembers.UserId AND GroupMembers.DeleteAt = 0 "+joinStr+" WHERE ChannelMembers.ChannelId = ? GROUP BY GroupId) AS ChannelMembers ON ChannelMembers.GroupId = g.Id", opts.IncludeChannelMemberCount) + } + if opts.FilterHasMember != "" { groupsQuery = groupsQuery. LeftJoin("GroupMembers ON GroupMembers.GroupId = g.Id"). diff --git a/web/params.go b/web/params.go index 20c944355b..64e8227119 100644 --- a/web/params.go +++ b/web/params.go @@ -90,6 +90,7 @@ type Params struct { ExcludePolicyConstrained bool GroupSource model.GroupSource FilterHasMember string + IncludeChannelMemberCount string // Cloud InvoiceId string @@ -208,6 +209,7 @@ func ParamsFromRequest(r *http.Request) *Params { params.NotAssociatedToChannel = query.Get("not_associated_to_channel") params.FilterAllowReference, _ = strconv.ParseBool(query.Get("filter_allow_reference")) params.FilterParentTeamPermitted, _ = strconv.ParseBool(query.Get("filter_parent_team_permitted")) + params.IncludeChannelMemberCount = query.Get("include_channel_member_count") if val, err := strconv.ParseBool(query.Get("paginate")); err == nil { params.Paginate = &val