Merge branch 'fix-group-without-owner' into 'master'

fix group without owner after transfer

Closes #58086

See merge request gitlab-org/gitlab-ce!25573

app/services/groups/transfer_service.rb

@@ -35,7 +35,10 @@ module Groups
     def proceed_to_transfer
       Group.transaction do
         update_group_attributes
+        ensure_ownership
       end
+
+      true
     end
 
     def ensure_allowed_transfer
@@ -95,6 +98,13 @@ module Groups
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
+    def ensure_ownership
+      return if @new_parent_group
+      return unless @group.owners.empty?
+
+      @group.add_owner(current_user)
+    end
+
     def raise_transfer_error(message)
       raise TransferError, ERROR_MESSAGES[message]
     end

changelogs/unreleased/fix-group-without-owner.yml

+---
+title: fix group without owner after transfer
+merge_request: 25573
+author: Peter Marko
+type: fixed

doc/user/group/index.md

@@ -181,6 +181,7 @@ Please make sure to understand that:
 - You can only transfer the group to a group you manage.
 - You will need to update your local repositories to point to the new location.
 - If the parent group's visibility is lower than the group current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility.
+- Only explicit group membership is transferred, not the inherited membership. If this would leave the group without an owner, the transferring user is added as owner instead.
 
 ## Group settings
 

spec/services/groups/transfer_service_spec.rb

@@ -410,5 +410,34 @@ describe Groups::TransferService, :postgresql do
         end
       end
     end
+
+    context 'when transferring a subgroup into root group' do
+      let(:group) { create(:group, :public, :nested) }
+      let(:subgroup) { create(:group, :public, parent: group) }
+      let(:transfer_service) { described_class.new(subgroup, user) }
+
+      it 'ensures there is still an owner for the transferred group' do
+        expect(subgroup.owners).to be_empty
+
+        transfer_service.execute(nil)
+        subgroup.reload
+
+        expect(subgroup.owners).to match_array(user)
+      end
+
+      context 'when group has explicit owner' do
+        let(:another_owner) { create(:user) }
+        let!(:another_member) { create(:group_member, :owner, group: subgroup, user: another_owner) }
+
+        it 'does not add additional owner' do
+          expect(subgroup.owners).to match_array(another_owner)
+
+          transfer_service.execute(nil)
+          subgroup.reload
+
+          expect(subgroup.owners).to match_array(another_owner)
+        end
+      end
+    end
   end
 end