FIX: resolve flags as good when deleting a spam user

This commit is contained in:
Régis Hanol 2014-10-20 16:59:06 +02:00
parent 4ef9e6e95a
commit 10094a0bcd
6 changed files with 43 additions and 8 deletions

View file

@ -391,6 +391,7 @@ Discourse.AdminUser = Discourse.User.extend({
block_email: true, block_email: true,
block_urls: true, block_urls: true,
block_ip: true, block_ip: true,
delete_as_spammer: true,
context: window.location.pathname context: window.location.pathname
} }
}).then(function(result) { }).then(function(result) {

View file

@ -211,22 +211,31 @@ class Admin::UsersController < Admin::AdminController
end end
def reject_bulk def reject_bulk
d = UserDestroyer.new(current_user)
success_count = 0 success_count = 0
d = UserDestroyer.new(current_user)
User.where(id: params[:users]).each do |u| User.where(id: params[:users]).each do |u|
success_count += 1 if guardian.can_delete_user?(u) and d.destroy(u, params.slice(:context)) rescue UserDestroyer::PostsExistError success_count += 1 if guardian.can_delete_user?(u) and d.destroy(u, params.slice(:context)) rescue UserDestroyer::PostsExistError
end end
render json: {success: success_count, failed: (params[:users].try(:size) || 0) - success_count}
render json: {
success: success_count,
failed: (params[:users].try(:size) || 0) - success_count
}
end end
def destroy def destroy
user = User.find_by(id: params[:id].to_i) user = User.find_by(id: params[:id].to_i)
guardian.ensure_can_delete_user!(user) guardian.ensure_can_delete_user!(user)
begin begin
if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :block_urls, :block_ip, :context)) options = params.slice(:delete_posts, :block_email, :block_urls, :block_ip, :context, :delete_as_spammer)
render json: {deleted: true} if UserDestroyer.new(current_user).destroy(user, options)
render json: { deleted: true }
else else
render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json} render json: {
deleted: false,
user: AdminDetailedUserSerializer.new(user, root: false).as_json
}
end end
rescue UserDestroyer::PostsExistError rescue UserDestroyer::PostsExistError
raise Discourse::InvalidAccess.new("User #{user.username} has #{user.post_count} posts, so can't be deleted.") raise Discourse::InvalidAccess.new("User #{user.username} has #{user.post_count} posts, so can't be deleted.")

View file

@ -505,7 +505,7 @@ class UsersController < ApplicationController
@user = fetch_user_from_params @user = fetch_user_from_params
guardian.ensure_can_delete_user!(@user) guardian.ensure_can_delete_user!(@user)
UserDestroyer.new(current_user).destroy(@user, {delete_posts: true, context: params[:context]}) UserDestroyer.new(current_user).destroy(@user, { delete_posts: true, context: params[:context] })
render json: success_json render json: success_json
end end

View file

@ -15,9 +15,14 @@ class UserDestroyer
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)
@guardian.ensure_can_delete_user!(user) @guardian.ensure_can_delete_user!(user)
raise PostsExistError if !opts[:delete_posts] && user.posts.count != 0 raise PostsExistError if !opts[:delete_posts] && user.posts.count != 0
User.transaction do User.transaction do
if opts[:delete_posts] if opts[:delete_posts]
user.posts.each do |post| user.posts.each do |post|
# agree with flags
PostAction.agree_flags!(post, @actor) if opts[:delete_as_spammer]
# block all external urls
if opts[:block_urls] if opts[:block_urls]
post.topic_links.each do |link| post.topic_links.each do |link|
unless link.internal or Oneboxer.oneboxer_exists_for_url?(link.url) unless link.internal or Oneboxer.oneboxer_exists_for_url?(link.url)
@ -25,15 +30,19 @@ class UserDestroyer
end end
end end
end end
PostDestroyer.new(@actor.staff? ? @actor : Discourse.system_user, post).destroy PostDestroyer.new(@actor.staff? ? @actor : Discourse.system_user, post).destroy
if post.topic and post.post_number == 1 if post.topic and post.post_number == 1
Topic.unscoped.where(id: post.topic.id).update_all(user_id: nil) Topic.unscoped.where(id: post.topic.id).update_all(user_id: nil)
end end
end end
end end
user.post_actions.each do |post_action| user.post_actions.each do |post_action|
post_action.remove_act!(Discourse.system_user) post_action.remove_act!(Discourse.system_user)
end end
user.destroy.tap do |u| user.destroy.tap do |u|
if u if u
if opts[:block_email] if opts[:block_email]

View file

@ -12,7 +12,7 @@ module FlagQuery
post_ids = actions.limit(per_page) post_ids = actions.limit(per_page)
.offset(offset) .offset(offset)
.group(:post_id) .group(:post_id)
.order('min(post_actions.created_at) DESC') .order('MIN(post_actions.created_at) DESC')
.pluck(:post_id) .pluck(:post_id)
.uniq .uniq
@ -114,7 +114,6 @@ module FlagQuery
.joins("INNER JOIN posts ON posts.id = post_actions.post_id") .joins("INNER JOIN posts ON posts.id = post_actions.post_id")
.joins("INNER JOIN topics ON topics.id = posts.topic_id") .joins("INNER JOIN topics ON topics.id = posts.topic_id")
.joins("LEFT JOIN users ON users.id = posts.user_id") .joins("LEFT JOIN users ON users.id = posts.user_id")
.where("users.id IS NOT NULL")
if filter == "old" if filter == "old"
post_actions.where("post_actions.disagreed_at IS NOT NULL OR post_actions.where("post_actions.disagreed_at IS NOT NULL OR

View file

@ -128,6 +128,23 @@ describe UserDestroyer do
spammer_topic.reload.deleted_at.should_not == nil spammer_topic.reload.deleted_at.should_not == nil
spammer_topic.user_id.should == nil spammer_topic.user_id.should == nil
end end
context "delete_as_spammer is true" do
before { destroy_opts[:delete_as_spammer] = true }
it "agrees with flags on user's posts" do
spammer_post = Fabricate(:post, user: @user)
flag = PostAction.act(@admin, spammer_post, PostActionType.types[:inappropriate])
flag.agreed_at.should == nil
destroy
flag.reload
flag.agreed_at.should_not == nil
end
end
end end
context "users deletes self" do context "users deletes self" do