Commit 05a44dcb authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'user-destroy-wo-groups' into 'master'

You can not remove user if he/she is an only owner of group

To prevent loose of group data you need to transfer or remove group
first before you can remove user
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

See merge request !730
parents 06250eef 2db02679
...@@ -28,6 +28,7 @@ v 7.12.0 (unreleased) ...@@ -28,6 +28,7 @@ v 7.12.0 (unreleased)
- Group project contributions by both name and email. - Group project contributions by both name and email.
- Clarify navigation labels for Project Settings and Group Settings. - Clarify navigation labels for Project Settings and Group Settings.
- Move user avatar and logout button to sidebar - Move user avatar and logout button to sidebar
- You can not remove user if he/she is an only owner of group
v 7.11.2 v 7.11.2
- no changes - no changes
......
...@@ -86,11 +86,7 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -86,11 +86,7 @@ class Admin::UsersController < Admin::ApplicationController
end end
def destroy def destroy
# 1. Remove groups where user is the only owner DeleteUserService.new.execute(user)
user.solo_owned_groups.map(&:destroy)
# 2. Remove user with all authored content including personal projects
user.destroy
respond_to do |format| respond_to do |format|
format.html { redirect_to admin_users_path } format.html { redirect_to admin_users_path }
......
...@@ -6,7 +6,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -6,7 +6,7 @@ class RegistrationsController < Devise::RegistrationsController
end end
def destroy def destroy
current_user.destroy DeleteUserService.new.execute(current_user)
respond_to do |format| respond_to do |format|
format.html { redirect_to new_user_session_path, notice: "Account successfully removed." } format.html { redirect_to new_user_session_path, notice: "Account successfully removed." }
......
...@@ -689,4 +689,8 @@ class User < ActiveRecord::Base ...@@ -689,4 +689,8 @@ class User < ActiveRecord::Base
true true
end end
def can_be_removed?
!solo_owned_groups.present?
end
end end
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
...@@ -79,11 +79,12 @@ ...@@ -79,11 +79,12 @@
%i.fa.fa-envelope %i.fa.fa-envelope
= mail_to user.email, user.email, class: 'light' = mail_to user.email, user.email, class: 'light'
&nbsp; &nbsp;
= 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 - unless user == current_user
- if user.blocked? - 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 - 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 'Block', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs btn-warning"
= 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" - 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" = paginate @users, theme: "gitlab"
...@@ -140,18 +140,22 @@ ...@@ -140,18 +140,22 @@
.panel-heading .panel-heading
Remove user Remove user
.panel-body .panel-body
- if @user.can_be_removed?
%p Deleting a user has the following effects: %p Deleting a user has the following effects:
%ul %ul
%li All user content like authored issues, snippets, comments will be removed %li All user content like authored issues, snippets, comments will be removed
- rp = @user.personal_projects.count - rp = @user.personal_projects.count
- unless rp.zero? - unless rp.zero?
%li #{pluralize rp, 'personal project'} will be removed and cannot be restored %li #{pluralize rp, 'personal project'} will be removed and cannot be restored
- if @user.solo_owned_groups.present?
%li
Next groups with all content will be removed:
%strong #{@user.solo_owned_groups.map(&:name).join(', ')}
%br %br
= link_to 'Remove user', [:admin, @user], data: { confirm: "USER #{@user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-remove" = 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?
%p
This user 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 this user.
#profile.tab-pane #profile.tab-pane
.row .row
......
...@@ -91,15 +91,19 @@ ...@@ -91,15 +91,19 @@
%legend %legend
Remove account Remove account
%div %div
- if @user.can_be_removed?
%p Deleting an account has the following effects: %p Deleting an account has the following effects:
%ul %ul
%li All user content like authored issues, snippets, comments will be removed %li All user content like authored issues, snippets, comments will be removed
- rp = current_user.personal_projects.count - rp = current_user.personal_projects.count
- unless rp.zero? - unless rp.zero?
%li #{pluralize rp, 'personal project'} will be removed and cannot be restored %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" = 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.
...@@ -194,7 +194,7 @@ module API ...@@ -194,7 +194,7 @@ module API
user = User.find_by(id: params[:id]) user = User.find_by(id: params[:id])
if user if user
user.destroy DeleteUserService.new.execute(user)
else else
not_found!('User') not_found!('User')
end end
......
...@@ -572,7 +572,6 @@ describe User do ...@@ -572,7 +572,6 @@ describe User do
end end
describe "#contributed_projects_ids" do describe "#contributed_projects_ids" do
subject { create(:user) } subject { create(:user) }
let!(:project1) { create(:project) } let!(:project1) { create(:project) }
let!(:project2) { create(:project, forked_from_project: project3) } let!(:project2) { create(:project, forked_from_project: project3) }
...@@ -598,4 +597,21 @@ describe User do ...@@ -598,4 +597,21 @@ describe User do
expect(subject.contributed_projects_ids).not_to include(project2.id) expect(subject.contributed_projects_ids).not_to include(project2.id)
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment