MM-17432: Removes deactivated users from the group member counts. (#11829)

* MM-17432: Removes deactivated users from the group member counts.

* MM-17432: Reuses variable.

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")

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)
+	_, err = ss.Group().UpsertMember(group1.Id, user2.Id)
-	group1WithMemberCount.MemberCount = model.NewInt(1)
+	require.Nil(t, err)
 
-	group2WithMemberCount := model.Group(*group2)
+	user2.DeleteAt = 1
-	group2WithMemberCount.MemberCount = model.NewInt(0)
+	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)
+}