diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/controllers/groups_controller.rb | 19 | ||||
| -rw-r--r-- | app/helpers/groups_helper.rb | 4 | ||||
| -rw-r--r-- | app/models/concerns/storage/legacy_namespace.rb | 1 | ||||
| -rw-r--r-- | app/models/group.rb | 13 | ||||
| -rw-r--r-- | app/services/groups/convert_to_root_service.rb | 35 | ||||
| -rw-r--r-- | app/services/groups/convert_to_subgroup_service.rb | 66 | ||||
| -rw-r--r-- | app/services/groups/transfer_error_messages.rb | 26 | ||||
| -rw-r--r-- | app/services/groups/transfer_service.rb | 85 | ||||
| -rw-r--r-- | app/views/groups/convert_to_root.js.haml | 2 | ||||
| -rw-r--r-- | app/views/groups/edit.html.haml | 14 | ||||
| -rw-r--r-- | app/views/groups/transfer.js.haml | 2 |
11 files changed, 94 insertions, 173 deletions
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index e5c3046a097..54c0bb4acb8 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -98,20 +98,15 @@ class GroupsController < Groups::ApplicationController return access_denied! unless can?(current_user, :admin_group, @group) parent_group = Group.find_by(id: params[:new_parent_group_id]) + service = ::Groups::TransferService.new(@group, current_user) - service = ::Groups::ConvertToSubgroupService.new(@group, current_user) - service.execute(parent_group) - - flash[:alert] = service.error if service.error.present? - end - - def convert_to_root - return access_denied! unless can?(current_user, :admin_group, @group) - - service = ::Groups::ConvertToRootService.new(@group, current_user) - service.execute + if service.execute(parent_group) + flash[:notice] = "Group '#{@group.name}' was successfully transferred." + else + flash[:alert] = service.error + end - flash[:alert] = service.error if service.error.present? + redirect_to @group end protected diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index bf85c97e4c4..1401ffdf1ea 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -97,10 +97,6 @@ module GroupsHelper Group.supports_nested_groups? end - def group_can_become_root?(group) - group.has_parent? && can?(current_user, :create_group) - end - private def group_title_link(group, hidable: false, show_avatar: false, for_dropdown: false) diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index f3535a23702..67a988addbe 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -18,6 +18,7 @@ module Storage gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" # if we cannot move namespace directory we should rollback diff --git a/app/models/group.rb b/app/models/group.rb index cb46302af62..8d17c8befee 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -274,6 +274,15 @@ class Group < Namespace list_of_ids.reverse.map { |group| variables[group.id] }.compact.flatten end + def full_path_was + if parent_id_was.nil? + path_was + else + previous_parent = Group.find_by(id: parent_id_was) + previous_parent.full_path + '/' + path_was + end + end + def group_member(user) if group_members.loaded? group_members.find { |gm| gm.user_id == user.id } @@ -286,10 +295,6 @@ class Group < Namespace false end - def has_parent? - !parent_id.nil? - end - private def update_two_factor_requirement diff --git a/app/services/groups/convert_to_root_service.rb b/app/services/groups/convert_to_root_service.rb deleted file mode 100644 index 7efbfd97b3b..00000000000 --- a/app/services/groups/convert_to_root_service.rb +++ /dev/null @@ -1,35 +0,0 @@ -module Groups - class ConvertToRootService < TransferService - def execute - with_transfer_error_handling do - proceed_to_transfer - end - end - - private - - def proceed_to_transfer - ensure_policies - update_group_attributes - end - - def ensure_policies - raise_transfer_error(:invalid_policies) unless valid_policies? - raise_transfer_error(:already_a_root_group) if root_group? - end - - def valid_policies? - can?(current_user, :admin_group, @group) && - can?(current_user, :create_group) - end - - def root_group? - !@group.has_parent? - end - - def update_group_attributes - @group.parent = nil - @group.save! - end - end -end diff --git a/app/services/groups/convert_to_subgroup_service.rb b/app/services/groups/convert_to_subgroup_service.rb deleted file mode 100644 index 786aadac948..00000000000 --- a/app/services/groups/convert_to_subgroup_service.rb +++ /dev/null @@ -1,66 +0,0 @@ -module Groups - class ConvertToSubgroupService < TransferService - def execute(new_parent_group) - @new_parent_group = new_parent_group - - with_transfer_error_handling do - proceed_to_transfer - end - end - - private - - def proceed_to_transfer - ensure_allowed_transfer - update_visibility(new_visibility_level) - update_group_attributes - end - - def ensure_allowed_transfer - raise_transfer_error(:database_not_supported) unless Group.supports_nested_groups? - raise_transfer_error(:missing_parent) if @new_parent_group.blank? - raise_transfer_error(:same_parent_as_current) if same_parent? - raise_transfer_error(:invalid_policies) unless valid_policies? - raise_transfer_error(:group_with_same_path) if group_with_same_path? - raise_transfer_error(visibility_error_for(@group, @new_parent_group)) unless allowed_visibility? - end - - def same_parent? - @new_parent_group.id == @group.parent_id - end - - def valid_policies? - can?(current_user, :admin_group, @group) && - can?(current_user, :create_subgroup, @new_parent_group) - end - - def allowed_visibility? - @group.visibility_level <= new_visibility_level - end - - def update_visibility(visibility_level) - @group.projects.update_all(visibility_level: visibility_level) - @group.self_and_descendants.each do |subgroup| - subgroup.update_attribute(:visibility_level, visibility_level) - end - @group.all_projects.update_all(visibility_level: visibility_level) - end - - def update_group_attributes - old_visibility_level = @group.visibility_level - @group.visibility_level = new_visibility_level - @group.parent = @new_parent_group - @group.save! - rescue ActiveRecord::RecordInvalid - update_visibility(old_visibility_level) - end - - def group_with_same_path? - @new_parent_group.children.exists?(path: @group.path) - end - - def new_visibility_level - @new_parent_group.visibility_level - end - end -end diff --git a/app/services/groups/transfer_error_messages.rb b/app/services/groups/transfer_error_messages.rb deleted file mode 100644 index d3f50d7fc36..00000000000 --- a/app/services/groups/transfer_error_messages.rb +++ /dev/null @@ -1,26 +0,0 @@ -module Groups - module TransferErrorMessages - def friendly_update_path_error - "Oops, there was a problem transferring the group, this is not your fault. Please contact an admin." - end - - def error_messages - { - already_a_root_group: "Group is already a root group.", - database_not_supported: "Database is not supported.", - group_with_same_path: "The parent group has a group with the same path.", - missing_parent: "Please select a new parent for your group.", - same_parent_as_current: "Group is already associated to the parent group.", - invalid_policies: "You don't have enough permissions." - } - end - - def visibility_error_for(group, new_parent_group) - "#{gitlab_visibility_level(group.visibility_level)} #{group.path} cannot be transferred because the parent visibility is #{gitlab_visibility_level(new_parent_group.visibility_level)}." - end - - def gitlab_visibility_level(visibility_level) - Gitlab::VisibilityLevel.string_level(visibility_level) - end - end -end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 58e8cca04bb..6f6f33c2903 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -1,8 +1,15 @@ module Groups class TransferService < Groups::BaseService - include TransferErrorMessages + ERROR_MESSAGES = { + database_not_supported: 'Database is not supported.', + group_with_same_path: 'The parent group has a group with the same path.', + group_is_already_root: 'Group is already a root group.', + same_parent_as_current: 'Group is already associated to the parent group.', + invalid_policies: "You don't have enough permissions." + }.freeze TransferError = Class.new(StandardError) + attr_reader :error def initialize(group, user, params = {}) @@ -10,24 +17,78 @@ module Groups @error = nil end - private + def execute(new_parent_group) + @new_parent_group = new_parent_group + ensure_allowed_transfer + proceed_to_transfer - def with_transfer_error_handling - yield - rescue TransferError => e + rescue TransferError, ActiveRecord::RecordInvalid, Gitlab::UpdatePathError => e @error = "Transfer failed: " + e.message false - rescue Gitlab::UpdatePathError - @error = friendly_update_path_error - false end - def raise_transfer_error(message) - if message.is_a?(Symbol) - raise TransferError, error_messages[message] + private + + def proceed_to_transfer + Group.transaction do + update_children_and_projects_visibility if @new_parent_group.present? + update_group_attributes + end + end + + def ensure_allowed_transfer + raise_transfer_error(:group_is_already_root) if group_is_already_root + raise_transfer_error(:database_not_supported) unless Group.supports_nested_groups? + raise_transfer_error(:same_parent_as_current) if same_parent? + raise_transfer_error(:group_with_same_path) if group_with_same_path? + raise_transfer_error(:invalid_policies) unless valid_policies? + end + + def group_is_already_root + @new_parent_group.blank? && @group.parent.nil? + end + + def same_parent? + @new_parent_group.present? && @new_parent_group.id == @group.parent_id + end + + def valid_policies? + if @new_parent_group.present? + can?(current_user, :admin_group, @group) && + can?(current_user, :create_subgroup, @new_parent_group) else - raise TransferError, message + can?(current_user, :admin_group, @group) + end + end + + def group_with_same_path? + if @new_parent_group.present? + @new_parent_group.children.exists?(path: @group.path) + else + Group.exists?(path: @group.path, parent: nil) + end + end + + def update_group_attributes + @group.visibility_level = @new_parent_group.visibility_level if @new_parent_group.present? + @group.parent = @new_parent_group + @group.save! + end + + def update_children_and_projects_visibility + descendants = @group.descendants.where("visibility_level > ?", @new_parent_group.visibility_level) + descendants.each do |subgroup| + subgroup.update_column(:visibility_level, @new_parent_group.visibility_level) end + + @group + .projects + .where("visibility_level > ?", @new_parent_group.visibility_level) + .update_all(visibility_level: @new_parent_group.visibility_level) + end + + def raise_transfer_error(message) + raise TransferError, ERROR_MESSAGES[message] end end end diff --git a/app/views/groups/convert_to_root.js.haml b/app/views/groups/convert_to_root.js.haml deleted file mode 100644 index efe60f4791b..00000000000 --- a/app/views/groups/convert_to_root.js.haml +++ /dev/null @@ -1,2 +0,0 @@ -:plain - location.href = "#{edit_group_path(@group)}"; diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 913eaf278b2..6d844c3e428 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -57,27 +57,21 @@ .form-actions = button_to 'Remove group', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_group_message(@group) } - - if supports_nested_groups? .panel.panel-warning .panel-heading Transfer group .panel-body - = form_for @group, url: transfer_group_path(@group), method: :put, remote: true do |f| + = form_for @group, url: transfer_group_path(@group), method: :put do |f| .form-group = label_tag :new_parent_group_id, nil, class: 'label-light' do %span Parent Group .form-group - = select_tag :new_parent_group_id, parent_group_options, include_blank: 'Select', class: 'select2' + = select_tag :new_parent_group_id, parent_group_options, include_blank: 'No parent group', class: 'select2' %ul %li Be careful. Changing a group's parent can have unintended side effects. - %li You can only transfer the group to a group you owned. + %li You can only transfer the group to a group you manage. %li You will need to update your local repositories to point to the new location. %li Group and project visibility levels will be changed to match the new parent group's values. - %li Permissions of the members of the group will not be transferred. You will need to change them manually. - = f.submit 'Transfer Group', class: "btn btn-warning" - - if group_can_become_root?(@group) - or - = link_to 'Convert to root group', convert_to_root_group_path(@group), remote: true, method: :put, class: "btn btn-warning" - + = f.submit 'Transfer group', class: "btn btn-warning" = render 'shared/confirm_modal', phrase: @group.path diff --git a/app/views/groups/transfer.js.haml b/app/views/groups/transfer.js.haml deleted file mode 100644 index efe60f4791b..00000000000 --- a/app/views/groups/transfer.js.haml +++ /dev/null @@ -1,2 +0,0 @@ -:plain - location.href = "#{edit_group_path(@group)}"; |