From 9edefc779f8e5a5bdedd5a27f6fd815105dd3e92 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 7 Feb 2023 01:14:44 +0100 Subject: [PATCH] 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 --- .../scheduler/user_cleanup_scheduler.rb | 2 +- .../scheduler/user_cleanup_scheduler_spec.rb | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 spec/workers/scheduler/user_cleanup_scheduler_spec.rb 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