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
pull/53/head
Claire 2023-02-07 01:14:44 +01:00 committed by GitHub
parent 523a86618f
commit 9edefc779f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 40 additions and 1 deletions

View File

@ -15,7 +15,7 @@ class Scheduler::UserCleanupScheduler
def clean_unconfirmed_accounts! def clean_unconfirmed_accounts!
User.where('confirmed_at is NULL AND confirmation_sent_at <= ?', 2.days.ago).reorder(nil).find_in_batches do |batch| 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 # 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 Account.where(id: batch.map(&:account_id)).delete_all
User.where(id: batch.map(&:id)).delete_all User.where(id: batch.map(&:id)).delete_all
end end

View File

@ -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