add a way to delete posts and topics when deleting a user with UserDestroyer

This commit is contained in:
Neil Lalonde
2013-07-24 13:48:55 -04:00
parent a8df9778b5
commit e25638dab0
6 changed files with 87 additions and 49 deletions

View File

@ -117,10 +117,14 @@ class Admin::UsersController < Admin::AdminController
def destroy def destroy
user = User.where(id: params[:id]).first user = User.where(id: params[:id]).first
guardian.ensure_can_delete_user!(user) guardian.ensure_can_delete_user!(user)
if UserDestroyer.new(current_user).destroy(user) begin
render json: {deleted: true} if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts))
else render json: {deleted: true}
render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json} else
render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json}
end
rescue UserDestroyer::PostsExistError
raise Discourse::InvalidAccess.new("User #{user.username} has #{user.post_count} posts, so can't be deleted.")
end end
end end

View File

@ -150,8 +150,8 @@ class Guardian
user && is_staff? user && is_staff?
end end
def can_delete_user?(user_to_delete) def can_delete_user?(user)
can_administer?(user_to_delete) && user_to_delete.post_count <= 0 can_administer?(user)
end end
# Can we see who acted on a post in a particular way? # Can we see who acted on a post in a particular way?

View File

@ -11,10 +11,16 @@ class UserDestroyer
# Returns false if the user failed to be deleted. # Returns false if the user failed to be deleted.
# Returns a frozen instance of the User if the delete succeeded. # Returns a frozen instance of the User if the delete succeeded.
def destroy(user) def destroy(user, opts={})
raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User) raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User)
raise PostsExistError if user.post_count != 0 raise PostsExistError if !opts[:delete_posts] && user.post_count != 0
User.transaction do User.transaction do
if opts[:delete_posts]
user.posts.each do |post|
PostDestroyer.new(@admin, post).destroy
end
raise PostsExistError if user.reload.post_count != 0
end
user.destroy.tap do |u| user.destroy.tap do |u|
if u if u
Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true")

View File

@ -1008,9 +1008,9 @@ describe Guardian do
end end
context "for admins" do context "for admins" do
it "is false if user has posts" do it "is true if user has posts" do # UserDestroyer is responsible for checking for posts
Fabricate(:post, user: user) user.stubs(:post_count).returns(1)
Guardian.new(admin).can_delete_user?(user).should be_false Guardian.new(admin).can_delete_user?(user).should be_true
end end
it "is true if user has no posts" do it "is true if user has no posts" do

View File

@ -35,8 +35,6 @@ describe UserDestroyer do
@user = Fabricate(:user) @user = Fabricate(:user)
end end
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) }
it 'raises an error when user is nil' do it 'raises an error when user is nil' do
expect { UserDestroyer.new(@admin).destroy(nil) }.to raise_error(Discourse::InvalidParameters) expect { UserDestroyer.new(@admin).destroy(nil) }.to raise_error(Discourse::InvalidParameters)
end end
@ -45,58 +43,79 @@ describe UserDestroyer do
expect { UserDestroyer.new(@admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters) expect { UserDestroyer.new(@admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters)
end end
context 'user has posts' do shared_examples "successfully destroy a user" do
before do it 'should delete the user' do
Fabricate(:post, user: @user) expect { destroy }.to change { User.count }.by(-1)
end end
it 'should not delete the user' do it 'should return the deleted user record' do
expect { destroy rescue nil }.to_not change { User.count } return_value = destroy
return_value.should == @user
return_value.should be_destroyed
end end
it 'should raise an error' do it 'should log the action' do
expect { destroy }.to raise_error( UserDestroyer::PostsExistError ) StaffActionLogger.any_instance.expects(:log_user_deletion).with(@user).once
destroy
end end
it 'should not log the action' do it 'should unregister the nickname as the discourse hub if hub integration is enabled' do
StaffActionLogger.any_instance.expects(:log_user_deletion).never SiteSetting.stubs(:call_discourse_hub?).returns(true)
destroy rescue nil DiscourseHub.expects(:unregister_nickname).with(@user.username)
destroy
end end
it 'should not unregister the user at the discourse hub' do it 'should not try to unregister the nickname as the discourse hub if hub integration is disabled' do
SiteSetting.stubs(:call_discourse_hub?).returns(false)
DiscourseHub.expects(:unregister_nickname).never DiscourseHub.expects(:unregister_nickname).never
destroy rescue nil destroy
end
end
context 'user has posts' do
let!(:post) { Fabricate(:post, user: @user) }
context "delete_posts is false" do
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) }
it 'should not delete the user' do
expect { destroy rescue nil }.to_not change { User.count }
end
it 'should raise an error' do
expect { destroy }.to raise_error( UserDestroyer::PostsExistError )
end
it 'should not log the action' do
StaffActionLogger.any_instance.expects(:log_user_deletion).never
destroy rescue nil
end
it 'should not unregister the user at the discourse hub' do
DiscourseHub.expects(:unregister_nickname).never
destroy rescue nil
end
end
context "delete_posts is true" do
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user, delete_posts: true) }
include_examples "successfully destroy a user"
it "deletes the posts" do
destroy
post.reload.deleted_at.should_not be_nil
post.nuked_user.should be_true
end
end end
end end
context 'user has no posts' do context 'user has no posts' do
context 'and destroy succeeds' do context 'and destroy succeeds' do
it 'should delete the user' do
expect { destroy }.to change { User.count }.by(-1)
end
it 'should return the deleted user record' do subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) }
return_value = destroy
return_value.should == @user
return_value.should be_destroyed
end
it 'should log the action' do include_examples "successfully destroy a user"
StaffActionLogger.any_instance.expects(:log_user_deletion).with(@user).once
destroy
end
it 'should unregister the nickname as the discourse hub if hub integration is enabled' do
SiteSetting.stubs(:call_discourse_hub?).returns(true)
DiscourseHub.expects(:unregister_nickname).with(@user.username)
destroy
end
it 'should not try to unregister the nickname as the discourse hub if hub integration is disabled' do
SiteSetting.stubs(:call_discourse_hub?).returns(false)
DiscourseHub.expects(:unregister_nickname).never
destroy
end
it "should mark the user's deleted posts as belonging to a nuked user" do it "should mark the user's deleted posts as belonging to a nuked user" do
post = Fabricate(:post, user: @user, deleted_at: 1.hour.ago) post = Fabricate(:post, user: @user, deleted_at: 1.hour.ago)
@ -106,6 +125,8 @@ describe UserDestroyer do
end end
context 'and destroy fails' do context 'and destroy fails' do
subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) }
before do before do
@user.stubs(:destroy).returns(false) @user.stubs(:destroy).returns(false)
end end

View File

@ -218,6 +218,13 @@ describe Admin::UsersController do
response.should be_forbidden response.should be_forbidden
end end
it "doesn't return an error if the user has posts and delete_posts == true" do
Fabricate(:post, user: @delete_me)
UserDestroyer.any_instance.expects(:destroy).with(@delete_me, has_entry('delete_posts' => true)).returns(true)
xhr :delete, :destroy, id: @delete_me.id, delete_posts: true
response.should be_success
end
it "deletes the user record" do it "deletes the user record" do
UserDestroyer.any_instance.expects(:destroy).returns(true) UserDestroyer.any_instance.expects(:destroy).returns(true)
xhr :delete, :destroy, id: @delete_me.id xhr :delete, :destroy, id: @delete_me.id