Also add children groups as members

app/services/groups/create_inherited_roles_service.rb

@@ -85,6 +85,7 @@ module Groups
           FROM #{MemberRole.table_name} member_roles
           JOIN #{Member.table_name} members
           ON members.id = member_roles.member_id AND members.user_id = :group_id
+          WHERE member_roles.inherited_from IS NULL
         ),
         -- find members that already exist
         existing_members AS (

app/services/groups/update_roles_service.rb

@@ -45,7 +45,10 @@ module Groups
 
     def modify_members_and_roles(params)
       member = params.fetch(:member)
-      user_ids = params.fetch(:user_ids) { model.self_and_descendants.flat_map(&:user_ids).uniq }
+      user_ids = params.fetch(:user_ids) do
+        group_ids = model.descendants.pluck(:id)
+        (model.self_and_descendants.flat_map(&:user_ids) + group_ids).uniq
+      end
 
       sql_query = ::OpenProject::SqlSanitization
                     .sanitize(update_roles_cte,
@@ -73,6 +76,7 @@ module Groups
                  member_roles.id
           FROM #{MemberRole.table_name} member_roles
           WHERE member_roles.member_id = :member_id
+          AND member_roles.inherited_from IS NULL
         ),
         -- delete all roles assigned to users that group no longer has but keep those that the user
         -- has independently of the group (not inherited) or inherited from a different group

app/services/groups/update_service.rb

@@ -113,13 +113,15 @@ class Groups::UpdateService < BaseServices::Update
   end
 
   def propagate_ancestor_memberships
+    group_ids = model.self_and_descendants.pluck(:id)
     user_ids = model.self_and_descendants.flat_map(&:user_ids).uniq
-    return if user_ids.empty?
+    principal_ids = (user_ids + group_ids).uniq
+    return if principal_ids.empty?
 
     model.ancestors.each do |ancestor|
       Groups::CreateInheritedRolesService
         .new(ancestor, current_user: user)
-        .call(user_ids:)
+        .call(user_ids: principal_ids)
     end
   end
 
@@ -128,16 +130,18 @@ class Groups::UpdateService < BaseServices::Update
     return unless former_parent
 
     affected_users = model.self_and_descendants.flat_map(&:users).uniq
-    return if affected_users.empty?
+    affected_group_ids = model.self_and_descendants.pluck(:id)
+    return if affected_users.empty? && affected_group_ids.empty?
 
     former_parent.self_and_ancestors.each do |ancestor|
       users_not_in_ancestor = affected_users.reject { |u| ancestor.user_ids.include?(u.id) }
-      next if users_not_in_ancestor.empty?
+      principal_ids_to_clean = users_not_in_ancestor.map(&:id) + affected_group_ids
+      next if principal_ids_to_clean.empty?
 
       role_ids_to_clean = MemberRole
         .joins(:member)
         .where(inherited_from: ancestor.members.joins(:member_roles).select("member_roles.id"))
-        .where(members: { user_id: users_not_in_ancestor.map(&:id) })
+        .where(members: { user_id: principal_ids_to_clean })
         .pluck(:id)
 
       next if role_ids_to_clean.empty?

app/services/members/create_service.rb

@@ -46,15 +46,35 @@ class Members::CreateService < BaseServices::Create
 
   protected
 
+  # When a Group is being added as a member to a project, an inherited Member
+  # may already exist (created by ancestor group membership propagation).
+  # In that case, find the existing member so we add direct roles to it
+  # rather than failing on the uniqueness constraint.
+  def instance(params)
+    principal = params[:principal]
+    if principal.is_a?(Group)
+      Member.find_or_initialize_by(
+        user_id: principal.id,
+        project_id: params[:project_id],
+        entity_type: params[:entity_type],
+        entity_id: params[:entity_id]
+      )
+    else
+      super
+    end
+  end
+
   def add_group_memberships(member)
     return unless member.principal.is_a?(Group)
 
     project_ids = member.project_id.nil? ? nil : [member.project_id]
+    group_ids = member.principal.descendants.pluck(:id)
     user_ids = member.principal.self_and_descendants.flat_map(&:user_ids).uniq
+    principal_ids = (user_ids + group_ids).uniq
 
     Groups::CreateInheritedRolesService
       .new(member.principal, current_user: user, contract_class: EmptyContract)
-      .call(user_ids: user_ids, send_notifications: false, project_ids:)
+      .call(user_ids: principal_ids, send_notifications: false, project_ids:)
   end
 
   def event_type

app/services/members/update_service.rb

@@ -51,11 +51,13 @@ class Members::UpdateService < BaseServices::Update
   end
 
   def update_group_roles(member)
+    group_ids = member.principal.descendants.pluck(:id)
     user_ids = member.principal.self_and_descendants.flat_map(&:user_ids).uniq
+    principal_ids = (user_ids + group_ids).uniq
 
     Groups::UpdateRolesService
       .new(member.principal, current_user: user, contract_class: EmptyContract)
-      .call(member:, user_ids:, send_notifications: send_notifications?, message: notification_message)
+      .call(member:, user_ids: principal_ids, send_notifications: send_notifications?, message: notification_message)
   end
 
   def event_type

spec/services/groups/hierarchy_membership_integration_spec.rb

@@ -553,4 +553,189 @@ RSpec.describe "Group hierarchy membership propagation", type: :model do
       expect(new_user.memberships.find_by(project:)&.roles).to contain_exactly(root_role, mid_role)
     end
   end
+
+  # ---------------------------------------------------------------------------
+  # Child group membership propagation — descendant groups themselves get
+  # inherited Member records, not just the users within them
+  # ---------------------------------------------------------------------------
+
+  describe "child group membership propagation" do
+    describe "Members::CreateService" do
+      it "creates inherited memberships for descendant groups when a parent group is added to a project" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [role.id])
+
+        mid_member = Member.find_by(principal: mid_group, project:)
+        leaf_member = Member.find_by(principal: leaf_group, project:)
+
+        expect(mid_member).to be_present
+        expect(mid_member.roles).to contain_exactly(role)
+        expect(mid_member.member_roles.all? { |mr| mr.inherited_from.present? }).to be(true)
+
+        expect(leaf_member).to be_present
+        expect(leaf_member.roles).to contain_exactly(role)
+        expect(leaf_member.member_roles.all? { |mr| mr.inherited_from.present? }).to be(true)
+      end
+
+      it "does not create inherited memberships for ancestor groups" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        create(:group, members: [leaf_user], parent: mid_group)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: mid_group, project_id: project.id, role_ids: [role.id])
+
+        expect(Member.find_by(principal: root_group, project:)).to be_nil
+      end
+    end
+
+    describe "Members::UpdateService" do
+      it "updates inherited roles on descendant group members when the parent group's roles change" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        second_role = create(:project_role)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [role.id])
+
+        group_member = Member.find_by!(principal: root_group, project:)
+
+        Members::UpdateService
+          .new(user: admin, model: group_member)
+          .call(role_ids: [role.id, second_role.id])
+
+        expect(Member.find_by(principal: mid_group, project:).roles).to contain_exactly(role, second_role)
+        expect(Member.find_by(principal: leaf_group, project:).roles).to contain_exactly(role, second_role)
+      end
+    end
+
+    describe "Members::DeleteService" do
+      it "removes inherited memberships from descendant groups when the parent group's membership is deleted" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [role.id])
+
+        group_member = Member.find_by!(principal: root_group, project:)
+
+        Members::DeleteService
+          .new(user: admin, model: group_member)
+          .call
+
+        expect(Member.find_by(principal: mid_group, project:)).to be_nil
+        expect(Member.find_by(principal: leaf_group, project:)).to be_nil
+      end
+    end
+
+    describe "parent change" do
+      it "propagates ancestor memberships to child groups when a parent is assigned" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user])
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [role.id])
+
+        Groups::UpdateService
+          .new(user: admin, model: mid_group)
+          .call(parent_id: root_group.id)
+
+        expect(Member.find_by(principal: mid_group, project:)&.roles).to contain_exactly(role)
+        expect(Member.find_by(principal: leaf_group, project:)&.roles).to contain_exactly(role)
+      end
+
+      it "cleans up inherited child group memberships when the parent link is broken" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [role.id])
+
+        # Verify child groups have memberships before breaking the link
+        expect(Member.find_by(principal: mid_group, project:)).to be_present
+        expect(Member.find_by(principal: leaf_group, project:)).to be_present
+
+        Groups::UpdateService
+          .new(user: admin, model: mid_group)
+          .call(parent_id: nil)
+
+        expect(Member.find_by(principal: mid_group, project:)).to be_nil
+        expect(Member.find_by(principal: leaf_group, project:)).to be_nil
+      end
+    end
+
+    describe "Members::DeleteService with pre-existing child group membership" do
+      it "retains the child group's own membership when the parent group's membership is deleted" do
+        mid_role   = create(:project_role)
+        root_role  = create(:project_role)
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        # mid_group gets its own direct membership first
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: mid_group, project_id: project.id, role_ids: [mid_role.id])
+
+        # Then root_group is added — this propagates root_role to mid_group, leaf_group, and all users
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [root_role.id])
+
+        # mid_group now has both its direct mid_role and inherited root_role
+        expect(Member.find_by(principal: mid_group, project:).roles).to contain_exactly(mid_role, root_role)
+        expect(Member.find_by(principal: leaf_group, project:)&.roles).to contain_exactly(mid_role, root_role)
+
+        # Delete root_group's membership
+        root_member = Member.find_by!(principal: root_group, project:)
+        Members::DeleteService
+          .new(user: admin, model: root_member)
+          .call
+
+        # mid_group keeps its own direct membership with mid_role
+        expect(Member.find_by(principal: mid_group, project:)&.roles).to contain_exactly(mid_role)
+        # leaf_group keeps the inherited mid_role from mid_group
+        expect(Member.find_by(principal: leaf_group, project:)&.roles).to contain_exactly(mid_role)
+        # Users also retain mid_role
+        expect(mid_user.memberships.find_by(project:)&.roles).to contain_exactly(mid_role)
+        expect(leaf_user.memberships.find_by(project:)&.roles).to contain_exactly(mid_role)
+      end
+    end
+
+    describe "user removal from group" do
+      it "does not affect child group memberships when a user is removed from a group" do
+        root_group = create(:group, members: [root_user])
+        mid_group  = create(:group, members: [mid_user], parent: root_group)
+        leaf_group = create(:group, members: [leaf_user], parent: mid_group)
+
+        Members::CreateService
+          .new(user: admin)
+          .call(principal: root_group, project_id: project.id, role_ids: [role.id])
+
+        # Remove leaf_user from leaf_group
+        Groups::UpdateService
+          .new(user: admin, model: leaf_group)
+          .call(remove_user_ids: [leaf_user.id])
+
+        # Child group memberships should remain intact
+        expect(Member.find_by(principal: mid_group, project:)&.roles).to contain_exactly(role)
+        expect(Member.find_by(principal: leaf_group, project:)&.roles).to contain_exactly(role)
+      end
+    end
+  end
 end