FIX: delete all posts in batches without hijack (#6747)

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() {
+          callback: () => {
-            ajax("/admin/users/" + user.get("id") + "/delete_all_posts", {
+            openProgressModal();
-              type: "PUT"
+            performDelete();
-            }).then(() => user.set("post_count", 0));
           }
         }
-      ];
+      ],
+      openProgressModal = () => {
+        bootbox.dialog(
+          `<p>${I18n.t(
+            "admin.user.delete_posts_progress"
+          )}</p><div class='progress-bar'><span></span></div>`,
+          [],
+          { 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" });
   },
 

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;
 

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
+  def delete_posts_batch
-    hijack do
+    user = User.find_by(id: params[:user_id])
-      user = User.find_by(id: params[:user_id])
+    deleted_posts = user.delete_posts_in_batches(guardian)
-      user.delete_all_posts!(guardian)
+    # staff action logs will have an entry for each post
-      # staff action logs will have an entry for each post
+
-      render body: nil
+    render json: { posts_deleted: deleted_posts.length }
-    end
   end
 
   # DELETE action to delete penalty history for a user

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

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"

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

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
+    it 'deletes only one batch of posts' do
-      @user.delete_all_posts!(@guardian)
+      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|

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