Merge branch '230401-improve-transfer-service' into 'master'

Refactor transfer service code

Closes #230401

See merge request gitlab-org/gitlab!38458

app/services/groups/transfer_service.rb

@@ -55,9 +55,7 @@ module Groups
 
       npm_packages = ::Packages::GroupPackagesFinder.new(current_user, group, package_type: :npm).execute
 
-      return true if different_root_ancestor? && npm_packages.exists?
-
-      false
+      different_root_ancestor? && npm_packages.exists?
     end
 
     def different_root_ancestor?

spec/models/project_spec.rb

@@ -6198,38 +6198,40 @@ RSpec.describe Project do
 
     subject { project.has_packages?(package_type) }
 
-    shared_examples 'returning true examples' do
-      let!(:package) { create("#{package_type}_package", project: project) }
+    shared_examples 'has_package' do
+      context 'package of package_type exists' do
+        let!(:package) { create("#{package_type}_package", project: project) }
 
-      it { is_expected.to be true }
-    end
+        it { is_expected.to be true }
+      end
 
-    shared_examples 'returning false examples' do
-      it { is_expected.to be false }
+      context 'package of package_type does not exist' do
+        it { is_expected.to be false }
+      end
     end
 
     context 'with maven packages' do
-      it_behaves_like 'returning true examples' do
+      it_behaves_like 'has_package' do
         let(:package_type) { :maven }
       end
     end
 
     context 'with npm packages' do
-      it_behaves_like 'returning true examples' do
+      it_behaves_like 'has_package' do
         let(:package_type) { :npm }
       end
     end
 
     context 'with conan packages' do
-      it_behaves_like 'returning true examples' do
+      it_behaves_like 'has_package' do
         let(:package_type) { :conan }
       end
     end
 
-    context 'with no package type' do
-      it_behaves_like 'returning false examples' do
-        let(:package_type) { nil }
-      end
+    context 'calling has_package? with nil' do
+      let(:package_type) { nil }
+
+      it { is_expected.to be false }
     end
   end
 

spec/services/groups/transfer_service_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Groups::TransferService do
   let(:transfer_service) { described_class.new(group, user) }
 
   context 'handling packages' do
-    let(:group) { create(:group, :public) }
+    let_it_be(:group) { create(:group, :public) }
     let(:project) { create(:project, :public, namespace: group) }
     let(:new_group) { create(:group, :public) }
 
@@ -28,6 +28,7 @@ RSpec.describe Groups::TransferService do
           transfer_service.execute(new_group)
 
           expect(transfer_service.error).to eq('Transfer failed: Group contains projects with NPM packages.')
+          expect(group.parent).not_to eq(new_group)
         end
       end
 
@@ -49,7 +50,7 @@ RSpec.describe Groups::TransferService do
           it 'allows transfer' do
             transfer_service.execute(new_group)
 
-            expect(transfer_service.error).not_to be
+            expect(transfer_service.error).to be nil
             expect(group.parent).to eq(new_group)
           end
         end
@@ -69,7 +70,7 @@ RSpec.describe Groups::TransferService do
         it 'allows transfer' do
           transfer_service.execute(nil)
 
-          expect(transfer_service.error).not_to be
+          expect(transfer_service.error).to be nil
           expect(group.parent).to be_nil
         end
       end