Backup from code review commentsfj-41900-import-endpoint-with-overwrite-support

26 files changed, 232 insertions, 167 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index b4cbcae1988..b2659f0d6b3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -631,7 +631,7 @@ class Project < ActiveRecord::Base
     valid?(:import_url) || errors.messages[:import_url].nil?
   end
 
-  def create_or_update_import_data(data: nil, credentials: nil, original_name: nil, original_path: nil)
+  def create_or_update_import_data(data: nil, credentials: nil)
     return if import_url.present? && !valid_import_url?
 
     project_import_data = import_data || build_import_data
@@ -644,9 +644,6 @@ class Project < ActiveRecord::Base
       project_import_data.credentials ||= {}
       project_import_data.credentials = project_import_data.credentials.merge(credentials)
     end
-
-    project_import_data.original_name = original_name
-    project_import_data.original_path = original_path
   end
 
   def import?
@@ -1469,7 +1466,7 @@ class Project < ActiveRecord::Base
   end
 
   def rename_repo_notify!
-    # When we import a project overwritting the original project, there
+    # When we import a project overwriting the original project, there
     # is a move operation. In that case we don't want to send the instructions.
     send_move_instructions(full_path_was) unless started?
     expires_full_path_cache
diff --git a/app/services/projects/move_service.rb b/app/services/projects/base_move_relations_service.rb
index 20d4c013807..c08ec489365 100644
--- a/app/services/projects/move_service.rb
+++ b/app/services/projects/base_move_relations_service.rb
@@ -1,5 +1,5 @@
 module Projects
-  class MoveService < BaseService
+  class BaseMoveRelationsService < BaseService
     def execute(source_project)
       return if source_project.blank? || !can_move_both_projects?(source_project)
 
@@ -9,8 +9,8 @@ module Projects
     private
 
     def can_move_both_projects?(source_project)
-      can?(current_user, :owner_access, source_project) &&
-        can?(current_user, :owner_access, project)
+      can?(current_user, :admin_namespace, source_project.namespace) &&
+        can?(current_user, :admin_namespace, project.namespace)
     end
   end
 end
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 7f595ad22ad..ed90aa914cb 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -122,7 +122,7 @@ module Projects
       Project.transaction do
         rename_to_tmp_name_path if @overwrite && project_with_same_full_path?
 
-        @project.create_or_update_import_data(@import_data.slice(:data, :credentials, :original_name, :original_path))
+        @project.create_or_update_import_data(@import_data.slice(:data, :credentials))
 
         if @project.save && !@project.import?
           raise 'Failed to create repository' unless @project.create_repository
@@ -168,10 +168,9 @@ module Projects
     private
 
     def rename_to_tmp_name_path
-      @import_data[:original_name] = @project.name
-      @import_data[:original_path] = @project.path
+      @import_data[:data] = { original_name: @project.name, original_path: @project_path }
 
-      @project.name = @project.path = SecureRandom.hex
+      @project.name = @project.path = [@project.path, SecureRandom.hex].join('-')
       @project.expires_full_path_cache
     end
 
diff --git a/app/services/projects/move_deploy_keys_project_service.rb b/app/services/projects/move_deploy_keys_project_service.rb
deleted file mode 100644
index 0d9de1d0603..00000000000
--- a/app/services/projects/move_deploy_keys_project_service.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-module Projects
-  class MoveDeployKeysProjectService < MoveService
-    def execute(source_project)
-      return unless super
-
-      source_project.deploy_keys_projects.update_all(project_id: @project.id)
-
-      success
-    end
-  end
-end
diff --git a/app/services/projects/move_deploy_keys_projects_service.rb b/app/services/projects/move_deploy_keys_projects_service.rb
new file mode 100644
index 00000000000..891478a7d00
--- /dev/null
+++ b/app/services/projects/move_deploy_keys_projects_service.rb
@@ -0,0 +1,33 @@
+module Projects
+  class MoveDeployKeysProjectsService < BaseMoveRelationsService
+    def execute(source_project)
+      return unless super
+
+      Project.transaction(requires_new: true) do
+        move_deploy_keys_projects(source_project)
+        remove_remaining_deploy_keys_projects(source_project)
+
+        success
+      end
+    end
+
+    private
+
+    def move_deploy_keys_projects(source_project)
+      relation = non_existent_deploy_keys(source_project)
+      relation = relation.pluck(:id) unless Gitlab::Database.postgresql?
+
+      DeployKeysProject.where(deploy_key: relation)
+                       .update_all(project_id: @project.id)
+    end
+
+    def non_existent_deploy_keys(source_project)
+      source_project.deploy_keys
+                    .where.not(fingerprint: @project.deploy_keys.select(:fingerprint))
+    end
+
+    def remove_remaining_deploy_keys_projects(source_project)
+      source_project.deploy_keys_projects.destroy_all
+    end
+  end
+end
diff --git a/app/services/projects/move_fork_service.rb b/app/services/projects/move_forks_service.rb
index 9d06d453943..e613d789795 100644
--- a/app/services/projects/move_fork_service.rb
+++ b/app/services/projects/move_forks_service.rb
@@ -1,5 +1,5 @@
 module Projects
-  class MoveForkService < MoveService
+  class MoveForksService < BaseMoveRelationsService
     def execute(source_project)
       return unless super && source_project.fork_network
 
@@ -16,11 +16,11 @@ module Projects
     private
 
     def move_forked_project_links(source_project)
-      # Update descendants forks
+      # Update ancestor
       ForkedProjectLink.where(forked_to_project: source_project)
                        .update_all(forked_to_project_id: @project.id)
 
-      # Update the ancestor
+      # Update the descendants
       ForkedProjectLink.where(forked_from_project: source_project)
                        .update_all(forked_from_project_id: @project.id)
     end
diff --git a/app/services/projects/move_lfs_objects_project_service.rb b/app/services/projects/move_lfs_objects_projects_service.rb
index 959b20db60a..cc90d2fb415 100644
--- a/app/services/projects/move_lfs_objects_project_service.rb
+++ b/app/services/projects/move_lfs_objects_projects_service.rb
@@ -1,5 +1,5 @@
 module Projects
-  class MoveLfsObjectsProjectService < MoveService
+  class MoveLfsObjectsProjectsService < BaseMoveRelationsService
     def execute(source_project)
       return unless super
 
@@ -15,9 +15,9 @@ module Projects
 
     def move_lfs_objects_projects(source_project)
       if Gitlab::Database.postgresql?
-        non_existent_lfs_objects(source_project)
+        non_existent_lfs_objects_projects(source_project)
       else
-        LfsObjectsProject.where(id: non_existent_lfs_objects(source_project).pluck(:id))
+        LfsObjectsProject.where(id: non_existent_lfs_objects_projects(source_project).pluck(:id))
       end.update_all(project_id: @project.lfs_storage_project.id)
     end
 
@@ -25,7 +25,7 @@ module Projects
       source_project.lfs_objects_projects.destroy_all
     end
 
-    def non_existent_lfs_objects(source_project)
+    def non_existent_lfs_objects_projects(source_project)
       source_project.lfs_objects_projects
                     .where.not(lfs_object: @project.lfs_objects)
     end
diff --git a/app/services/projects/move_notification_setting_service.rb b/app/services/projects/move_notification_settings_service.rb
index 6123eb3923d..6c5136a6aba 100644
--- a/app/services/projects/move_notification_setting_service.rb
+++ b/app/services/projects/move_notification_settings_service.rb
@@ -1,5 +1,5 @@
 module Projects
-  class MoveNotificationSettingService < MoveService
+  class MoveNotificationSettingsService < BaseMoveRelationsService
     def execute(source_project)
       return unless super
 
diff --git a/app/services/projects/move_project_member_service.rb b/app/services/projects/move_project_authorizations_service.rb
index 946c91dab20..9d5c7aed2e6 100644
--- a/app/services/projects/move_project_member_service.rb
+++ b/app/services/projects/move_project_authorizations_service.rb
@@ -1,22 +1,16 @@
 module Projects
-  class MoveProjectMemberService < MoveService
+  class MoveProjectAuthorizationsService < BaseMoveRelationsService
     def execute(source_project)
       return unless super
 
-      result_move_members = result_move_auth = result_remove_members = nil
-
-      Project.transaction(requires_new: true) do
-        result_move_members = move_project_members(source_project)
-        result_move_auth = move_project_authorizations(source_project)
-        result_remove_members = remove_remaining_members(source_project)
-      end
+      result_move_auth = move_project_authorizations(source_project)
 
       # There is a problem when we move remove_remaining_authorizations inside
       # the transaction. Once the transaction checks the result of all the operations
       # it doesn't take the project authorizations records to delete them.
       # I think there is a Rails bug because of the project_authorizations not
       # having a primary key and/or id column
-      if result_move_members && result_move_auth && result_remove_members
+      if result_move_auth
         remove_remaining_authorizations(source_project)
 
         success
@@ -25,14 +19,6 @@ module Projects
 
     private
 
-    def move_project_members(source_project)
-      if Gitlab::Database.postgresql?
-        non_existent_members(source_project)
-      else
-        ProjectMember.where(id: non_existent_members(source_project).pluck(:id))
-      end.update_all(source_id: @project.id)
-    end
-
     def move_project_authorizations(source_project)
       if Gitlab::Database.postgresql?
         non_existent_authorization(source_project)
@@ -42,11 +28,6 @@ module Projects
       end.update_all(project_id: @project.id)
     end
 
-    def remove_remaining_members(source_project)
-      # Remove remaining members and authorizations from source_project
-      source_project.project_members.destroy_all
-    end
-
     def remove_remaining_authorizations(source_project)
       # I think because the Project Authorization table does not have a primary key
       # it brings a lot a problems/bugs. First, Rails raises PG::SyntaxException if we use
@@ -54,13 +35,6 @@ module Projects
       source_project.project_authorizations.delete_all
     end
 
-    # Look for members in source_project that are not in the target project
-    def non_existent_members(source_project)
-      source_project.members
-                    .select(:id)
-                    .where.not(user: @project.users)
-    end
-
     # Look for authorizations in source_project that are not in the target project
     def non_existent_authorization(source_project)
       source_project.project_authorizations
diff --git a/app/services/projects/move_project_group_link_service.rb b/app/services/projects/move_project_group_links_service.rb
index 2f42fa4b039..2c387fc6b11 100644
--- a/app/services/projects/move_project_group_link_service.rb
+++ b/app/services/projects/move_project_group_links_service.rb
@@ -1,5 +1,5 @@
 module Projects
-  class MoveProjectGroupLinkService < MoveService
+  class MoveProjectGroupLinksService < BaseMoveRelationsService
     def execute(source_project)
       return unless super
 
diff --git a/app/services/projects/move_project_members_service.rb b/app/services/projects/move_project_members_service.rb
new file mode 100644
index 00000000000..c0389e08faa
--- /dev/null
+++ b/app/services/projects/move_project_members_service.rb
@@ -0,0 +1,36 @@
+module Projects
+  class MoveProjectMembersService < BaseMoveRelationsService
+    def execute(source_project)
+      return unless super
+
+      Project.transaction(requires_new: true) do
+        move_project_members(source_project)
+        remove_remaining_members(source_project)
+
+        success
+      end
+    end
+
+    private
+
+    def move_project_members(source_project)
+      if Gitlab::Database.postgresql?
+        non_existent_members(source_project)
+      else
+        ProjectMember.where(id: non_existent_members(source_project).pluck(:id))
+      end.update_all(source_id: @project.id)
+    end
+
+    def remove_remaining_members(source_project)
+      # Remove remaining members and authorizations from source_project
+      source_project.project_members.destroy_all
+    end
+
+    # Look for members in source_project that are not in the target project
+    def non_existent_members(source_project)
+      source_project.members
+                    .select(:id)
+                    .where.not(user: @project.users)
+    end
+  end
+end
diff --git a/app/services/projects/move_users_star_project_service.rb b/app/services/projects/move_users_star_projects_service.rb
index 8aa0c5c38ba..d782eea88a6 100644
--- a/app/services/projects/move_users_star_project_service.rb
+++ b/app/services/projects/move_users_star_projects_service.rb
@@ -1,5 +1,5 @@
 module Projects
-  class MoveUsersStarProjectService < MoveService
+  class MoveUsersStarProjectsService < BaseMoveRelationsService
     def execute(source_project)
       return unless super
 
diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb
index 0242e0ea4fe..d420cf464fd 100644
--- a/app/services/projects/overwrite_project_service.rb
+++ b/app/services/projects/overwrite_project_service.rb
@@ -25,23 +25,15 @@ module Projects
 
     private
 
-    def rename_project
-      # Update de project's name and path to the original name/path
-      ::Projects::UpdateService.new(@project,
-                                    @current_user,
-                                    { name: @project.import_data.original_name,
-                                      path: @project.import_data.original_path })
-                               .execute
-    end
-
     def move_before_destroy_relationships(source_project)
-      ::Projects::MoveUsersStarProjectService.new(@project, @current_user).execute(source_project)
-      ::Projects::MoveProjectMemberService.new(@project, @current_user).execute(source_project)
-      ::Projects::MoveProjectGroupLinkService.new(@project, @current_user).execute(source_project)
-      ::Projects::MoveDeployKeysProjectService.new(@project, @current_user).execute(source_project)
-      ::Projects::MoveNotificationSettingService.new(@project, @current_user).execute(source_project)
-      ::Projects::MoveForkService.new(@project, @current_user).execute(source_project)
-      ::Projects::MoveLfsObjectsProjectService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveUsersStarProjectsService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveProjectMembersService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveProjectAuthorizationsService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveProjectGroupLinksService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveDeployKeysProjectsService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveNotificationSettingsService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveForksService.new(@project, @current_user).execute(source_project)
+      ::Projects::MoveLfsObjectsProjectsService.new(@project, @current_user).execute(source_project)
       add_source_project_to_fork_network(source_project)
     end
 
@@ -50,6 +42,15 @@ module Projects
       ::Projects::DestroyService.new(source_project, @current_user).execute
     end
 
+    def rename_project
+      # Update de project's name and path to the original name/path
+      ::Projects::UpdateService.new(@project,
+                                    @current_user,
+                                    { name: @project.import_data.data['original_path'],
+                                      path: @project.import_data.data['original_path'] })
+                               .execute
+    end
+
     def attempt_restore_repositories(project)
       ::Projects::DestroyService.new(project, @current_user).attempt_repositories_rollback
     end
diff --git a/db/migrate/20180320162641_add_original_name_and_path_to_project_import_data.rb b/db/migrate/20180320162641_add_original_name_and_path_to_project_import_data.rb
deleted file mode 100644
index 981ad6e8a1b..00000000000
--- a/db/migrate/20180320162641_add_original_name_and_path_to_project_import_data.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
-class AddOriginalNameAndPathToProjectImportData < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  def change
-    add_column :project_import_data, :original_name, :string
-    add_column :project_import_data, :original_path, :string
-  end
-end
diff --git a/db/schema.rb b/db/schema.rb
index c227176cdfc..1be0570f85a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1446,8 +1446,6 @@ ActiveRecord::Schema.define(version: 20180320182229) do
     t.text "encrypted_credentials"
     t.string "encrypted_credentials_iv"
     t.string "encrypted_credentials_salt"
-    t.string "original_name"
-    t.string "original_path"
   end
 
   add_index "project_import_data", ["project_id"], name: "index_project_import_data_on_project_id", using: :btree
diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb
index bf83952ee74..ead85de7c41 100644
--- a/lib/gitlab/import_export/importer.rb
+++ b/lib/gitlab/import_export/importer.rb
@@ -15,10 +15,8 @@ module Gitlab
       end
 
       def execute
-        if import_file && check_version! && restore_services
-          project_tree.restored_project.tap do |restored_project|
-            overwrite_project(restored_project) if overwrite_project?
-          end
+        if import_file && check_version! && restore_export && overwrite_project
+          project_tree.restored_project
         else
           raise Projects::ImportService::Error.new(@shared.errors.join(', '))
         end
@@ -30,21 +28,7 @@ module Gitlab
 
       private
 
-      def overwrite_project(restored_project)
-        ::Projects::OverwriteProjectService.new(restored_project, @current_user).execute(project_to_overwrite)
-      end
-
-      def overwrite_project?
-        @project.import_data.original_name.present? && @project.import_data.original_path.present?
-      end
-
-      def project_to_overwrite
-        strong_memoize(:project_to_overwrite) do
-          Project.find_by_full_path("#{@project.namespace.full_path}/#{@project.import_data.original_path}")
-        end
-      end
-
-      def restore_services
+      def restore_export
         [repo_restorer, wiki_restorer, project_tree, avatar_restorer, uploads_restorer, statistics_restorer].all?(&:restore)
       end
 
@@ -103,6 +87,25 @@ module Gitlab
       def remove_import_file
         FileUtils.rm_rf(@archive_file)
       end
+
+      def overwrite_project
+        if overwrite_project?
+          ::Projects::OverwriteProjectService.new(project_tree.restored_project, @current_user)
+                                             .execute(project_to_overwrite)
+        end
+
+        true
+      end
+
+      def overwrite_project?
+        @project.import_data.original_name.present? && @project.import_data.original_path.present?
+      end
+
+      def project_to_overwrite
+        strong_memoize(:project_to_overwrite) do
+          Project.find_by_full_path("#{@project.namespace.full_path}/#{@project.import_data.original_path}")
+        end
+      end
     end
   end
 end
diff --git a/spec/services/projects/move_service_spec.rb b/spec/services/projects/base_move_relations_service_spec.rb
index 4ae9dd464e2..6e48a31271c 100644
--- a/spec/services/projects/move_service_spec.rb
+++ b/spec/services/projects/base_move_relations_service_spec.rb
@@ -1,5 +1,5 @@
 require 'spec_helper'
 
-describe Projects::MoveService do
+describe Projects::BaseMoveRelationsService do
   it_behaves_like 'move services access rights'
 end
diff --git a/spec/services/projects/move_deploy_keys_project_service_spec.rb b/spec/services/projects/move_deploy_keys_project_service_spec.rb
deleted file mode 100644
index 0c656ad66d4..00000000000
--- a/spec/services/projects/move_deploy_keys_project_service_spec.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-require 'spec_helper'
-
-describe Projects::MoveDeployKeysProjectService do
-  let!(:user) { create(:user) }
-  let!(:project_with_deploy_keys) { create(:project, namespace: user.namespace) }
-  let!(:target_project) { create(:project, namespace: user.namespace) }
-
-  subject { described_class.new(target_project, user) }
-
-  describe '#execute' do
-    before do
-      create_list(:deploy_keys_project, 2, project: project_with_deploy_keys)
-    end
-
-    it 'moves the user\'s deploy keys from one project to another' do
-      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 2
-      expect(target_project.deploy_keys_projects.count).to eq 0
-
-      subject.execute(project_with_deploy_keys)
-
-      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 0
-      expect(target_project.deploy_keys_projects.count).to eq 2
-    end
-  end
-end
diff --git a/spec/services/projects/move_deploy_keys_projects_service_spec.rb b/spec/services/projects/move_deploy_keys_projects_service_spec.rb
new file mode 100644
index 00000000000..1917f03d032
--- /dev/null
+++ b/spec/services/projects/move_deploy_keys_projects_service_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Projects::MoveDeployKeysProjectsService do
+  let!(:user) { create(:user) }
+  let!(:project_with_deploy_keys) { create(:project, namespace: user.namespace) }
+  let!(:target_project) { create(:project, namespace: user.namespace) }
+
+  subject { described_class.new(target_project, user) }
+
+  describe '#execute' do
+    before do
+      create_list(:deploy_keys_project, 2, project: project_with_deploy_keys)
+    end
+
+    it 'moves the user\'s deploy keys from one project to another' do
+      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 2
+      expect(target_project.deploy_keys_projects.count).to eq 0
+
+      subject.execute(project_with_deploy_keys)
+
+      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 0
+      expect(target_project.deploy_keys_projects.count).to eq 2
+    end
+
+    it 'does not link existent deploy_keys in the current project' do
+      target_project.deploy_keys << project_with_deploy_keys.deploy_keys.first
+
+      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 2
+      expect(target_project.deploy_keys_projects.count).to eq 1
+
+      subject.execute(project_with_deploy_keys)
+
+      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 0
+      expect(target_project.deploy_keys_projects.count).to eq 2
+    end
+
+    it 'rollbacks changes if transaction fails' do
+      allow(subject).to receive(:success).and_raise(StandardError)
+
+      expect { subject.execute(project_with_deploy_keys) }.to raise_error(StandardError)
+
+      expect(project_with_deploy_keys.deploy_keys_projects.count).to eq 2
+      expect(target_project.deploy_keys_projects.count).to eq 0
+    end
+  end
+end
diff --git a/spec/services/projects/move_fork_service_spec.rb b/spec/services/projects/move_forks_service_spec.rb
index 8fd249e10e7..f4a5a7f9fc2 100644
--- a/spec/services/projects/move_fork_service_spec.rb
+++ b/spec/services/projects/move_forks_service_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::MoveForkService do
+describe Projects::MoveForksService do
   include ProjectForksHelper
 
   let!(:user) { create(:user) }
@@ -85,7 +85,7 @@ describe Projects::MoveForkService do
     end
 
     it 'rollbacks changes if transaction fails' do
-      allow(subject).to receive(:refresh_forks_count).and_raise(StandardError)
+      allow(subject).to receive(:success).and_raise(StandardError)
 
       expect { subject.execute(project_with_forks) }.to raise_error(StandardError)
 
diff --git a/spec/services/projects/move_lfs_objects_project_service_spec.rb b/spec/services/projects/move_lfs_objects_projects_service_spec.rb
index e79fc228a48..409ba1e6cfa 100644
--- a/spec/services/projects/move_lfs_objects_project_service_spec.rb
+++ b/spec/services/projects/move_lfs_objects_projects_service_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::MoveLfsObjectsProjectService do
+describe Projects::MoveLfsObjectsProjectsService do
   let!(:user) { create(:user) }
   let!(:project_with_lfs_objects) { create(:project, namespace: user.namespace) }
   let!(:target_project) { create(:project, namespace: user.namespace) }
@@ -32,7 +32,7 @@ describe Projects::MoveLfsObjectsProjectService do
     end
 
     it 'rollbacks changes if transaction fails' do
-      allow_any_instance_of(ActiveRecord::Associations::CollectionProxy).to receive(:destroy_all).and_raise(StandardError)
+      allow(subject).to receive(:success).and_raise(StandardError)
 
       expect { subject.execute(project_with_lfs_objects) }.to raise_error(StandardError)
 
diff --git a/spec/services/projects/move_notification_setting_service_spec.rb b/spec/services/projects/move_notification_settings_service_spec.rb
index 95edd380a8c..347edc39e2b 100644
--- a/spec/services/projects/move_notification_setting_service_spec.rb
+++ b/spec/services/projects/move_notification_settings_service_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::MoveNotificationSettingService do
+describe Projects::MoveNotificationSettingsService do
   let(:user) { create(:user) }
   let(:project_with_notifications) { create(:project, namespace: user.namespace) }
   let(:target_project) { create(:project, namespace: user.namespace) }
@@ -24,7 +24,7 @@ describe Projects::MoveNotificationSettingService do
       end
 
       it 'rollbacks changes if transaction fails' do
-        allow_any_instance_of(ActiveRecord::Associations::CollectionProxy).to receive(:destroy_all).and_raise(StandardError)
+        allow(subject).to receive(:success).and_raise(StandardError)
 
         expect { subject.execute(project_with_notifications) }.to raise_error(StandardError)
 
diff --git a/spec/services/projects/move_project_authorizations_service_spec.rb b/spec/services/projects/move_project_authorizations_service_spec.rb
new file mode 100644
index 00000000000..9c76c0ba0e9
--- /dev/null
+++ b/spec/services/projects/move_project_authorizations_service_spec.rb
@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Projects::MoveProjectAuthorizationsService do
+  let!(:user) { create(:user) }
+  let(:project_with_users) { create(:project, namespace: user.namespace) }
+  let(:target_project) { create(:project, namespace: user.namespace) }
+  let(:master_user) { create(:user) }
+  let(:reporter_user) { create(:user) }
+  let(:developer_user) { create(:user) }
+
+  subject { described_class.new(target_project, user) }
+
+  describe '#execute' do
+    before do
+      project_with_users.add_master(master_user)
+      project_with_users.add_developer(developer_user)
+      project_with_users.add_reporter(reporter_user)
+    end
+
+    it 'moves the authorizations from one project to another' do
+      expect(project_with_users.authorized_users.count).to eq 4
+      expect(target_project.authorized_users.count).to eq 1
+
+      subject.execute(project_with_users)
+
+      expect(project_with_users.authorized_users.count).to eq 0
+      expect(target_project.authorized_users.count).to eq 4
+    end
+
+    it 'does not move existent authorizations to the current project' do
+      target_project.add_master(developer_user)
+      target_project.add_developer(reporter_user)
+
+      expect(project_with_users.authorized_users.count).to eq 4
+      expect(target_project.authorized_users.count).to eq 3
+
+      subject.execute(project_with_users)
+
+      expect(project_with_users.authorized_users.count).to eq 0
+      expect(target_project.authorized_users.count).to eq 4
+    end
+  end
+end
diff --git a/spec/services/projects/move_project_group_link_service_spec.rb b/spec/services/projects/move_project_group_links_service_spec.rb
index 4b79b81e9cd..a27eee926c7 100644
--- a/spec/services/projects/move_project_group_link_service_spec.rb
+++ b/spec/services/projects/move_project_group_links_service_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::MoveProjectGroupLinkService do
+describe Projects::MoveProjectGroupLinksService do
   let!(:user) { create(:user) }
   let(:project_with_groups) { create(:project, namespace: user.namespace) }
   let(:target_project) { create(:project, namespace: user.namespace) }
@@ -41,7 +41,7 @@ describe Projects::MoveProjectGroupLinkService do
     end
 
     it 'rollbacks changes if transaction fails' do
-      allow_any_instance_of(ActiveRecord::Associations::CollectionProxy).to receive(:destroy_all).and_raise(StandardError)
+      allow(subject).to receive(:success).and_raise(StandardError)
 
       expect { subject.execute(project_with_groups) }.to raise_error(StandardError)
 
diff --git a/spec/services/projects/move_project_member_service_spec.rb b/spec/services/projects/move_project_members_service_spec.rb
index 6f96ed9d906..d1d4e06e91e 100644
--- a/spec/services/projects/move_project_member_service_spec.rb
+++ b/spec/services/projects/move_project_members_service_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::MoveProjectMemberService do
+describe Projects::MoveProjectMembersService do
   let!(:user) { create(:user) }
   let(:project_with_users) { create(:project, namespace: user.namespace) }
   let(:target_project) { create(:project, namespace: user.namespace) }
@@ -27,35 +27,21 @@ describe Projects::MoveProjectMemberService do
       expect(target_project.project_members.count).to eq 4
     end
 
-    it 'moves the authorizations from one project to another' do
-      expect(project_with_users.authorized_users.count).to eq 4
-      expect(target_project.authorized_users.count).to eq 1
-
-      subject.execute(project_with_users)
-
-      expect(project_with_users.authorized_users.count).to eq 0
-      expect(target_project.authorized_users.count).to eq 4
-    end
-
-    it 'does not move existent members and/or authorizations to the current project' do
+    it 'does not move existent members to the current project' do
       target_project.add_master(developer_user)
       target_project.add_developer(reporter_user)
 
       expect(project_with_users.project_members.count).to eq 4
       expect(target_project.project_members.count).to eq 3
-      expect(project_with_users.authorized_users.count).to eq 4
-      expect(target_project.authorized_users.count).to eq 3
 
       subject.execute(project_with_users)
 
       expect(project_with_users.project_members.count).to eq 0
       expect(target_project.project_members.count).to eq 4
-      expect(project_with_users.authorized_users.count).to eq 0
-      expect(target_project.authorized_users.count).to eq 4
     end
 
     it 'rollbacks changes if transaction fails' do
-      allow(subject).to receive(:remove_remaining_members).and_raise(StandardError)
+      allow(subject).to receive(:success).and_raise(StandardError)
 
       expect { subject.execute(project_with_users) }.to raise_error(StandardError)
 
diff --git a/spec/services/projects/move_users_star_project_service_spec.rb b/spec/services/projects/move_users_star_projects_service_spec.rb
index d2fe1986255..e0545c5a21b 100644
--- a/spec/services/projects/move_users_star_project_service_spec.rb
+++ b/spec/services/projects/move_users_star_projects_service_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Projects::MoveUsersStarProjectService do
+describe Projects::MoveUsersStarProjectsService do
   let!(:user) { create(:user) }
   let!(:project_with_stars) { create(:project, namespace: user.namespace) }
   let!(:target_project) { create(:project, namespace: user.namespace) }
@@ -29,9 +29,7 @@ describe Projects::MoveUsersStarProjectService do
     end
 
     it 'rollbacks changes if transaction fails' do
-      # Necessary in order to mock the first call to :reset_counters
-      allow(Project).to receive(:reset_counters).and_call_original
-      allow(Project).to receive(:reset_counters).with(project_with_stars.id, :users_star_projects).and_raise(StandardError)
+      allow(subject).to receive(:success).and_raise(StandardError)
 
       expect { subject.execute(project_with_stars) }.to raise_error(StandardError)