Automatic merge of gitlab-org/gitlab-ce master

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/ci/yaml/README.md

@@ -1653,7 +1653,7 @@ include:
 
 ### `include:file`
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/53903) in GitLab 11.9.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/53903) in GitLab 11.7.
 
 To include files from another private project under the same GitLab instance,
 use `include:file`. This file is referenced using full  paths relative to the

doc/user/group/index.md

@@ -214,6 +214,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
 

lib/gitlab/checks/branch_check.rb

@@ -46,7 +46,9 @@ module Gitlab
           end
         end
 
-        if creation? && protected_branch_creation_enabled?
+        if project.empty_repo?
+          protected_branch_push_checks
+        elsif creation? && protected_branch_creation_enabled?
           protected_branch_creation_checks
         elsif deletion?
           protected_branch_deletion_checks

spec/lib/gitlab/checks/branch_check_spec.rb

@@ -48,10 +48,28 @@ describe Gitlab::Checks::BranchCheck do
       context 'when project repository is empty' do
         let(:project) { create(:project) }
 
-        it 'raises an error if the user is not allowed to push to protected branches' do
-          expect(user_access).to receive(:can_push_to_branch?).and_return(false)
+        context 'user is not allowed to push to protected branches' do
+          before do
+            allow(user_access)
+              .to receive(:can_push_to_branch?)
+              .and_return(false)
+          end
+
+          it 'raises an error' do
+            expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /Ask a project Owner or Maintainer to create a default branch/)
+          end
+        end
 
-          expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /Ask a project Owner or Maintainer to create a default branch/)
+        context 'user is allowed to push to protected branches' do
+          before do
+            allow(user_access)
+              .to receive(:can_push_to_branch?)
+              .and_return(true)
+          end
+
+          it 'allows branch creation' do
+            expect { subject.validate! }.not_to raise_error
+          end
         end
       end
 

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