From 9f89aadd333567635a0eb41ab28ad588a8547260 Mon Sep 17 00:00:00 2001 From: Maja Komel Date: Fri, 14 Dec 2018 11:04:18 +0100 Subject: [PATCH] FIX: delete all posts in batches without hijack (#6747) --- .../admin/models/admin-user.js.es6 | 50 +++++++++++++++++-- app/assets/stylesheets/common/base/modal.scss | 19 +++++++ app/controllers/admin/users_controller.rb | 13 +++-- app/models/user.rb | 4 +- config/locales/client.en.yml | 2 + config/routes.rb | 2 +- spec/models/user_spec.rb | 15 ++++-- spec/requests/admin/users_controller_spec.rb | 28 +++++++++++ 8 files changed, 114 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6 index fc36b9bfe88..d98e148a5a4 100644 --- a/app/assets/javascripts/admin/models/admin-user.js.es6 +++ b/app/assets/javascripts/admin/models/admin-user.js.es6 @@ -98,6 +98,7 @@ const AdminUser = Discourse.User.extend({ }, deleteAllPosts() { + let deletedPosts = 0; const user = this, message = I18n.messageFormat("admin.user.delete_all_posts_confirm_MF", { POSTS: user.get("post_count"), @@ -114,13 +115,52 @@ const AdminUser = Discourse.User.extend({ `${iconHTML("exclamation-triangle")} ` + I18n.t("admin.user.delete_all_posts"), class: "btn btn-danger", - callback: function() { - ajax("/admin/users/" + user.get("id") + "/delete_all_posts", { - type: "PUT" - }).then(() => user.set("post_count", 0)); + callback: () => { + openProgressModal(); + performDelete(); } } - ]; + ], + openProgressModal = () => { + bootbox.dialog( + `

${I18n.t( + "admin.user.delete_posts_progress" + )}

`, + [], + { classes: "delete-posts-progress" } + ); + }, + performDelete = () => { + let deletedPercentage = 0; + return ajax(`/admin/users/${user.get("id")}/delete_posts_batch`, { + type: "PUT" + }) + .then(({ posts_deleted }) => { + if (posts_deleted === 0) { + user.set("post_count", 0); + bootbox.hideAll(); + } else { + deletedPosts += posts_deleted; + deletedPercentage = Math.floor( + (deletedPosts * 100) / user.get("post_count") + ); + $(".delete-posts-progress .progress-bar > span").css({ + width: `${deletedPercentage}%` + }); + performDelete(); + } + }) + .catch(e => { + bootbox.hideAll(); + let error; + AdminUser.find(user.get("id")).then(u => user.setProperties(u)); + if (e.responseJSON && e.responseJSON.errors) { + error = e.responseJSON.errors[0]; + } + error = error || I18n.t("admin.user.delete_posts_failed"); + bootbox.alert(error); + }); + }; bootbox.dialog(message, buttons, { classes: "delete-all-posts" }); }, diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index 1858c06ddcc..e9017d21121 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -334,6 +334,25 @@ } } +.delete-posts-progress { + .progress-bar { + height: 15px; + position: relative; + background: $primary-low-mid; + border-radius: 25px; + overflow: hidden; + margin: 30px 0 20px; + span { + display: block; + width: 0%; + height: 100%; + background-color: $tertiary; + position: relative; + transition: width 0.6s linear; + } + } +} + #invite-modal { overflow: visible; diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3d4ca1eda45..ae0ed9de28b 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -45,13 +45,12 @@ class Admin::UsersController < Admin::AdminController render_serialized(@user, AdminDetailedUserSerializer, root: false) end - def delete_all_posts - hijack do - user = User.find_by(id: params[:user_id]) - user.delete_all_posts!(guardian) - # staff action logs will have an entry for each post - render body: nil - end + def delete_posts_batch + user = User.find_by(id: params[:user_id]) + deleted_posts = user.delete_posts_in_batches(guardian) + # staff action logs will have an entry for each post + + render json: { posts_deleted: deleted_posts.length } end # DELETE action to delete penalty history for a user diff --git a/app/models/user.rb b/app/models/user.rb index dea7a5f2916..8ce55b3e06b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -779,12 +779,12 @@ class User < ActiveRecord::Base (since_reply.count >= SiteSetting.newuser_max_replies_per_topic) end - def delete_all_posts!(guardian) + def delete_posts_in_batches(guardian, batch_size = 20) raise Discourse::InvalidAccess unless guardian.can_delete_all_posts? self QueuedPost.where(user_id: id).delete_all - posts.order("post_number desc").each do |p| + posts.order("post_number desc").limit(batch_size).each do |p| PostDestroyer.new(guardian.user, p).destroy end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e9db885c898..33d9dbd4d3f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3741,6 +3741,8 @@ en: suspended_until: "(until %{until})" cant_suspend: "This user cannot be suspended." delete_all_posts: "Delete all posts" + delete_posts_progress: "Deleting posts..." + delete_posts_failed: "There was a problem deleting the posts." penalty_post_actions: "What would you like to do with the associated post?" penalty_post_delete: "Delete the post" penalty_post_edit: "Edit the post" diff --git a/config/routes.rb b/config/routes.rb index 96cee5324de..b16f3330882 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,7 +107,7 @@ Discourse::Application.routes.draw do end delete "penalty_history", constraints: AdminConstraint.new put "suspend" - put "delete_all_posts" + put "delete_posts_batch" put "unsuspend" put "revoke_admin", constraints: AdminConstraint.new put "grant_admin", constraints: AdminConstraint.new diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0c1a35d6de2..37452a3620f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -194,7 +194,7 @@ describe User do end end - describe 'delete posts' do + describe 'delete posts in batches' do before do @post1 = Fabricate(:post) @user = @post1.user @@ -205,8 +205,15 @@ describe User do @queued_post = Fabricate(:queued_post, user: @user) end - it 'allows moderator to delete all posts' do - @user.delete_all_posts!(@guardian) + it 'deletes only one batch of posts' do + deleted_posts = @user.delete_posts_in_batches(@guardian, 1) + expect(Post.where(id: @posts.map(&:id)).count).to eq(2) + expect(deleted_posts.length).to eq(1) + expect(deleted_posts[0]).to eq(@post2) + end + + it 'correctly deletes posts and topics' do + @user.delete_posts_in_batches(@guardian, 20) expect(Post.where(id: @posts.map(&:id))).to be_empty expect(QueuedPost.where(user_id: @user.id).count).to eq(0) @posts.each do |p| @@ -220,7 +227,7 @@ describe User do invalid_guardian = Guardian.new(Fabricate(:user)) expect do - @user.delete_all_posts!(invalid_guardian) + @user.delete_posts_in_batches(invalid_guardian) end.to raise_error Discourse::InvalidAccess @posts.each do |p| diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 60beacbd727..6671cdd0393 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -942,4 +942,32 @@ RSpec.describe Admin::UsersController do end + describe "#delete_posts_batch" do + context "when there are user posts" do + before do + post = Fabricate(:post, user: user) + Fabricate(:post, topic: post.topic, user: user) + Fabricate(:post, user: user) + end + + it 'returns how many posts were deleted' do + sign_in(admin) + + put "/admin/users/#{user.id}/delete_posts_batch.json" + expect(response.status).to eq(200) + expect(JSON.parse(response.body)["posts_deleted"]).to eq(3) + end + end + + context "when there are no posts left to be deleted" do + it "returns correct json" do + sign_in(admin) + + put "/admin/users/#{user.id}/delete_posts_batch.json" + expect(response.status).to eq(200) + expect(JSON.parse(response.body)["posts_deleted"]).to eq(0) + end + end + end + end