From fb05be0588b98d21043bc4e032e18c8911c23236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Thu, 5 Apr 2018 11:40:34 +0200 Subject: Backup from code review comments --- app/models/project.rb | 7 +- .../projects/base_move_relations_service.rb | 16 ++++ app/services/projects/create_service.rb | 7 +- .../projects/move_deploy_keys_project_service.rb | 11 --- .../projects/move_deploy_keys_projects_service.rb | 33 ++++++++ app/services/projects/move_fork_service.rb | 42 ---------- app/services/projects/move_forks_service.rb | 42 ++++++++++ .../projects/move_lfs_objects_project_service.rb | 33 -------- .../projects/move_lfs_objects_projects_service.rb | 33 ++++++++ .../projects/move_notification_setting_service.rb | 38 --------- .../projects/move_notification_settings_service.rb | 38 +++++++++ .../move_project_authorizations_service.rb | 45 ++++++++++ .../projects/move_project_group_link_service.rb | 36 -------- .../projects/move_project_group_links_service.rb | 36 ++++++++ .../projects/move_project_member_service.rb | 71 ---------------- .../projects/move_project_members_service.rb | 36 ++++++++ app/services/projects/move_service.rb | 16 ---- .../projects/move_users_star_project_service.rb | 20 ----- .../projects/move_users_star_projects_service.rb | 20 +++++ app/services/projects/overwrite_project_service.rb | 33 ++++---- ...riginal_name_and_path_to_project_import_data.rb | 13 --- db/schema.rb | 2 - lib/gitlab/import_export/importer.rb | 41 ++++----- .../projects/base_move_relations_service_spec.rb | 5 ++ .../move_deploy_keys_project_service_spec.rb | 25 ------ .../move_deploy_keys_projects_service_spec.rb | 46 +++++++++++ spec/services/projects/move_fork_service_spec.rb | 96 ---------------------- spec/services/projects/move_forks_service_spec.rb | 96 ++++++++++++++++++++++ .../move_lfs_objects_project_service_spec.rb | 43 ---------- .../move_lfs_objects_projects_service_spec.rb | 43 ++++++++++ .../move_notification_setting_service_spec.rb | 46 ----------- .../move_notification_settings_service_spec.rb | 46 +++++++++++ .../move_project_authorizations_service_spec.rb | 43 ++++++++++ .../move_project_group_link_service_spec.rb | 52 ------------ .../move_project_group_links_service_spec.rb | 52 ++++++++++++ .../projects/move_project_member_service_spec.rb | 66 --------------- .../projects/move_project_members_service_spec.rb | 52 ++++++++++++ spec/services/projects/move_service_spec.rb | 5 -- .../move_users_star_project_service_spec.rb | 44 ---------- .../move_users_star_projects_service_spec.rb | 42 ++++++++++ 40 files changed, 768 insertions(+), 703 deletions(-) create mode 100644 app/services/projects/base_move_relations_service.rb delete mode 100644 app/services/projects/move_deploy_keys_project_service.rb create mode 100644 app/services/projects/move_deploy_keys_projects_service.rb delete mode 100644 app/services/projects/move_fork_service.rb create mode 100644 app/services/projects/move_forks_service.rb delete mode 100644 app/services/projects/move_lfs_objects_project_service.rb create mode 100644 app/services/projects/move_lfs_objects_projects_service.rb delete mode 100644 app/services/projects/move_notification_setting_service.rb create mode 100644 app/services/projects/move_notification_settings_service.rb create mode 100644 app/services/projects/move_project_authorizations_service.rb delete mode 100644 app/services/projects/move_project_group_link_service.rb create mode 100644 app/services/projects/move_project_group_links_service.rb delete mode 100644 app/services/projects/move_project_member_service.rb create mode 100644 app/services/projects/move_project_members_service.rb delete mode 100644 app/services/projects/move_service.rb delete mode 100644 app/services/projects/move_users_star_project_service.rb create mode 100644 app/services/projects/move_users_star_projects_service.rb delete mode 100644 db/migrate/20180320162641_add_original_name_and_path_to_project_import_data.rb create mode 100644 spec/services/projects/base_move_relations_service_spec.rb delete mode 100644 spec/services/projects/move_deploy_keys_project_service_spec.rb create mode 100644 spec/services/projects/move_deploy_keys_projects_service_spec.rb delete mode 100644 spec/services/projects/move_fork_service_spec.rb create mode 100644 spec/services/projects/move_forks_service_spec.rb delete mode 100644 spec/services/projects/move_lfs_objects_project_service_spec.rb create mode 100644 spec/services/projects/move_lfs_objects_projects_service_spec.rb delete mode 100644 spec/services/projects/move_notification_setting_service_spec.rb create mode 100644 spec/services/projects/move_notification_settings_service_spec.rb create mode 100644 spec/services/projects/move_project_authorizations_service_spec.rb delete mode 100644 spec/services/projects/move_project_group_link_service_spec.rb create mode 100644 spec/services/projects/move_project_group_links_service_spec.rb delete mode 100644 spec/services/projects/move_project_member_service_spec.rb create mode 100644 spec/services/projects/move_project_members_service_spec.rb delete mode 100644 spec/services/projects/move_service_spec.rb delete mode 100644 spec/services/projects/move_users_star_project_service_spec.rb create mode 100644 spec/services/projects/move_users_star_projects_service_spec.rb 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/base_move_relations_service.rb b/app/services/projects/base_move_relations_service.rb new file mode 100644 index 00000000000..c08ec489365 --- /dev/null +++ b/app/services/projects/base_move_relations_service.rb @@ -0,0 +1,16 @@ +module Projects + class BaseMoveRelationsService < BaseService + def execute(source_project) + return if source_project.blank? || !can_move_both_projects?(source_project) + + true + end + + private + + def can_move_both_projects?(source_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_fork_service.rb deleted file mode 100644 index 9d06d453943..00000000000 --- a/app/services/projects/move_fork_service.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Projects - class MoveForkService < MoveService - def execute(source_project) - return unless super && source_project.fork_network - - Project.transaction(requires_new: true) do - move_forked_project_links(source_project) - move_fork_network_members(source_project) - update_root_project(source_project) - refresh_forks_count - - success - end - end - - private - - def move_forked_project_links(source_project) - # Update descendants forks - ForkedProjectLink.where(forked_to_project: source_project) - .update_all(forked_to_project_id: @project.id) - - # Update the ancestor - ForkedProjectLink.where(forked_from_project: source_project) - .update_all(forked_from_project_id: @project.id) - end - - def move_fork_network_members(source_project) - ForkNetworkMember.where(project: source_project).update_all(project_id: @project.id) - ForkNetworkMember.where(forked_from_project: source_project).update_all(forked_from_project_id: @project.id) - end - - def update_root_project(source_project) - # Update root network project - ForkNetwork.where(root_project: source_project).update_all(root_project_id: @project.id) - end - - def refresh_forks_count - Projects::ForksCountService.new(@project).refresh_cache - end - end -end diff --git a/app/services/projects/move_forks_service.rb b/app/services/projects/move_forks_service.rb new file mode 100644 index 00000000000..e613d789795 --- /dev/null +++ b/app/services/projects/move_forks_service.rb @@ -0,0 +1,42 @@ +module Projects + class MoveForksService < BaseMoveRelationsService + def execute(source_project) + return unless super && source_project.fork_network + + Project.transaction(requires_new: true) do + move_forked_project_links(source_project) + move_fork_network_members(source_project) + update_root_project(source_project) + refresh_forks_count + + success + end + end + + private + + def move_forked_project_links(source_project) + # Update ancestor + ForkedProjectLink.where(forked_to_project: source_project) + .update_all(forked_to_project_id: @project.id) + + # Update the descendants + ForkedProjectLink.where(forked_from_project: source_project) + .update_all(forked_from_project_id: @project.id) + end + + def move_fork_network_members(source_project) + ForkNetworkMember.where(project: source_project).update_all(project_id: @project.id) + ForkNetworkMember.where(forked_from_project: source_project).update_all(forked_from_project_id: @project.id) + end + + def update_root_project(source_project) + # Update root network project + ForkNetwork.where(root_project: source_project).update_all(root_project_id: @project.id) + end + + def refresh_forks_count + Projects::ForksCountService.new(@project).refresh_cache + end + end +end diff --git a/app/services/projects/move_lfs_objects_project_service.rb b/app/services/projects/move_lfs_objects_project_service.rb deleted file mode 100644 index 959b20db60a..00000000000 --- a/app/services/projects/move_lfs_objects_project_service.rb +++ /dev/null @@ -1,33 +0,0 @@ -module Projects - class MoveLfsObjectsProjectService < MoveService - def execute(source_project) - return unless super - - Project.transaction(requires_new: true) do - move_lfs_objects_projects(source_project) - remove_remaining_lfs_objects_project(source_project.reload) - - success - end - end - - private - - def move_lfs_objects_projects(source_project) - if Gitlab::Database.postgresql? - non_existent_lfs_objects(source_project) - else - LfsObjectsProject.where(id: non_existent_lfs_objects(source_project).pluck(:id)) - end.update_all(project_id: @project.lfs_storage_project.id) - end - - def remove_remaining_lfs_objects_project(source_project) - source_project.lfs_objects_projects.destroy_all - end - - def non_existent_lfs_objects(source_project) - source_project.lfs_objects_projects - .where.not(lfs_object: @project.lfs_objects) - end - end -end diff --git a/app/services/projects/move_lfs_objects_projects_service.rb b/app/services/projects/move_lfs_objects_projects_service.rb new file mode 100644 index 00000000000..cc90d2fb415 --- /dev/null +++ b/app/services/projects/move_lfs_objects_projects_service.rb @@ -0,0 +1,33 @@ +module Projects + class MoveLfsObjectsProjectsService < BaseMoveRelationsService + def execute(source_project) + return unless super + + Project.transaction(requires_new: true) do + move_lfs_objects_projects(source_project) + remove_remaining_lfs_objects_project(source_project.reload) + + success + end + end + + private + + def move_lfs_objects_projects(source_project) + if Gitlab::Database.postgresql? + non_existent_lfs_objects_projects(source_project) + else + LfsObjectsProject.where(id: non_existent_lfs_objects_projects(source_project).pluck(:id)) + end.update_all(project_id: @project.lfs_storage_project.id) + end + + def remove_remaining_lfs_objects_project(source_project) + source_project.lfs_objects_projects.destroy_all + end + + def non_existent_lfs_objects_projects(source_project) + source_project.lfs_objects_projects + .where.not(lfs_object: @project.lfs_objects) + end + end +end diff --git a/app/services/projects/move_notification_setting_service.rb b/app/services/projects/move_notification_setting_service.rb deleted file mode 100644 index 6123eb3923d..00000000000 --- a/app/services/projects/move_notification_setting_service.rb +++ /dev/null @@ -1,38 +0,0 @@ -module Projects - class MoveNotificationSettingService < MoveService - def execute(source_project) - return unless super - - Project.transaction(requires_new: true) do - move_notification_settings(source_project) - - # Remove remaining notification settings from source_project - source_project.reload.notification_settings.destroy_all - - success - end - end - - private - - def move_notification_settings(source_project) - if Gitlab::Database.postgresql? - non_existent_notifications(source_project) - else - NotificationSetting.where(id: non_existent_notifications(source_project).pluck(:id)) - end.update_all(source_id: @project.id) - end - - # Get users of current notification_settings - def users_in_target_project - @project.notification_settings.select(:user_id) - end - - # Look for notification_settings in source_project that are not in the target project - def non_existent_notifications(source_project) - source_project.notification_settings - .select(:id) - .where.not(user_id: users_in_target_project) - end - end -end diff --git a/app/services/projects/move_notification_settings_service.rb b/app/services/projects/move_notification_settings_service.rb new file mode 100644 index 00000000000..6c5136a6aba --- /dev/null +++ b/app/services/projects/move_notification_settings_service.rb @@ -0,0 +1,38 @@ +module Projects + class MoveNotificationSettingsService < BaseMoveRelationsService + def execute(source_project) + return unless super + + Project.transaction(requires_new: true) do + move_notification_settings(source_project) + + # Remove remaining notification settings from source_project + source_project.reload.notification_settings.destroy_all + + success + end + end + + private + + def move_notification_settings(source_project) + if Gitlab::Database.postgresql? + non_existent_notifications(source_project) + else + NotificationSetting.where(id: non_existent_notifications(source_project).pluck(:id)) + end.update_all(source_id: @project.id) + end + + # Get users of current notification_settings + def users_in_target_project + @project.notification_settings.select(:user_id) + end + + # Look for notification_settings in source_project that are not in the target project + def non_existent_notifications(source_project) + source_project.notification_settings + .select(:id) + .where.not(user_id: users_in_target_project) + end + end +end diff --git a/app/services/projects/move_project_authorizations_service.rb b/app/services/projects/move_project_authorizations_service.rb new file mode 100644 index 00000000000..9d5c7aed2e6 --- /dev/null +++ b/app/services/projects/move_project_authorizations_service.rb @@ -0,0 +1,45 @@ +module Projects + class MoveProjectAuthorizationsService < BaseMoveRelationsService + def execute(source_project) + return unless super + + 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_auth + remove_remaining_authorizations(source_project) + + success + end + end + + private + + def move_project_authorizations(source_project) + if Gitlab::Database.postgresql? + non_existent_authorization(source_project) + else + ProjectAuthorization.where(user_id: non_existent_authorization(source_project).pluck(:user_id), + project: source_project) + end.update_all(project_id: @project.id) + 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 + # destroy_all instead of delete_all. + source_project.project_authorizations.delete_all + end + + # Look for authorizations in source_project that are not in the target project + def non_existent_authorization(source_project) + source_project.project_authorizations + .select(:user_id) + .where.not(user: @project.authorized_users) + end + end +end diff --git a/app/services/projects/move_project_group_link_service.rb b/app/services/projects/move_project_group_link_service.rb deleted file mode 100644 index 2f42fa4b039..00000000000 --- a/app/services/projects/move_project_group_link_service.rb +++ /dev/null @@ -1,36 +0,0 @@ -module Projects - class MoveProjectGroupLinkService < MoveService - def execute(source_project) - return unless super - - Project.transaction(requires_new: true) do - move_group_links(source_project) - - # Remove remaining project group links from source_project - source_project.reload.project_group_links.destroy_all - - success - end - end - - private - - def groups_in_target_project - Group.where(id: @project.project_group_links.select(:group_id)) - end - - def move_group_links(source_project) - if Gitlab::Database.postgresql? - non_existent_group_links(source_project) - else - ProjectGroupLink.where(id: non_existent_group_links(source_project).pluck(:id)) - end.update_all(project_id: @project.id) - end - - def non_existent_group_links(source_project) - # Look for groups in source_project that are not in the target project - source_project.project_group_links - .where.not(group: groups_in_target_project) - end - end -end diff --git a/app/services/projects/move_project_group_links_service.rb b/app/services/projects/move_project_group_links_service.rb new file mode 100644 index 00000000000..2c387fc6b11 --- /dev/null +++ b/app/services/projects/move_project_group_links_service.rb @@ -0,0 +1,36 @@ +module Projects + class MoveProjectGroupLinksService < BaseMoveRelationsService + def execute(source_project) + return unless super + + Project.transaction(requires_new: true) do + move_group_links(source_project) + + # Remove remaining project group links from source_project + source_project.reload.project_group_links.destroy_all + + success + end + end + + private + + def groups_in_target_project + Group.where(id: @project.project_group_links.select(:group_id)) + end + + def move_group_links(source_project) + if Gitlab::Database.postgresql? + non_existent_group_links(source_project) + else + ProjectGroupLink.where(id: non_existent_group_links(source_project).pluck(:id)) + end.update_all(project_id: @project.id) + end + + def non_existent_group_links(source_project) + # Look for groups in source_project that are not in the target project + source_project.project_group_links + .where.not(group: groups_in_target_project) + end + end +end diff --git a/app/services/projects/move_project_member_service.rb b/app/services/projects/move_project_member_service.rb deleted file mode 100644 index 946c91dab20..00000000000 --- a/app/services/projects/move_project_member_service.rb +++ /dev/null @@ -1,71 +0,0 @@ -module Projects - class MoveProjectMemberService < MoveService - 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 - - # 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 - remove_remaining_authorizations(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 move_project_authorizations(source_project) - if Gitlab::Database.postgresql? - non_existent_authorization(source_project) - else - ProjectAuthorization.where(user_id: non_existent_authorization(source_project).pluck(:user_id), - project: source_project) - 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 - # destroy_all instead of delete_all. - 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 - .select(:user_id) - .where.not(user: @project.authorized_users) - end - end -end 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_service.rb b/app/services/projects/move_service.rb deleted file mode 100644 index 20d4c013807..00000000000 --- a/app/services/projects/move_service.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Projects - class MoveService < BaseService - def execute(source_project) - return if source_project.blank? || !can_move_both_projects?(source_project) - - true - end - - private - - def can_move_both_projects?(source_project) - can?(current_user, :owner_access, source_project) && - can?(current_user, :owner_access, project) - end - end -end diff --git a/app/services/projects/move_users_star_project_service.rb b/app/services/projects/move_users_star_project_service.rb deleted file mode 100644 index 8aa0c5c38ba..00000000000 --- a/app/services/projects/move_users_star_project_service.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Projects - class MoveUsersStarProjectService < MoveService - def execute(source_project) - return unless super - - user_stars = source_project.users_star_projects - - return unless user_stars.any? - - Project.transaction(requires_new: true) do - user_stars.update_all(project_id: @project.id) - - Project.reset_counters @project.id, :users_star_projects - Project.reset_counters source_project.id, :users_star_projects - - success - end - end - end -end diff --git a/app/services/projects/move_users_star_projects_service.rb b/app/services/projects/move_users_star_projects_service.rb new file mode 100644 index 00000000000..d782eea88a6 --- /dev/null +++ b/app/services/projects/move_users_star_projects_service.rb @@ -0,0 +1,20 @@ +module Projects + class MoveUsersStarProjectsService < BaseMoveRelationsService + def execute(source_project) + return unless super + + user_stars = source_project.users_star_projects + + return unless user_stars.any? + + Project.transaction(requires_new: true) do + user_stars.update_all(project_id: @project.id) + + Project.reset_counters @project.id, :users_star_projects + Project.reset_counters source_project.id, :users_star_projects + + success + end + end + end +end 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/base_move_relations_service_spec.rb b/spec/services/projects/base_move_relations_service_spec.rb new file mode 100644 index 00000000000..6e48a31271c --- /dev/null +++ b/spec/services/projects/base_move_relations_service_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +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_fork_service_spec.rb deleted file mode 100644 index 8fd249e10e7..00000000000 --- a/spec/services/projects/move_fork_service_spec.rb +++ /dev/null @@ -1,96 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveForkService do - include ProjectForksHelper - - let!(:user) { create(:user) } - let!(:project_with_forks) { create(:project, namespace: user.namespace) } - let!(:target_project) { create(:project, namespace: user.namespace) } - let!(:lvl1_forked_project_1) { fork_project(project_with_forks, user) } - let!(:lvl1_forked_project_2) { fork_project(project_with_forks, user) } - let!(:lvl2_forked_project_1_1) { fork_project(lvl1_forked_project_1, user) } - let!(:lvl2_forked_project_1_2) { fork_project(lvl1_forked_project_1, user) } - - subject { described_class.new(target_project, user) } - - describe '#execute' do - context 'when moving a root forked project' do - it 'moves the descendant forks' do - expect(project_with_forks.forks.count).to eq 2 - expect(target_project.forks.count).to eq 0 - - subject.execute(project_with_forks) - - expect(project_with_forks.forks.count).to eq 0 - expect(target_project.forks.count).to eq 2 - expect(lvl1_forked_project_1.forked_from_project).to eq target_project - expect(lvl1_forked_project_1.fork_network_member.forked_from_project).to eq target_project - expect(lvl1_forked_project_2.forked_from_project).to eq target_project - expect(lvl1_forked_project_2.fork_network_member.forked_from_project).to eq target_project - end - - it 'updates the fork network' do - expect(project_with_forks.fork_network.root_project).to eq project_with_forks - expect(project_with_forks.fork_network.fork_network_members.map(&:project)).to include project_with_forks - - subject.execute(project_with_forks) - - expect(target_project.reload.fork_network.root_project).to eq target_project - expect(target_project.fork_network.fork_network_members.map(&:project)).not_to include project_with_forks - end - end - - context 'when moving a intermediate forked project' do - it 'moves the descendant forks' do - expect(lvl1_forked_project_1.forks.count).to eq 2 - expect(target_project.forks.count).to eq 0 - - subject.execute(lvl1_forked_project_1) - - expect(lvl1_forked_project_1.forks.count).to eq 0 - expect(target_project.forks.count).to eq 2 - expect(lvl2_forked_project_1_1.forked_from_project).to eq target_project - expect(lvl2_forked_project_1_1.fork_network_member.forked_from_project).to eq target_project - expect(lvl2_forked_project_1_2.forked_from_project).to eq target_project - expect(lvl2_forked_project_1_2.fork_network_member.forked_from_project).to eq target_project - end - - it 'moves the ascendant fork' do - subject.execute(lvl1_forked_project_1) - - expect(target_project.forked_from_project).to eq project_with_forks - expect(target_project.fork_network_member.forked_from_project).to eq project_with_forks - end - - it 'does not update fork network' do - subject.execute(lvl1_forked_project_1) - - expect(target_project.reload.fork_network.root_project).to eq project_with_forks - end - end - - context 'when moving a leaf forked project' do - it 'moves the ascendant fork' do - subject.execute(lvl2_forked_project_1_1) - - expect(target_project.forked_from_project).to eq lvl1_forked_project_1 - expect(target_project.fork_network_member.forked_from_project).to eq lvl1_forked_project_1 - end - - it 'does not update fork network' do - subject.execute(lvl2_forked_project_1_1) - - expect(target_project.reload.fork_network.root_project).to eq project_with_forks - end - end - - it 'rollbacks changes if transaction fails' do - allow(subject).to receive(:refresh_forks_count).and_raise(StandardError) - - expect { subject.execute(project_with_forks) }.to raise_error(StandardError) - - expect(project_with_forks.forks.count).to eq 2 - expect(target_project.forks.count).to eq 0 - end - end -end diff --git a/spec/services/projects/move_forks_service_spec.rb b/spec/services/projects/move_forks_service_spec.rb new file mode 100644 index 00000000000..f4a5a7f9fc2 --- /dev/null +++ b/spec/services/projects/move_forks_service_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' + +describe Projects::MoveForksService do + include ProjectForksHelper + + let!(:user) { create(:user) } + let!(:project_with_forks) { create(:project, namespace: user.namespace) } + let!(:target_project) { create(:project, namespace: user.namespace) } + let!(:lvl1_forked_project_1) { fork_project(project_with_forks, user) } + let!(:lvl1_forked_project_2) { fork_project(project_with_forks, user) } + let!(:lvl2_forked_project_1_1) { fork_project(lvl1_forked_project_1, user) } + let!(:lvl2_forked_project_1_2) { fork_project(lvl1_forked_project_1, user) } + + subject { described_class.new(target_project, user) } + + describe '#execute' do + context 'when moving a root forked project' do + it 'moves the descendant forks' do + expect(project_with_forks.forks.count).to eq 2 + expect(target_project.forks.count).to eq 0 + + subject.execute(project_with_forks) + + expect(project_with_forks.forks.count).to eq 0 + expect(target_project.forks.count).to eq 2 + expect(lvl1_forked_project_1.forked_from_project).to eq target_project + expect(lvl1_forked_project_1.fork_network_member.forked_from_project).to eq target_project + expect(lvl1_forked_project_2.forked_from_project).to eq target_project + expect(lvl1_forked_project_2.fork_network_member.forked_from_project).to eq target_project + end + + it 'updates the fork network' do + expect(project_with_forks.fork_network.root_project).to eq project_with_forks + expect(project_with_forks.fork_network.fork_network_members.map(&:project)).to include project_with_forks + + subject.execute(project_with_forks) + + expect(target_project.reload.fork_network.root_project).to eq target_project + expect(target_project.fork_network.fork_network_members.map(&:project)).not_to include project_with_forks + end + end + + context 'when moving a intermediate forked project' do + it 'moves the descendant forks' do + expect(lvl1_forked_project_1.forks.count).to eq 2 + expect(target_project.forks.count).to eq 0 + + subject.execute(lvl1_forked_project_1) + + expect(lvl1_forked_project_1.forks.count).to eq 0 + expect(target_project.forks.count).to eq 2 + expect(lvl2_forked_project_1_1.forked_from_project).to eq target_project + expect(lvl2_forked_project_1_1.fork_network_member.forked_from_project).to eq target_project + expect(lvl2_forked_project_1_2.forked_from_project).to eq target_project + expect(lvl2_forked_project_1_2.fork_network_member.forked_from_project).to eq target_project + end + + it 'moves the ascendant fork' do + subject.execute(lvl1_forked_project_1) + + expect(target_project.forked_from_project).to eq project_with_forks + expect(target_project.fork_network_member.forked_from_project).to eq project_with_forks + end + + it 'does not update fork network' do + subject.execute(lvl1_forked_project_1) + + expect(target_project.reload.fork_network.root_project).to eq project_with_forks + end + end + + context 'when moving a leaf forked project' do + it 'moves the ascendant fork' do + subject.execute(lvl2_forked_project_1_1) + + expect(target_project.forked_from_project).to eq lvl1_forked_project_1 + expect(target_project.fork_network_member.forked_from_project).to eq lvl1_forked_project_1 + end + + it 'does not update fork network' do + subject.execute(lvl2_forked_project_1_1) + + expect(target_project.reload.fork_network.root_project).to eq project_with_forks + end + end + + it 'rollbacks changes if transaction fails' do + allow(subject).to receive(:success).and_raise(StandardError) + + expect { subject.execute(project_with_forks) }.to raise_error(StandardError) + + expect(project_with_forks.forks.count).to eq 2 + expect(target_project.forks.count).to eq 0 + end + end +end diff --git a/spec/services/projects/move_lfs_objects_project_service_spec.rb b/spec/services/projects/move_lfs_objects_project_service_spec.rb deleted file mode 100644 index e79fc228a48..00000000000 --- a/spec/services/projects/move_lfs_objects_project_service_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveLfsObjectsProjectService do - let!(:user) { create(:user) } - let!(:project_with_lfs_objects) { create(:project, namespace: user.namespace) } - let!(:target_project) { create(:project, namespace: user.namespace) } - - subject { described_class.new(target_project, user) } - - before do - create_list(:lfs_objects_project, 3, project: project_with_lfs_objects) - end - - describe '#execute' do - it 'links the lfs objects from existent in source project' do - expect(target_project.lfs_objects.count).to eq 0 - - subject.execute(project_with_lfs_objects) - - expect(project_with_lfs_objects.reload.lfs_objects.count).to eq 0 - expect(target_project.reload.lfs_objects.count).to eq 3 - end - - it 'does not link existent lfs_object in the current project' do - target_project.lfs_objects << project_with_lfs_objects.lfs_objects.first(2) - - expect(target_project.lfs_objects.count).to eq 2 - - subject.execute(project_with_lfs_objects) - - expect(target_project.lfs_objects.count).to eq 3 - end - - it 'rollbacks changes if transaction fails' do - allow_any_instance_of(ActiveRecord::Associations::CollectionProxy).to receive(:destroy_all).and_raise(StandardError) - - expect { subject.execute(project_with_lfs_objects) }.to raise_error(StandardError) - - expect(project_with_lfs_objects.lfs_objects.count).to eq 3 - expect(target_project.lfs_objects.count).to eq 0 - end - end -end diff --git a/spec/services/projects/move_lfs_objects_projects_service_spec.rb b/spec/services/projects/move_lfs_objects_projects_service_spec.rb new file mode 100644 index 00000000000..409ba1e6cfa --- /dev/null +++ b/spec/services/projects/move_lfs_objects_projects_service_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +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) } + + subject { described_class.new(target_project, user) } + + before do + create_list(:lfs_objects_project, 3, project: project_with_lfs_objects) + end + + describe '#execute' do + it 'links the lfs objects from existent in source project' do + expect(target_project.lfs_objects.count).to eq 0 + + subject.execute(project_with_lfs_objects) + + expect(project_with_lfs_objects.reload.lfs_objects.count).to eq 0 + expect(target_project.reload.lfs_objects.count).to eq 3 + end + + it 'does not link existent lfs_object in the current project' do + target_project.lfs_objects << project_with_lfs_objects.lfs_objects.first(2) + + expect(target_project.lfs_objects.count).to eq 2 + + subject.execute(project_with_lfs_objects) + + expect(target_project.lfs_objects.count).to eq 3 + end + + it 'rollbacks changes if transaction fails' do + allow(subject).to receive(:success).and_raise(StandardError) + + expect { subject.execute(project_with_lfs_objects) }.to raise_error(StandardError) + + expect(project_with_lfs_objects.lfs_objects.count).to eq 3 + expect(target_project.lfs_objects.count).to eq 0 + end + end +end diff --git a/spec/services/projects/move_notification_setting_service_spec.rb b/spec/services/projects/move_notification_setting_service_spec.rb deleted file mode 100644 index 95edd380a8c..00000000000 --- a/spec/services/projects/move_notification_setting_service_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveNotificationSettingService do - let(:user) { create(:user) } - let(:project_with_notifications) { create(:project, namespace: user.namespace) } - let(:target_project) { create(:project, namespace: user.namespace) } - - subject { described_class.new(target_project, user) } - - describe '#execute' do - context 'with notification settings' do - before do - create_list(:notification_setting, 2, source: project_with_notifications) - end - - it 'moves the user\'s notification settings from one project to another' do - expect(project_with_notifications.notification_settings.count).to eq 3 - expect(target_project.notification_settings.count).to eq 1 - - subject.execute(project_with_notifications) - - expect(project_with_notifications.notification_settings.count).to eq 0 - expect(target_project.notification_settings.count).to eq 3 - end - - it 'rollbacks changes if transaction fails' do - allow_any_instance_of(ActiveRecord::Associations::CollectionProxy).to receive(:destroy_all).and_raise(StandardError) - - expect { subject.execute(project_with_notifications) }.to raise_error(StandardError) - - expect(project_with_notifications.notification_settings.count).to eq 3 - expect(target_project.notification_settings.count).to eq 1 - end - end - - it 'does not move existent notification settings in the current project' do - expect(project_with_notifications.notification_settings.count).to eq 1 - expect(target_project.notification_settings.count).to eq 1 - expect(user.notification_settings.count).to eq 2 - - subject.execute(project_with_notifications) - - expect(user.notification_settings.count).to eq 1 - end - end -end diff --git a/spec/services/projects/move_notification_settings_service_spec.rb b/spec/services/projects/move_notification_settings_service_spec.rb new file mode 100644 index 00000000000..347edc39e2b --- /dev/null +++ b/spec/services/projects/move_notification_settings_service_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +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) } + + subject { described_class.new(target_project, user) } + + describe '#execute' do + context 'with notification settings' do + before do + create_list(:notification_setting, 2, source: project_with_notifications) + end + + it 'moves the user\'s notification settings from one project to another' do + expect(project_with_notifications.notification_settings.count).to eq 3 + expect(target_project.notification_settings.count).to eq 1 + + subject.execute(project_with_notifications) + + expect(project_with_notifications.notification_settings.count).to eq 0 + expect(target_project.notification_settings.count).to eq 3 + end + + it 'rollbacks changes if transaction fails' do + allow(subject).to receive(:success).and_raise(StandardError) + + expect { subject.execute(project_with_notifications) }.to raise_error(StandardError) + + expect(project_with_notifications.notification_settings.count).to eq 3 + expect(target_project.notification_settings.count).to eq 1 + end + end + + it 'does not move existent notification settings in the current project' do + expect(project_with_notifications.notification_settings.count).to eq 1 + expect(target_project.notification_settings.count).to eq 1 + expect(user.notification_settings.count).to eq 2 + + subject.execute(project_with_notifications) + + expect(user.notification_settings.count).to eq 1 + end + end +end 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_link_service_spec.rb deleted file mode 100644 index 4b79b81e9cd..00000000000 --- a/spec/services/projects/move_project_group_link_service_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveProjectGroupLinkService do - let!(:user) { create(:user) } - let(:project_with_groups) { create(:project, namespace: user.namespace) } - let(:target_project) { create(:project, namespace: user.namespace) } - let(:master_group) { create(:group) } - let(:reporter_group) { create(:group) } - let(:developer_group) { create(:group) } - - subject { described_class.new(target_project, user) } - - describe '#execute' do - before do - project_with_groups.project_group_links.create(group: master_group, group_access: Gitlab::Access::MASTER) - project_with_groups.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) - project_with_groups.project_group_links.create(group: reporter_group, group_access: Gitlab::Access::REPORTER) - end - - it 'moves the group links from one project to another' do - expect(project_with_groups.project_group_links.count).to eq 3 - expect(target_project.project_group_links.count).to eq 0 - - subject.execute(project_with_groups) - - expect(project_with_groups.project_group_links.count).to eq 0 - expect(target_project.project_group_links.count).to eq 3 - end - - it 'does not move existent group links in the current project' do - target_project.project_group_links.create(group: master_group, group_access: Gitlab::Access::MASTER) - target_project.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) - - expect(project_with_groups.project_group_links.count).to eq 3 - expect(target_project.project_group_links.count).to eq 2 - - subject.execute(project_with_groups) - - expect(project_with_groups.project_group_links.count).to eq 0 - expect(target_project.project_group_links.count).to eq 3 - end - - it 'rollbacks changes if transaction fails' do - allow_any_instance_of(ActiveRecord::Associations::CollectionProxy).to receive(:destroy_all).and_raise(StandardError) - - expect { subject.execute(project_with_groups) }.to raise_error(StandardError) - - expect(project_with_groups.project_group_links.count).to eq 3 - expect(target_project.project_group_links.count).to eq 0 - end - end -end diff --git a/spec/services/projects/move_project_group_links_service_spec.rb b/spec/services/projects/move_project_group_links_service_spec.rb new file mode 100644 index 00000000000..a27eee926c7 --- /dev/null +++ b/spec/services/projects/move_project_group_links_service_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +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) } + let(:master_group) { create(:group) } + let(:reporter_group) { create(:group) } + let(:developer_group) { create(:group) } + + subject { described_class.new(target_project, user) } + + describe '#execute' do + before do + project_with_groups.project_group_links.create(group: master_group, group_access: Gitlab::Access::MASTER) + project_with_groups.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) + project_with_groups.project_group_links.create(group: reporter_group, group_access: Gitlab::Access::REPORTER) + end + + it 'moves the group links from one project to another' do + expect(project_with_groups.project_group_links.count).to eq 3 + expect(target_project.project_group_links.count).to eq 0 + + subject.execute(project_with_groups) + + expect(project_with_groups.project_group_links.count).to eq 0 + expect(target_project.project_group_links.count).to eq 3 + end + + it 'does not move existent group links in the current project' do + target_project.project_group_links.create(group: master_group, group_access: Gitlab::Access::MASTER) + target_project.project_group_links.create(group: developer_group, group_access: Gitlab::Access::DEVELOPER) + + expect(project_with_groups.project_group_links.count).to eq 3 + expect(target_project.project_group_links.count).to eq 2 + + subject.execute(project_with_groups) + + expect(project_with_groups.project_group_links.count).to eq 0 + expect(target_project.project_group_links.count).to eq 3 + end + + it 'rollbacks changes if transaction fails' do + allow(subject).to receive(:success).and_raise(StandardError) + + expect { subject.execute(project_with_groups) }.to raise_error(StandardError) + + expect(project_with_groups.project_group_links.count).to eq 3 + expect(target_project.project_group_links.count).to eq 0 + end + end +end diff --git a/spec/services/projects/move_project_member_service_spec.rb b/spec/services/projects/move_project_member_service_spec.rb deleted file mode 100644 index 6f96ed9d906..00000000000 --- a/spec/services/projects/move_project_member_service_spec.rb +++ /dev/null @@ -1,66 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveProjectMemberService 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 members from one project to another' do - expect(project_with_users.project_members.count).to eq 4 - expect(target_project.project_members.count).to eq 1 - - subject.execute(project_with_users) - - expect(project_with_users.project_members.count).to eq 0 - 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 - 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) - - expect { subject.execute(project_with_users) }.to raise_error(StandardError) - - expect(project_with_users.project_members.count).to eq 4 - expect(target_project.project_members.count).to eq 1 - end - end -end diff --git a/spec/services/projects/move_project_members_service_spec.rb b/spec/services/projects/move_project_members_service_spec.rb new file mode 100644 index 00000000000..d1d4e06e91e --- /dev/null +++ b/spec/services/projects/move_project_members_service_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +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) } + 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 members from one project to another' do + expect(project_with_users.project_members.count).to eq 4 + expect(target_project.project_members.count).to eq 1 + + subject.execute(project_with_users) + + expect(project_with_users.project_members.count).to eq 0 + expect(target_project.project_members.count).to eq 4 + end + + 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 + + subject.execute(project_with_users) + + expect(project_with_users.project_members.count).to eq 0 + expect(target_project.project_members.count).to eq 4 + end + + it 'rollbacks changes if transaction fails' do + allow(subject).to receive(:success).and_raise(StandardError) + + expect { subject.execute(project_with_users) }.to raise_error(StandardError) + + expect(project_with_users.project_members.count).to eq 4 + expect(target_project.project_members.count).to eq 1 + end + end +end diff --git a/spec/services/projects/move_service_spec.rb b/spec/services/projects/move_service_spec.rb deleted file mode 100644 index 4ae9dd464e2..00000000000 --- a/spec/services/projects/move_service_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveService do - it_behaves_like 'move services access rights' -end diff --git a/spec/services/projects/move_users_star_project_service_spec.rb b/spec/services/projects/move_users_star_project_service_spec.rb deleted file mode 100644 index d2fe1986255..00000000000 --- a/spec/services/projects/move_users_star_project_service_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'spec_helper' - -describe Projects::MoveUsersStarProjectService do - let!(:user) { create(:user) } - let!(:project_with_stars) { 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(:users_star_project, 2, project: project_with_stars) - end - - it 'moves the user\'s stars from one project to another' do - expect(project_with_stars.users_star_projects.count).to eq 2 - expect(project_with_stars.star_count).to eq 2 - expect(target_project.users_star_projects.count).to eq 0 - expect(target_project.star_count).to eq 0 - - subject.execute(project_with_stars) - project_with_stars.reload - target_project.reload - - expect(project_with_stars.users_star_projects.count).to eq 0 - expect(project_with_stars.star_count).to eq 0 - expect(target_project.users_star_projects.count).to eq 2 - expect(target_project.star_count).to eq 2 - 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) - - expect { subject.execute(project_with_stars) }.to raise_error(StandardError) - - expect(project_with_stars.users_star_projects.count).to eq 2 - expect(project_with_stars.star_count).to eq 2 - expect(target_project.users_star_projects.count).to eq 0 - expect(target_project.star_count).to eq 0 - end - end -end diff --git a/spec/services/projects/move_users_star_projects_service_spec.rb b/spec/services/projects/move_users_star_projects_service_spec.rb new file mode 100644 index 00000000000..e0545c5a21b --- /dev/null +++ b/spec/services/projects/move_users_star_projects_service_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +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) } + + subject { described_class.new(target_project, user) } + + describe '#execute' do + before do + create_list(:users_star_project, 2, project: project_with_stars) + end + + it 'moves the user\'s stars from one project to another' do + expect(project_with_stars.users_star_projects.count).to eq 2 + expect(project_with_stars.star_count).to eq 2 + expect(target_project.users_star_projects.count).to eq 0 + expect(target_project.star_count).to eq 0 + + subject.execute(project_with_stars) + project_with_stars.reload + target_project.reload + + expect(project_with_stars.users_star_projects.count).to eq 0 + expect(project_with_stars.star_count).to eq 0 + expect(target_project.users_star_projects.count).to eq 2 + expect(target_project.star_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_stars) }.to raise_error(StandardError) + + expect(project_with_stars.users_star_projects.count).to eq 2 + expect(project_with_stars.star_count).to eq 2 + expect(target_project.users_star_projects.count).to eq 0 + expect(target_project.star_count).to eq 0 + end + end +end -- cgit v1.2.1