diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index d36e359934cf1a846d81b23b9b55c04db58ebeaf..06d6d61e907aa80c0cd4c91b9da2674db1f64b6d 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -86,11 +86,7 @@ class Admin::UsersController < Admin::ApplicationController
end
def destroy
- # 1. Remove groups where user is the only owner
- user.solo_owned_groups.map(&:destroy)
-
- # 2. Remove user with all authored content including personal projects
- user.destroy
+ DeleteUserService.new.execute(user)
respond_to do |format|
format.html { redirect_to admin_users_path }
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 830751a989f25042cd994fa4bb3892776ab8a894..6e57fded337263cdf3d1596f117ec39a681dc3dd 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -6,7 +6,7 @@ class RegistrationsController < Devise::RegistrationsController
end
def destroy
- current_user.destroy
+ DeleteUserService.new.execute(user)
respond_to do |format|
format.html { redirect_to new_user_session_path, notice: "Account successfully removed." }
diff --git a/app/models/user.rb b/app/models/user.rb
index 50ca4bc5acc5941eb5fc914fc94cf39c4cb5f2fb..c1bb51e86fcb2b757316204c9e609c9cabaa1b2b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -689,4 +689,8 @@ class User < ActiveRecord::Base
true
end
+
+ def can_be_removed?
+ !solo_owned_groups.present?
+ end
end
diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d259b4efca63fe6bbb25a150a556ac97bef9a9e5
--- /dev/null
+++ b/app/services/delete_user_service.rb
@@ -0,0 +1,10 @@
+class DeleteUserService
+ def execute(user)
+ if user.solo_owned_groups.present?
+ user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
+ user
+ else
+ user.destroy
+ end
+ end
+end
diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml
index fe6484702333c52873e4217bb81ec7e455922413..45dee86b0177e56aefd7049211bf9b6cadf86e9a 100644
--- a/app/views/admin/users/index.html.haml
+++ b/app/views/admin/users/index.html.haml
@@ -79,11 +79,12 @@
%i.fa.fa-envelope
= mail_to user.email, user.email, class: 'light'
- = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-sm"
+ = link_to 'Edit', edit_admin_user_path(user), id: "edit_#{dom_id(user)}", class: "btn btn-xs"
- unless user == current_user
- if user.blocked?
- = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-sm success"
+ = link_to 'Unblock', unblock_admin_user_path(user), method: :put, class: "btn btn-xs btn-success"
- else
- = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-sm btn-remove"
- = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All tickets linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: "btn btn-sm btn-remove"
+ = link_to 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
+ - if user.can_be_removed?
+ = link_to 'Destroy', [:admin, user], data: { confirm: "USER #{user.name} WILL BE REMOVED! All tickets linked to this user will also be removed! Maybe block the user instead? Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
= paginate @users, theme: "gitlab"
diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml
index 7fc85206109d2472d77e8a9c84de192544e8e954..f7195ac3326e9c34f775f37fe2bd6adbe91bdebf 100644
--- a/app/views/admin/users/show.html.haml
+++ b/app/views/admin/users/show.html.haml
@@ -140,18 +140,22 @@
.panel-heading
Remove user
.panel-body
- %p Deleting a user has the following effects:
- %ul
- %li All user content like authored issues, snippets, comments will be removed
- - rp = @user.personal_projects.count
- - unless rp.zero?
- %li #{pluralize rp, 'personal project'} will be removed and cannot be restored
+ - if @user.can_be_removed?
+ %p Deleting a user has the following effects:
+ %ul
+ %li All user content like authored issues, snippets, comments will be removed
+ - rp = @user.personal_projects.count
+ - unless rp.zero?
+ %li #{pluralize rp, 'personal project'} will be removed and cannot be restored
+ %br
+ = link_to 'Remove user', [:admin, @user], data: { confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-remove"
+ - else
- if @user.solo_owned_groups.present?
- %li
- Next groups with all content will be removed:
+ %p
+ This user is currently an owner in these groups:
%strong #{@user.solo_owned_groups.map(&:name).join(', ')}
- %br
- = link_to 'Remove user', [:admin, @user], data: { confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-remove"
+ %p
+ You must transfer ownership or delete these groups before you can delete this user.
#profile.tab-pane
.row
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index 06bad7dd84a48fa4924e512cb9d2e122702a0b4a..4d1d50dcbabb3ac10a78d950fb189b2817f20832 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -91,15 +91,19 @@
%legend
Remove account
%div
- %p Deleting an account has the following effects:
- %ul
- %li All user content like authored issues, snippets, comments will be removed
- - rp = current_user.personal_projects.count
- - unless rp.zero?
- %li #{pluralize rp, 'personal project'} will be removed and cannot be restored
- - if current_user.solo_owned_groups.present?
- %li
- The following groups will be abandoned. You should transfer or remove them:
- %strong #{current_user.solo_owned_groups.map(&:name).join(', ')}
- = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove"
+ - if @user.can_be_removed?
+ %p Deleting an account has the following effects:
+ %ul
+ %li All user content like authored issues, snippets, comments will be removed
+ - rp = current_user.personal_projects.count
+ - unless rp.zero?
+ %li #{pluralize rp, 'personal project'} will be removed and cannot be restored
+ = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove"
+ - else
+ - if @user.solo_owned_groups.present?
+ %p
+ Your account is currently an owner in these groups:
+ %strong #{@user.solo_owned_groups.map(&:name).join(', ')}
+ %p
+ You must transfer ownership or delete these groups before you can delete yur account.
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 032a5d76e4394027129936b6c2bfa8a1cae118bc..7d4c68c7412e1108e33265efd57598c746e04ebc 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -194,7 +194,7 @@ module API
user = User.find_by(id: params[:id])
if user
- user.destroy
+ DeleteUserService.new.execute(user)
else
not_found!('User')
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index e1205c18a85610d87cacf838eec936581b0ced22..49c7b7d99cedce03754f2f0ef726a2d96c1c135f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -572,7 +572,6 @@ describe User do
end
describe "#contributed_projects_ids" do
-
subject { create(:user) }
let!(:project1) { create(:project) }
let!(:project2) { create(:project, forked_from_project: project3) }
@@ -598,4 +597,21 @@ describe User do
expect(subject.contributed_projects_ids).not_to include(project2.id)
end
end
+
+ describe :can_be_removed? do
+ subject { create(:user) }
+
+ context 'no owned groups' do
+ it { expect(subject.can_be_removed?).to be_truthy }
+ end
+
+ context 'has owned groups' do
+ before do
+ group = create(:group)
+ group.add_owner(subject)
+ end
+
+ it { expect(subject.can_be_removed?).to be_falsey }
+ end
+ end
end