From 67b23c0e22ed851ec7924709b53e416356980682 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Wed, 11 May 2022 13:39:31 +0100
Subject: [PATCH] PERF: Update all user_histories with one query in
 UserDestroyer (#16717)

7a284164 previously switched the UserDestroyer to use find_each when iterating over UserHistory records. Unfortunately, since this logic is wrapped in a transaction, this didn't actually solve the memory usage problem. ActiveRecord maintains references to all modified models within a transaction.

This commit updates the logic to use a single SQL query, rather than updating models one-by-one
---
 app/services/user_destroyer.rb       | 11 ++++-------
 spec/services/user_destroyer_spec.rb | 19 ++++++++++++++-----
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/app/services/user_destroyer.rb b/app/services/user_destroyer.rb
index b191cbe3dcb..408dffcb9ef 100644
--- a/app/services/user_destroyer.rb
+++ b/app/services/user_destroyer.rb
@@ -47,13 +47,10 @@ class UserDestroyer
       # Add info about the user to staff action logs
       UserHistory.staff_action_records(
         Discourse.system_user, acting_user: user.username
-      ).unscope(:order).find_each do |log|
-        log.details ||= ''
-        log.details = (log.details.split("\n") +
-            ["user_id: #{user.id}", "username: #{user.username}"]
-          ).join("\n")
-        log.save!
-      end
+      ).update_all([
+        "details = CONCAT(details, ?)",
+        "\nuser_id: #{user.id}\nusername: #{user.username}"
+      ])
 
       # keep track of emails used
       user_emails = user.user_emails.pluck(:email)
diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb
index dcd6d3c5b75..51d3254bc88 100644
--- a/spec/services/user_destroyer_spec.rb
+++ b/spec/services/user_destroyer_spec.rb
@@ -442,19 +442,28 @@ describe UserDestroyer do
         logger.log_site_setting_change(
           'site_description',
           'Our friendly community',
-          'My favourite community'
+          'My favourite community',
+        )
+        logger.log_site_setting_change(
+          'site_description',
+          'Our friendly community',
+          'My favourite community',
+          details: "existing details"
         )
       end
 
       it "should keep the staff action log and add the username" do
         username = user.username
-        log = UserHistory.staff_action_records(
+        ids = UserHistory.staff_action_records(
           Discourse.system_user,
           acting_user: username
-        ).to_a[0]
+        ).map(&:id)
         UserDestroyer.new(admin).destroy(user, delete_posts: true)
-        log.reload
-        expect(log.details).to include(username)
+        details = UserHistory.where(id: ids).map(&:details)
+        expect(details).to contain_exactly(
+          "\nuser_id: #{user.id}\nusername: #{username}",
+          "existing details\nuser_id: #{user.id}\nusername: #{username}"
+        )
       end
     end