diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index f64e327416a..94185605ab9 100644 --- a/app/services/groups/transfer_service.rb +++ b/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 diff --git a/changelogs/unreleased/fix-group-without-owner.yml b/changelogs/unreleased/fix-group-without-owner.yml new file mode 100644 index 00000000000..884f1b3a08a --- /dev/null +++ b/changelogs/unreleased/fix-group-without-owner.yml @@ -0,0 +1,5 @@ +--- +title: fix group without owner after transfer +merge_request: 25573 +author: Peter Marko +type: fixed diff --git a/doc/user/group/index.md b/doc/user/group/index.md index 300e0115c60..1325a529fa1 100644 --- a/doc/user/group/index.md +++ b/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 diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 6b48c993c57..79d504b9b45 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/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