Prevent project transfer if a new group is not selected

7 files changed, 82 insertions, 5 deletions

diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index a27abee5431..2112342aebe 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 ProjectEdit 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':
+          new ProjectEdit();
+          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..042ec8e4494
--- /dev/null
+++ b/app/assets/javascripts/project_edit.js
@@ -0,0 +1,16 @@
+export default class ProjectEdit {
+  constructor() {
+    this.transferForm = $('.js-project-transfer-form');
+    this.selectNamespace = $('.js-project-transfer-form').find('.select2');
+
+    this.selectNamespaceChangedWrapper = this.selectNamespaceChanged.bind(this);
+    this.selectNamespace.on('change', this.selectNamespaceChangedWrapper);
+    this.selectNamespaceChanged();
+  }
+
+  selectNamespaceChanged() {
+    const selectedNamespaceValue = this.selectNamespace.val();
+
+    this.transferForm.find(':submit').prop('disabled', !selectedNamespaceValue);
+  }
+}
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index da6e6acd4a7..a5e5d713ebd 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -15,7 +15,12 @@ module Projects
       if allowed_transfer?(current_user, project, new_namespace)
         transfer(project, new_namespace)
       else
-        project.errors.add(:new_namespace, 'is invalid')
+        error_message = if new_namespace.blank? 
+                          'Please select a namespace to transfer the project to'
+                        else
+                          'Transfer failed, please contact an admin'
+                        end
+        project.errors.add(:new_namespace, error_message)
         false
       end
     rescue Projects::TransferService::TransferError => ex
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index dd27e0866de..435d5f4aa87 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 namespace to transfer to
             .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..f117598555b 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -226,6 +226,51 @@ describe ProjectsController do
     end
   end
 
+  describe '#transfer' do
+    render_views
+
+    subject(:project) { create(:project) }
+    let(:admin) { create(:admin) }
+    let(:new_namespace) { create(:namespace) }
+
+    it 'updates namespace' do
+      controller.instance_variable_set(:@project, project)
+      sign_in(admin)
+
+      put :transfer,
+          namespace_id: project.namespace.id,
+          new_namespace_id: new_namespace.id,
+          id: project.id,
+          format: :js
+
+      project.reload
+
+      expect(project.namespace.id).to eq(new_namespace.id)
+      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_id = project.namespace.id
+
+        put :transfer,
+            namespace_id: old_namespace_id,
+            new_namespace_id: nil,
+            id: project.id,
+            format: :js
+
+        project.reload
+
+        expect(project.namespace.id).to eq(old_namespace_id)
+        expect(response).to have_http_status(200) 
+        expect(flash[:alert]).to eq 'Please select a namespace to transfer the project to'
+      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..5c872047bdf 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][0]).to eq 'Please select a namespace to transfer the project to' }
   end
 
   context 'disallow transfering of project with tags' do