refactor(group): migrate group creation to membership package

cmd/serve.go

@@ -432,10 +432,11 @@ func buildAPIDependencies(
 		authnService, serviceUserService, groupService, roleService)
 
 	membershipService := membership.NewService(logger, policyService, relationService, roleService, organizationService, userService, projectService, groupService, serviceUserService, auditRecordRepository)
-	// Setter injection: org → membership is circular (membership needs org for validation,
-	// org needs membership for Create/AdminCreate). Break the cycle with a post-init setter.
+	// Setter injection: org/group → membership is circular (membership needs them
+	// for validation; they need membership for Create). Break the cycle post-init.
 	organizationService.SetMembershipService(membershipService)
 	serviceUserService.SetMembershipService(membershipService)
+	groupService.SetMembershipService(membershipService)
 
 	orgKycRepository := postgres.NewOrgKycRepository(dbc)
 	orgKycService := kyc.NewService(orgKycRepository)

core/group/service.go

@@ -38,11 +38,16 @@ type PolicyService interface {
 	GroupMemberCount(ctx context.Context, ids []string) ([]policy.MemberCount, error)
 }
 
+type MembershipService interface {
+	OnGroupCreated(ctx context.Context, groupID, orgID, creatorID, creatorType string) error
+}
+
 type Service struct {
-	repository      Repository
-	relationService RelationService
-	authnService    AuthnService
-	policyService   PolicyService
+	repository        Repository
+	relationService   RelationService
+	authnService      AuthnService
+	policyService     PolicyService
+	membershipService MembershipService
 }
 
 func NewService(repository Repository, relationService RelationService,
@@ -55,6 +60,12 @@ func NewService(repository Repository, relationService RelationService,
 	}
 }
 
+// SetMembershipService sets the membership dependency after construction to break
+// the circular init order between group and membership services.
+func (s *Service) SetMembershipService(ms MembershipService) {
+	s.membershipService = ms
+}
+
 func (s Service) Create(ctx context.Context, grp Group) (Group, error) {
 	principal, err := s.authnService.GetPrincipal(ctx)
 	if err != nil {
@@ -66,17 +77,7 @@ func (s Service) Create(ctx context.Context, grp Group) (Group, error) {
 		return Group{}, err
 	}
 
-	// attach group to org
-	if err = s.addAsOrgMember(ctx, newGroup); err != nil {
-		return Group{}, err
-	}
-	// add relationship between group to org
-	if err = s.addOrgToGroup(ctx, newGroup); err != nil {
-		return Group{}, err
-	}
-
-	// attach current user to group as owner
-	if err = s.addOwner(ctx, newGroup.ID, principal); err != nil {
+	if err = s.membershipService.OnGroupCreated(ctx, newGroup.ID, newGroup.OrganizationID, principal.ID, principal.Type); err != nil {
 		return Group{}, err
 	}
 
@@ -190,36 +191,6 @@ func (s Service) AddMember(ctx context.Context, groupID string, principal authen
 	return nil
 }
 
-// addOwner adds a user as an owner of group by creating a policy of owner role and an owner relation
-func (s Service) addOwner(ctx context.Context, groupID string, principal authenticate.Principal) error {
-	pol := policy.Policy{
-		RoleID:        schema.GroupOwnerRole,
-		ResourceID:    groupID,
-		ResourceType:  schema.GroupNamespace,
-		PrincipalID:   principal.ID,
-		PrincipalType: principal.Type,
-	}
-	if _, err := s.policyService.Create(ctx, pol); err != nil {
-		return err
-	}
-	// then create a relation between group and user
-	rel := relation.Relation{
-		Object: relation.Object{
-			ID:        groupID,
-			Namespace: schema.GroupNamespace,
-		},
-		Subject: relation.Subject{
-			ID:        principal.ID,
-			Namespace: principal.Type,
-		},
-		RelationName: schema.OwnerRelationName,
-	}
-	if _, err := s.relationService.Create(ctx, rel); err != nil {
-		return err
-	}
-	return nil
-}
-
 // add a policy to user as member of group
 func (s Service) addMemberPolicy(ctx context.Context, groupID string, principal authenticate.Principal) error {
 	pol := policy.Policy{
@@ -235,51 +206,6 @@ func (s Service) addMemberPolicy(ctx context.Context, groupID string, principal
 	return nil
 }
 
-// addOrgToGroup creates an inverse relation that connects group to org
-func (s Service) addOrgToGroup(ctx context.Context, team Group) error {
-	rel := relation.Relation{
-		Object: relation.Object{
-			ID:        team.ID,
-			Namespace: schema.GroupNamespace,
-		},
-		Subject: relation.Subject{
-			ID:        team.OrganizationID,
-			Namespace: schema.OrganizationNamespace,
-		},
-		RelationName: schema.OrganizationRelationName,
-	}
-
-	_, err := s.relationService.Create(ctx, rel)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-// addAsOrgMember connects group as a member to org
-func (s Service) addAsOrgMember(ctx context.Context, team Group) error {
-	rel := relation.Relation{
-		Object: relation.Object{
-			ID:        team.OrganizationID,
-			Namespace: schema.OrganizationNamespace,
-		},
-		Subject: relation.Subject{
-			ID:              team.ID,
-			Namespace:       schema.GroupNamespace,
-			SubRelationName: schema.MemberRelationName,
-		},
-		RelationName: schema.MemberRelationName,
-	}
-
-	_, err := s.relationService.Create(ctx, rel)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // ListByOrganization will be useful for nested groups but we don't do that at the moment
 // so it will not be directly used
 func (s Service) ListByOrganization(ctx context.Context, id string) ([]Group, error) {

core/group/service_test.go

@@ -21,75 +21,35 @@ import (
 )
 
 func TestService_Create(t *testing.T) {
-	t.Run("should create group successfully by adding member to org, adding relation between group and org, and making current user owner", func(t *testing.T) {
+	t.Run("should create group and delegate hierarchy + owner wiring to membership", func(t *testing.T) {
 		mockRepo := mocks.NewRepository(t)
 		mockAuthnSvc := mocks.NewAuthnService(t)
 		mockRelationSvc := mocks.NewRelationService(t)
 		mockPolicySvc := mocks.NewPolicyService(t)
+		mockMembershipSvc := mocks.NewMembershipService(t)
 
 		svc := group.NewService(mockRepo, mockRelationSvc, mockAuthnSvc, mockPolicySvc)
+		svc.SetMembershipService(mockMembershipSvc)
 
-		mockUserID := uuid.New()
+		mockUserID := uuid.New().String()
 		mockAuthnSvc.On("GetPrincipal", mock.Anything).Return(authenticate.Principal{
-			ID:   mockUserID.String(),
-			Type: "user",
-			User: &user.User{
-				ID: mockUserID.String(),
-			},
+			ID:   mockUserID,
+			Type: schema.UserPrincipal,
+			User: &user.User{ID: mockUserID},
 		}, nil)
 
 		groupParam := group.Group{
 			Name:           "test-group",
 			Title:          "Test Group",
 			OrganizationID: uuid.New().String(),
 		}
-
 		groupInRepo := groupParam
 		groupInRepo.ID = uuid.New().String()
 		mockRepo.On("Create", mock.Anything, groupParam).Return(groupInRepo, nil)
 
-		// when adding group as org member
-		mockRelationSvc.On("Create", mock.Anything, mock.AnythingOfType("relation.Relation")).Run(func(args mock.Arguments) {
-			arg := args.Get(1)
-			r := arg.(relation.Relation)
-			assert.Equal(t, r.Object.ID, groupInRepo.OrganizationID)
-			assert.Equal(t, r.Subject.ID, groupInRepo.ID)
-			assert.Equal(t, r.RelationName, schema.MemberRelationName)
-		}).Return(relation.Relation{}, nil).Once()
-
-		// when adding group to org
-		mockRelationSvc.On("Create", mock.Anything, mock.AnythingOfType("relation.Relation")).Run(func(args mock.Arguments) {
-			arg := args.Get(1)
-			r := arg.(relation.Relation)
-			assert.Equal(t, r.Object.ID, groupInRepo.ID)
-			assert.Equal(t, r.Subject.ID, groupInRepo.OrganizationID)
-			assert.Equal(t, r.RelationName, schema.OrganizationRelationName)
-		}).Return(relation.Relation{}, nil).Once()
-
-		// when adding current user as group owner
-		mockPolicySvc.On("Create", mock.Anything, mock.AnythingOfType("policy.Policy")).Run(func(args mock.Arguments) {
-			arg := args.Get(1)
-			r := arg.(policy.Policy)
-			assert.Equal(t, r.RoleID, schema.GroupOwnerRole)
-			assert.Equal(t, r.ResourceID, groupInRepo.ID)
-			assert.Equal(t, r.ResourceType, schema.GroupNamespace)
-			assert.Equal(t, r.PrincipalID, mockUserID.String())
-			assert.Equal(t, r.PrincipalType, "user")
-		}).Return(policy.Policy{}, nil).Once()
-
-		// adding relation between group and user
-		mockRelationSvc.On("Create", mock.Anything, mock.AnythingOfType("relation.Relation")).Run(func(args mock.Arguments) {
-			arg := args.Get(1)
-			r := arg.(relation.Relation)
-			assert.Equal(t, r.Object.ID, groupInRepo.ID)
-			assert.Equal(t, r.Object.Namespace, schema.GroupNamespace)
-			assert.Equal(t, r.Subject.ID, mockUserID.String())
-			assert.Equal(t, r.Subject.Namespace, "user")
-			assert.Equal(t, r.RelationName, schema.OwnerRelationName)
-		}).Return(relation.Relation{}, nil).Once()
+		mockMembershipSvc.EXPECT().OnGroupCreated(mock.Anything, groupInRepo.ID, groupInRepo.OrganizationID, mockUserID, schema.UserPrincipal).Return(nil)
 
 		grp, err := svc.Create(context.Background(), groupParam)
-
 		assert.Nil(t, err)
 		assert.Equal(t, grp.Name, groupParam.Name)
 	})
@@ -108,6 +68,33 @@ func TestService_Create(t *testing.T) {
 		assert.NotNil(t, err)
 		assert.Equal(t, strings.Contains(err.Error(), authenticate.ErrInvalidID.Error()), true)
 	})
+
+	t.Run("should propagate error from membership.OnGroupCreated", func(t *testing.T) {
+		mockRepo := mocks.NewRepository(t)
+		mockAuthnSvc := mocks.NewAuthnService(t)
+		mockRelationSvc := mocks.NewRelationService(t)
+		mockPolicySvc := mocks.NewPolicyService(t)
+		mockMembershipSvc := mocks.NewMembershipService(t)
+
+		svc := group.NewService(mockRepo, mockRelationSvc, mockAuthnSvc, mockPolicySvc)
+		svc.SetMembershipService(mockMembershipSvc)
+
+		mockUserID := uuid.New().String()
+		mockAuthnSvc.On("GetPrincipal", mock.Anything).Return(authenticate.Principal{
+			ID:   mockUserID,
+			Type: schema.UserPrincipal,
+			User: &user.User{ID: mockUserID},
+		}, nil)
+
+		groupParam := group.Group{Name: "g", OrganizationID: uuid.New().String()}
+		groupInRepo := groupParam
+		groupInRepo.ID = uuid.New().String()
+		mockRepo.On("Create", mock.Anything, groupParam).Return(groupInRepo, nil)
+		mockMembershipSvc.EXPECT().OnGroupCreated(mock.Anything, groupInRepo.ID, groupInRepo.OrganizationID, mockUserID, schema.UserPrincipal).Return(errors.New("spicedb down"))
+
+		_, err := svc.Create(context.Background(), groupParam)
+		assert.ErrorContains(t, err, "spicedb down")
+	})
 }
 
 func TestService_Get(t *testing.T) {