Merge branch 'fix-api-project-transfer' into 'master'

Fix transferring of project to another group using the API.

Addresses internal issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2397.

See merge request !926

CHANGELOG

@@ -50,6 +50,7 @@ v 7.12.2
   - Correctly show anonymous authorized applications under Profile > Applications.
   - Faster automerge check and merge itself when source and target branches are in same repository
   - Audit log for user authentication
+  - Fix transferring of project to another group using the API.
 
 v 7.12.1
   - Fix error when deleting a user who has projects (Stan Hu)

app/controllers/admin/projects_controller.rb

@@ -23,7 +23,8 @@ class Admin::ProjectsController < Admin::ApplicationController
   end
 
   def transfer
-    ::Projects::TransferService.new(@project, current_user, params.dup).execute
+    namespace = Namespace.find_by(id: params[:new_namespace_id])
+    ::Projects::TransferService.new(@project, current_user, params.dup).execute(namespace)
 
     @project.reload
     redirect_to admin_namespace_project_path(@project.namespace, @project)

app/controllers/projects_controller.rb

@@ -52,10 +52,11 @@ class ProjectsController < ApplicationController
   end
 
   def transfer
-    transfer_params = params.permit(:new_namespace_id)
-    ::Projects::TransferService.new(project, current_user, transfer_params).execute
-    if @project.errors[:namespace_id].present?
-      flash[:alert] = @project.errors[:namespace_id].first
+    namespace = Namespace.find_by(id: params[:new_namespace_id])
+    ::Projects::TransferService.new(project, current_user).execute(namespace)
+
+    if @project.errors[:new_namespace].present?
+      flash[:alert] = @project.errors[:new_namespace].first
     end
   end
 

app/services/projects/transfer_service.rb

@@ -11,19 +11,16 @@ module Projects
     include Gitlab::ShellAdapter
     class TransferError < StandardError; end
 
-    def execute
-      namespace_id = params[:new_namespace_id]
-      namespace = Namespace.find_by(id: namespace_id)
-
-      if allowed_transfer?(current_user, project, namespace)
-        transfer(project, namespace)
+    def execute(new_namespace)
+      if allowed_transfer?(current_user, project, new_namespace)
+        transfer(project, new_namespace)
       else
-        project.errors.add(:namespace, 'is invalid')
+        project.errors.add(:new_namespace, 'is invalid')
         false
       end
     rescue Projects::TransferService::TransferError => ex
       project.reload
-      project.errors.add(:namespace_id, ex.message)
+      project.errors.add(:new_namespace, ex.message)
       false
     end
 

lib/api/groups.rb

@@ -74,9 +74,9 @@ module API
       #   POST /groups/:id/projects/:project_id
       post ":id/projects/:project_id" do
         authenticated_as_admin!
-        group = Group.find(params[:id])
+        group = Group.find_by(id: params[:id])
         project = Project.find(params[:project_id])
-        result = ::Projects::TransferService.new(project, current_user, namespace_id: group.id).execute
+        result = ::Projects::TransferService.new(project, current_user).execute(group)
 
         if result
           present group

spec/services/projects/transfer_service_spec.rb

@@ -8,7 +8,7 @@ describe Projects::TransferService do
   context 'namespace -> namespace' do
     before do
       group.add_owner(user)
-      @result = transfer_project(project, user, new_namespace_id: group.id)
+      @result = transfer_project(project, user, group)
     end
 
     it { expect(@result).to be_truthy }
@@ -17,7 +17,7 @@ describe Projects::TransferService do
 
   context 'namespace -> no namespace' do
     before do
-      @result = transfer_project(project, user, new_namespace_id: nil)
+      @result = transfer_project(project, user, nil)
     end
 
     it { expect(@result).to eq false }
@@ -26,14 +26,14 @@ describe Projects::TransferService do
 
   context 'namespace -> not allowed namespace' do
     before do
-      @result = transfer_project(project, user, new_namespace_id: group.id)
+      @result = transfer_project(project, user, group)
     end
 
     it { expect(@result).to eq false }
     it { expect(project.namespace).to eq(user.namespace) }
   end
 
-  def transfer_project(project, user, params)
-    Projects::TransferService.new(project, user, params).execute
+  def transfer_project(project, user, new_namespace)
+    Projects::TransferService.new(project, user).execute(new_namespace)
   end
 end