fix(group): Calculate member count as a whole (#2257)

Signed-off-by: Javier Rodriguez <[email protected]>

Author: javirln

app/controlplane/cmd/wire_gen.go

@@ -55,6 +55,8 @@ type GroupRepo interface {
 	ListPendingInvitationsByGroup(ctx context.Context, orgID uuid.UUID, groupID uuid.UUID, paginationOpts *pagination.OffsetPaginationOpts) ([]*OrgInvitation, int, error)
 	// ListProjectsByGroup retrieves a list of projects that a group is a member of with pagination.
 	ListProjectsByGroup(ctx context.Context, orgID uuid.UUID, groupID uuid.UUID, visibleProjectIDs []uuid.UUID, paginationOpts *pagination.OffsetPaginationOpts) ([]*GroupProjectInfo, int, error)
+	// UpdateGroupMemberCount updates the member count of a group.
+	UpdateGroupMemberCount(ctx context.Context, groupID uuid.UUID) error
 }
 
 // GroupMembership represents a membership of a user in a group.

app/controlplane/pkg/biz/group.go

@@ -0,0 +1,12 @@
+-- Fix group member counts by calculating the correct number of members for each group
+-- This migration addresses any inconsistencies in the member_count field of the groups table
+-- by counting the actual number of non-deleted memberships for each group.
+
+-- Update the member_count in groups table based on a count of active memberships
+UPDATE "groups" g
+SET "member_count" = (
+    SELECT COUNT(*)
+    FROM "group_memberships" gm
+    WHERE gm."group_id" = g."id"
+      AND gm."deleted_at" IS NULL
+); -- Apply to all groups

app/controlplane/pkg/biz/testhelpers/wire_gen.go

@@ -1,4 +1,4 @@
-h1:WasPyGuTE05lx75h+qqmoAknSDJPoN50E8hqbhNFVQs=
+h1:bxjg7J7Xk0+87+PhS0+F/c4AcHJZ3pkf0FnPMZa39Q0=
 20230706165452_init-schema.sql h1:VvqbNFEQnCvUVyj2iDYVQQxDM0+sSXqocpt/5H64k8M=
 20230710111950-cas-backend.sql h1:A8iBuSzZIEbdsv9ipBtscZQuaBp3V5/VMw7eZH6GX+g=
 20230712094107-cas-backends-workflow-runs.sql h1:a5rzxpVGyd56nLRSsKrmCFc9sebg65RWzLghKHh5xvI=
@@ -97,3 +97,4 @@ h1:WasPyGuTE05lx75h+qqmoAknSDJPoN50E8hqbhNFVQs=
 20250704090359.sql h1:a0ksfjy2dtzviJL16HbC4eT1xBxy2qFH5mNFOpYlUrA=
 20250710105502.sql h1:EA6Ta1qsZcrNoOrO5zUNgiweHDtjl0HUlobukRuruko=
 20250714172256.sql h1:S0ImNk0sMjWVVZvS6VVHn2h96/nx8GOf4aVxELbJAcg=
+20250715100956.sql h1:y9eOaPMpQTlcJppjaGzeuHBTNDwe6sGbxSVU8e7LL1o=

app/controlplane/pkg/data/ent/migrate/migrations/20250715100956.sql

@@ -20,13 +20,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/projectversion"
-
-	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/project"
-
-	"entgo.io/ent/dialect/sql/sqljson"
-
-	"entgo.io/ent/dialect/sql"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent"
@@ -36,10 +29,14 @@ import (
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/organization"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/orginvitation"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/predicate"
+	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/project"
+	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/projectversion"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/user"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/data/ent/workflow"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/pagination"
 
+	"entgo.io/ent/dialect/sql"
+	"entgo.io/ent/dialect/sql/sqljson"
 	"github.com/go-kratos/kratos/v2/log"
 	"github.com/google/uuid"
 )
@@ -231,13 +228,9 @@ func (g GroupRepo) Create(ctx context.Context, orgID uuid.UUID, opts *biz.Create
 			SetDescription(opts.Description).
 			SetOrganizationID(orgID)
 
-		// Only set the member count and add member if userID is provided
+		// Add member if userID is provided
 		if opts.UserID != nil {
-			builder = builder.
-				AddMemberIDs(*opts.UserID).
-				SetMemberCount(1)
-		} else {
-			builder = builder.SetMemberCount(0)
+			builder = builder.AddMemberIDs(*opts.UserID)
 		}
 
 		gr, err := builder.Save(ctx)
@@ -287,6 +280,11 @@ func (g GroupRepo) Create(ctx context.Context, orgID uuid.UUID, opts *biz.Create
 		return nil, fmt.Errorf("failed to create group: %w", err)
 	}
 
+	// Update the member count based on actual query
+	if err := g.UpdateGroupMemberCount(ctx, entGroup.ID); err != nil {
+		g.log.Warnf("failed to update member count for newly created group %s: %v", entGroup.ID, err)
+	}
+
 	return g.FindByOrgAndID(ctx, orgID, entGroup.ID)
 }
 
@@ -474,16 +472,16 @@ func (g GroupRepo) AddMemberToGroup(ctx context.Context, orgID uuid.UUID, groupI
 			}
 		}
 
-		// Increment the member count of the group
-		if err := tx.Group.UpdateOneID(groupID).AddMemberCount(1).Exec(ctx); err != nil {
-			return fmt.Errorf("failed to increment group member count: %w", err)
-		}
-
 		return nil
 	}); err != nil {
 		return nil, fmt.Errorf("failed to add member to group: %w", err)
 	}
 
+	// Update the member count based on actual query after transaction
+	if err := g.UpdateGroupMemberCount(ctx, groupID); err != nil {
+		g.log.Warnf("failed to update member count after adding member to group %s: %v", groupID, err)
+	}
+
 	// Return the newly created membership
 	return g.FindGroupMembershipByGroupAndID(ctx, groupID, userID)
 }
@@ -527,18 +525,18 @@ func (g GroupRepo) RemoveMemberFromGroup(ctx context.Context, orgID uuid.UUID, g
 			}
 		}
 
-		// Decrement the member count of the group
-		if err := tx.Group.UpdateOneID(groupID).AddMemberCount(-1).Exec(ctx); err != nil {
-			return fmt.Errorf("failed to increment group member count: %w", err)
-		}
-
 		return nil
 	})
 
 	if err != nil {
 		return err
 	}
 
+	// Update the member count based on actual query after transaction
+	if err := g.UpdateGroupMemberCount(ctx, groupID); err != nil {
+		g.log.Warnf("failed to update member count after removing member from group %s: %v", groupID, err)
+	}
+
 	return nil
 }
 
@@ -758,3 +756,27 @@ func entGroupMembershipToBiz(gu *ent.GroupMembership) *biz.GroupMembership {
 		DeletedAt:  toTimePtr(gu.DeletedAt),
 	}
 }
+
+// UpdateGroupMemberCount updates the member count of a group based on an actual count query
+// This should be called after membership changes have been committed.
+func (g GroupRepo) UpdateGroupMemberCount(ctx context.Context, groupID uuid.UUID) error {
+	// Count active members in the group
+	count, err := g.data.DB.GroupMembership.Query().
+		Where(
+			groupmembership.GroupIDEQ(groupID),
+			groupmembership.DeletedAtIsNil(),
+		).
+		Count(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to count group members: %w", err)
+	}
+
+	// Update the group's member count to the actual count
+	if _, err := g.data.DB.Group.UpdateOneID(groupID).
+		SetMemberCount(count).
+		Save(ctx); err != nil {
+		return fmt.Errorf("failed to update group member count: %w", err)
+	}
+
+	return nil
+}

app/controlplane/pkg/data/ent/migrate/migrations/atlas.sum

@@ -34,14 +34,16 @@ import (
 )
 
 type MembershipRepo struct {
-	data *Data
-	log  *log.Helper
+	data      *Data
+	log       *log.Helper
+	groupRepo biz.GroupRepo
 }
 
-func NewMembershipRepo(data *Data, logger log.Logger) biz.MembershipRepo {
+func NewMembershipRepo(data *Data, groupRepo biz.GroupRepo, logger log.Logger) biz.MembershipRepo {
 	return &MembershipRepo{
-		data: data,
-		log:  log.NewHelper(logger),
+		data:      data,
+		groupRepo: groupRepo,
+		log:       log.NewHelper(logger),
 	}
 }
 
@@ -253,7 +255,10 @@ func (r *MembershipRepo) Delete(ctx context.Context, id uuid.UUID) error {
 		return fmt.Errorf("failed to get membership: %w", err)
 	}
 
-	return WithTx(ctx, r.data.DB, func(tx *ent.Tx) error {
+	// Prepare a slice to hold group IDs that need to be updated
+	var groupIDs []uuid.UUID
+
+	if trErr := WithTx(ctx, r.data.DB, func(tx *ent.Tx) error {
 		// Delete the specific membership
 		if err := tx.Membership.DeleteOneID(id).Exec(ctx); err != nil {
 			return fmt.Errorf("failed to delete membership: %w", err)
@@ -279,6 +284,21 @@ func (r *MembershipRepo) Delete(ctx context.Context, id uuid.UUID) error {
 			// Remove the user from all groups in the organization by soft-deleting group memberships
 			now := time.Now()
 
+			// Find all group IDs where this user is a member in this organization
+			groupMemberships, grpMemErr := tx.GroupMembership.Query().Where(
+				groupmembership.UserID(userID),
+				groupmembership.DeletedAtIsNil(),
+				groupmembership.HasGroupWith(group.OrganizationID(orgID)),
+			).Select(groupmembership.FieldGroupID).All(ctx)
+			if grpMemErr != nil {
+				return fmt.Errorf("failed to fetch group IDs for user %s in organization %s: %w", userID, orgID, err)
+			}
+
+			// Collect group IDs to update member counts later
+			for _, gm := range groupMemberships {
+				groupIDs = append(groupIDs, gm.GroupID)
+			}
+
 			// Soft delete all group memberships for this user in this organization
 			if _, err := tx.GroupMembership.Update().Where(
 				groupmembership.UserID(userID),
@@ -287,22 +307,27 @@ func (r *MembershipRepo) Delete(ctx context.Context, id uuid.UUID) error {
 			).SetDeletedAt(now).SetUpdatedAt(now).Save(ctx); err != nil {
 				return fmt.Errorf("failed to delete group memberships for user %s in organization %s: %w", userID, orgID, err)
 			}
-
-			// Decrement the member count of each group this user was a member of
-			// We don't need a separate check for groups the user was a maintainer of
-			// because all memberships were already deleted above
-			if _, err := tx.Group.Update().
-				Where(
-					group.HasMembersWith(user.ID(userID)),
-					group.HasOrganizationWith(organization.ID(orgID)),
-				).
-				AddMemberCount(-1).Save(ctx); err != nil {
-				return fmt.Errorf("failed to decrement group member count: %w", err)
-			}
 		}
 
 		return nil
-	})
+	}); trErr != nil {
+		return trErr
+	}
+
+	// For each affected group, update the member count based on actual query
+	updated := map[uuid.UUID]struct{}{}
+	for _, gid := range groupIDs {
+		if _, seen := updated[gid]; seen {
+			// deduplicate group IDs
+			continue
+		}
+		updated[gid] = struct{}{}
+		if err := r.groupRepo.UpdateGroupMemberCount(ctx, gid); err != nil {
+			return fmt.Errorf("failed to update group member count for group %s: %w", gid, err)
+		}
+	}
+
+	return nil
 }
 
 // RBAC methods