Merge branch 'prevent-project-transfer' into 'master'

Prevent project transfer if a new group is not selected

Closes #25455

See merge request !11214

7 files changed, 74 insertions, 9 deletions

diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index a27abee5431..2090a7e12d6 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -40,6 +40,7 @@ import Group from './group';
 import GroupName from './group_name';
 import GroupsList from './groups_list';
 import ProjectsList from './projects_list';
+import setupProjectEdit from './project_edit';
 import MiniPipelineGraph from './mini_pipeline_graph_dropdown';
 import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater';
 import Landing from './landing';
@@ -264,6 +265,9 @@ import ShortcutsBlob from './shortcuts_blob';
             new BlobViewer();
           }
           break;
+        case 'projects:edit':
+          setupProjectEdit();
+          break;
         case 'projects:pipelines:builds':
         case 'projects:pipelines:failures':
         case 'projects:pipelines:show':
diff --git a/app/assets/javascripts/project_edit.js b/app/assets/javascripts/project_edit.js
new file mode 100644
index 00000000000..d7d284b6c86
--- /dev/null
+++ b/app/assets/javascripts/project_edit.js
@@ -0,0 +1,9 @@
+export default function setupProjectEdit() {
+  const $transferForm = $('.js-project-transfer-form');
+  const $selectNamespace = $transferForm.find('.select2');
+
+  $selectNamespace.on('change', () => {
+    $transferForm.find(':submit').prop('disabled', !$selectNamespace.val());
+  });
+  $selectNamespace.trigger('change');
+}
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index da6e6acd4a7..1c24b27a870 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -12,12 +12,13 @@ module Projects
     TransferError = Class.new(StandardError)
 
     def execute(new_namespace)
-      if allowed_transfer?(current_user, project, new_namespace)
-        transfer(project, new_namespace)
-      else
-        project.errors.add(:new_namespace, 'is invalid')
-        false
+      if new_namespace.blank?
+        raise TransferError, 'Please select a new namespace for your project.'
       end
+      unless allowed_transfer?(current_user, project, new_namespace)
+        raise TransferError, 'Transfer failed, please contact an admin.'
+      end
+      transfer(project, new_namespace)
     rescue Projects::TransferService::TransferError => ex
       project.reload
       project.errors.add(:new_namespace, ex.message)
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index dd27e0866de..f5549d7f4cd 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -246,14 +246,16 @@
     .row.prepend-top-default
       .col-lg-3
         %h4.prepend-top-0.danger-title
-          Transfer project
+          Transfer project to new group
+        %p.append-bottom-0
+          Please select the group you want to transfer this project to in the dropdown to the right.
       .col-lg-9
-        = form_for([@project.namespace.becomes(Namespace), @project], url: transfer_namespace_project_path(@project.namespace, @project), method: :put, remote: true) do |f|
+        = form_for([@project.namespace.becomes(Namespace), @project], url: transfer_namespace_project_path(@project.namespace, @project), method: :put, remote: true, html: { class: 'js-project-transfer-form' } ) do |f|
           .form-group
             = label_tag :new_namespace_id, nil, class: 'label-light' do
-              %span Namespace
+              %span  Select a new namespace
             .form-group
-              = select_tag :new_namespace_id, namespaces_options(@project.namespace_id), { prompt: 'Choose a project namespace', class: 'select2' }
+              = select_tag :new_namespace_id, namespaces_options(nil), include_blank: true, class: 'select2'
             %ul
               %li Be careful. Changing the project's namespace can have unintended side effects.
               %li You can only transfer the project to namespaces you manage.
diff --git a/changelogs/unreleased/prevent-project-transfer.yml b/changelogs/unreleased/prevent-project-transfer.yml
new file mode 100644
index 00000000000..a5c74676aab
--- /dev/null
+++ b/changelogs/unreleased/prevent-project-transfer.yml
@@ -0,0 +1,4 @@
+---
+title: Prevent project transfers if a new group is not selected
+merge_request:
+author:
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index a8be6768a47..4f6fc6691be 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -226,6 +226,50 @@ describe ProjectsController do
     end
   end
 
+  describe '#transfer' do
+    render_views
+
+    let(:project) { create(:project) }
+    let(:admin) { create(:admin) }
+    let(:new_namespace) { create(:namespace) }
+
+    it 'updates namespace' do
+      sign_in(admin)
+
+      put :transfer,
+          namespace_id: project.namespace.path,
+          new_namespace_id: new_namespace.id,
+          id: project.path,
+          format: :js
+
+      project.reload
+
+      expect(project.namespace).to eq(new_namespace)
+      expect(response).to have_http_status(200)
+    end
+
+    context 'when new namespace is empty' do
+      it 'project namespace is not changed' do
+        controller.instance_variable_set(:@project, project)
+        sign_in(admin)
+
+        old_namespace = project.namespace
+
+        put :transfer,
+            namespace_id: old_namespace.path,
+            new_namespace_id: nil,
+            id: project.path,
+            format: :js
+
+        project.reload
+
+        expect(project.namespace).to eq(old_namespace)
+        expect(response).to have_http_status(200)
+        expect(flash[:alert]).to eq 'Please select a new namespace for your project.'
+      end
+    end
+  end
+
   describe "#destroy" do
     let(:admin) { create(:admin) }
 
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 29ccce59c53..b957517c715 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -26,6 +26,7 @@ describe Projects::TransferService, services: true do
 
     it { expect(@result).to eq false }
     it { expect(project.namespace).to eq(user.namespace) }
+    it { expect(project.errors.messages[:new_namespace].first).to eq 'Please select a new namespace for your project.' }
   end
 
   context 'disallow transfering of project with tags' do