Fix `UserCleanupScheduler` crash when an unconfirmed account has a moderation note (#23318)

* Fix `UserCleanupScheduler` crash when an unconfirmed account has a moderation note

* Add tests

2 files changed, 40 insertions, 1 deletions

diff --git a/app/workers/scheduler/user_cleanup_scheduler.rb b/app/workers/scheduler/user_cleanup_scheduler.rb
index 63f9ed78c..45cfbc62e 100644
--- a/app/workers/scheduler/user_cleanup_scheduler.rb
+++ b/app/workers/scheduler/user_cleanup_scheduler.rb
@@ -15,7 +15,7 @@ class Scheduler::UserCleanupScheduler
   def clean_unconfirmed_accounts!
     User.where('confirmed_at is NULL AND confirmation_sent_at <= ?', 2.days.ago).reorder(nil).find_in_batches do |batch|
       # We have to do it separately because of missing database constraints
-      AccountModerationNote.where(account_id: batch.map(&:account_id)).delete_all
+      AccountModerationNote.where(target_account_id: batch.map(&:account_id)).delete_all
       Account.where(id: batch.map(&:account_id)).delete_all
       User.where(id: batch.map(&:id)).delete_all
     end
diff --git a/spec/workers/scheduler/user_cleanup_scheduler_spec.rb b/spec/workers/scheduler/user_cleanup_scheduler_spec.rb
new file mode 100644
index 000000000..da99f10f9
--- /dev/null
+++ b/spec/workers/scheduler/user_cleanup_scheduler_spec.rb
@@ -0,0 +1,39 @@
+require 'rails_helper'
+
+describe Scheduler::UserCleanupScheduler do
+  subject { described_class.new }
+
+  let!(:new_unconfirmed_user) { Fabricate(:user) }
+  let!(:old_unconfirmed_user) { Fabricate(:user) }
+  let!(:confirmed_user)       { Fabricate(:user) }
+  let!(:moderation_note)      { Fabricate(:account_moderation_note, account: Fabricate(:account), target_account: old_unconfirmed_user.account) }
+
+  describe '#perform' do
+    before do
+      # Need to update the already-existing users because their initialization overrides confirmation_sent_at
+      new_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: Time.now.utc)
+      old_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: 1.week.ago)
+      confirmed_user.update!(confirmed_at: 1.day.ago)
+    end
+
+    it 'deletes the old unconfirmed user' do
+      expect { subject.perform }.to change { User.exists?(old_unconfirmed_user.id) }.from(true).to(false)
+    end
+
+    it "deletes the old unconfirmed user's account" do
+      expect { subject.perform }.to change { Account.exists?(old_unconfirmed_user.account_id) }.from(true).to(false)
+    end
+
+    it 'does not delete the new unconfirmed user or their account' do
+      subject.perform
+      expect(User.exists?(new_unconfirmed_user.id)).to be true
+      expect(Account.exists?(new_unconfirmed_user.account_id)).to be true
+    end
+
+    it 'does not delete the confirmed user or their account' do
+      subject.perform
+      expect(User.exists?(confirmed_user.id)).to be true
+      expect(Account.exists?(confirmed_user.account_id)).to be true
+    end
+  end
+end