diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index b2c36643c3..fbb6708765 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -261,8 +261,10 @@ func (s *SqlGroupStore) GetMemberCount(groupID string) (int64, *model.AppError) count(*) FROM GroupMembers + JOIN Users ON Users.Id = GroupMembers.UserId WHERE - GroupMembers.GroupId = :GroupId` + GroupMembers.GroupId = :GroupId + AND Users.DeleteAt = 0` count, err := s.GetReplica().SelectInt(query, map[string]interface{}{"GroupId": groupID}) if err != nil { @@ -840,7 +842,7 @@ func (s *SqlGroupStore) groupsBySyncableBaseQuery(st model.GroupSyncableType, t query = s.getQueryBuilder(). Select("ug.*, coalesce(Members.MemberCount, 0) AS MemberCount"). From("UserGroups ug"). - LeftJoin("(SELECT GroupMembers.GroupId, COUNT(*) AS MemberCount FROM GroupMembers WHERE GroupMembers.DeleteAt = 0 GROUP BY GroupId) AS Members ON Members.GroupId = ug.Id"). + LeftJoin("(SELECT GroupMembers.GroupId, COUNT(*) AS MemberCount FROM GroupMembers LEFT JOIN Users ON Users.Id = GroupMembers.UserId WHERE GroupMembers.DeleteAt = 0 AND Users.DeleteAt = 0 GROUP BY GroupId) AS Members ON Members.GroupId = ug.Id"). LeftJoin(fmt.Sprintf("%[1]s ON %[1]s.GroupId = ug.Id", table)). Where(fmt.Sprintf("%[1]s.DeleteAt = 0 AND %[1]s.%[2]s = ?", table, idCol), syncableID). OrderBy("ug.DisplayName") @@ -906,7 +908,7 @@ func (s *SqlGroupStore) GetGroups(page, perPage int, opts model.GroupSearchOpts) groupsQuery = s.getQueryBuilder(). Select("g.*, coalesce(Members.MemberCount, 0) AS MemberCount"). From("UserGroups g"). - LeftJoin("(SELECT GroupMembers.GroupId, COUNT(*) AS MemberCount FROM GroupMembers WHERE GroupMembers.DeleteAt = 0 GROUP BY GroupId) AS Members ON Members.GroupId = g.Id"). + LeftJoin("(SELECT GroupMembers.GroupId, COUNT(*) AS MemberCount FROM GroupMembers LEFT JOIN Users ON Users.Id = GroupMembers.UserId WHERE GroupMembers.DeleteAt = 0 AND Users.DeleteAt = 0 GROUP BY GroupId) AS Members ON Members.GroupId = g.Id"). Limit(uint64(perPage)). Offset(uint64(page * perPage)). OrderBy("g.DisplayName") diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index c5aa02aaed..cf26186816 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -50,6 +50,8 @@ func TestGroupStore(t *testing.T, ss store.Store) { t.Run("TeamMembersMinusGroupMembers", func(t *testing.T) { testTeamMembersMinusGroupMembers(t, ss) }) t.Run("ChannelMembersMinusGroupMembers", func(t *testing.T) { testChannelMembersMinusGroupMembers(t, ss) }) + + t.Run("GetMemberCount", func(t *testing.T) { groupTestGetMemberCount(t, ss) }) } func testGroupStoreCreate(t *testing.T, ss store.Store) { @@ -1635,9 +1637,23 @@ func testGetGroupsByChannel(t *testing.T, ss store.Store) { user1, err := ss.User().Save(u1) require.Nil(t, err) + u2 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user2, err := ss.User().Save(u2) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(group1.Id, user1.Id) require.Nil(t, err) + _, err = ss.Group().UpsertMember(group1.Id, user2.Id) + require.Nil(t, err) + + user2.DeleteAt = 1 + _, err = ss.User().Update(user2, true) + require.Nil(t, err) + group1WithMemberCount := model.Group(*group1) group1WithMemberCount.MemberCount = model.NewInt(1) @@ -1834,9 +1850,23 @@ func testGetGroupsByTeam(t *testing.T, ss store.Store) { user1, err := ss.User().Save(u1) require.Nil(t, err) + u2 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user2, err := ss.User().Save(u2) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(group1.Id, user1.Id) require.Nil(t, err) + _, err = ss.Group().UpsertMember(group1.Id, user2.Id) + require.Nil(t, err) + + user2.DeleteAt = 1 + _, err = ss.User().Update(user2, true) + require.Nil(t, err) + group1WithMemberCount := model.Group(*group1) group1WithMemberCount.MemberCount = model.NewInt(1) @@ -2074,14 +2104,21 @@ func testGetGroups(t *testing.T, ss store.Store) { user1, err := ss.User().Save(u1) require.Nil(t, err) + u2 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user2, err := ss.User().Save(u2) + require.Nil(t, err) + _, err = ss.Group().UpsertMember(group1.Id, user1.Id) require.Nil(t, err) - group1WithMemberCount := model.Group(*group1) - group1WithMemberCount.MemberCount = model.NewInt(1) + _, err = ss.Group().UpsertMember(group1.Id, user2.Id) + require.Nil(t, err) - group2WithMemberCount := model.Group(*group2) - group2WithMemberCount.MemberCount = model.NewInt(0) + user2.DeleteAt = 1 + ss.User().Update(user2, true) group2NameSubstring := string([]rune(group2.Name)[2:5]) @@ -2166,12 +2203,15 @@ func testGetGroups(t *testing.T, ss store.Store) { Name: "Include member counts", Opts: model.GroupSearchOpts{IncludeMemberCount: true}, Page: 0, - PerPage: 2, + PerPage: 100, Resultf: func(groups []*model.Group) bool { for _, g := range groups { if g.MemberCount == nil { return false } + if g.Id == group1.Id && *g.MemberCount != 1 { + return false + } } return true }, @@ -2539,3 +2579,41 @@ func testChannelMembersMinusGroupMembers(t *testing.T, ss store.Store) { }) } } + +func groupTestGetMemberCount(t *testing.T, ss store.Store) { + group := &model.Group{ + Name: model.NewId(), + DisplayName: model.NewId(), + Source: model.GroupSourceLdap, + Description: model.NewId(), + RemoteId: model.NewId(), + } + group, err := ss.Group().Create(group) + require.Nil(t, err) + + var user *model.User + + for i := 0; i < 2; i++ { + user = &model.User{ + Email: MakeEmail(), + Username: fmt.Sprintf("%d_%s", i, model.NewId()), + } + user, err = ss.User().Save(user) + require.Nil(t, err) + + _, err = ss.Group().UpsertMember(group.Id, user.Id) + require.Nil(t, err) + } + + count, err := ss.Group().GetMemberCount(group.Id) + require.Nil(t, err) + require.Equal(t, int64(2), count) + + user.DeleteAt = 1 + _, err = ss.User().Update(user, true) + require.Nil(t, err) + + count, err = ss.Group().GetMemberCount(group.Id) + require.Nil(t, err) + require.Equal(t, int64(1), count) +}