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