FIX: don't purge unactivated users with a message

This commit is contained in:
Régis Hanol 2018-05-16 18:24:11 +02:00
parent 938934f5e9
commit 39aceed63d
3 changed files with 25 additions and 21 deletions

View File

@ -1,5 +1,5 @@
module Jobs module Jobs
class PurgeInactive < Jobs::Scheduled class PurgeUnactived < Jobs::Scheduled
every 1.day every 1.day
def execute(args) def execute(args)

View File

@ -1198,22 +1198,22 @@ class User < ActiveRecord::Base
end end
end end
# Delete unactivated accounts (without verified email) that are over a week old
def self.purge_unactivated def self.purge_unactivated
return [] if SiteSetting.purge_unactivated_users_grace_period_days <= 0 return [] if SiteSetting.purge_unactivated_users_grace_period_days <= 0
to_destroy = User.where(active: false)
.joins('INNER JOIN user_stats AS us ON us.user_id = users.id')
.where("created_at < ?", SiteSetting.purge_unactivated_users_grace_period_days.days.ago)
.where('NOT admin AND NOT moderator')
.limit(200)
destroyer = UserDestroyer.new(Discourse.system_user) destroyer = UserDestroyer.new(Discourse.system_user)
to_destroy.each do |u|
User
.where(active: false)
.where("created_at < ?", SiteSetting.purge_unactivated_users_grace_period_days.days.ago)
.where("NOT admin AND NOT moderator")
.where("NOT EXISTS (SELECT 1 FROM topic_allowed_users WHERE user_id = users.id LIMIT 1)")
.limit(200)
.find_each do |user|
begin begin
destroyer.destroy(u, context: I18n.t(:purge_reason)) destroyer.destroy(user, context: I18n.t(:purge_reason))
rescue Discourse::InvalidAccess, UserDestroyer::PostsExistError rescue Discourse::InvalidAccess, UserDestroyer::PostsExistError
# if for some reason the user can't be deleted, continue on to the next one # keep going
end end
end end
end end

View File

@ -1236,24 +1236,28 @@ describe User do
describe "#purge_unactivated" do describe "#purge_unactivated" do
let!(:user) { Fabricate(:user) } let!(:user) { Fabricate(:user) }
let!(:inactive) { Fabricate(:user, active: false) } let!(:unactivated) { Fabricate(:user, active: false) }
let!(:inactive_old) { Fabricate(:user, active: false, created_at: 1.month.ago) } let!(:unactivated_old) { Fabricate(:user, active: false, created_at: 1.month.ago) }
let!(:unactivated_old_with_pm) { Fabricate(:user, active: false, created_at: 2.months.ago) }
before do
PostCreator.new(Discourse.system_user,
title: "Welcome to our Discourse",
raw: "This is a welcome message",
archetype: Archetype.private_message,
target_usernames: [unactivated_old_with_pm.username],
).create
end
it 'should only remove old, unactivated users' do it 'should only remove old, unactivated users' do
User.purge_unactivated User.purge_unactivated
all_users = User.all expect(User.real.all).to match_array([user, unactivated, unactivated_old_with_pm])
expect(all_users.include?(user)).to eq(true)
expect(all_users.include?(inactive)).to eq(true)
expect(all_users.include?(inactive_old)).to eq(false)
end end
it "does nothing if purge_unactivated_users_grace_period_days is 0" do it "does nothing if purge_unactivated_users_grace_period_days is 0" do
SiteSetting.purge_unactivated_users_grace_period_days = 0 SiteSetting.purge_unactivated_users_grace_period_days = 0
User.purge_unactivated User.purge_unactivated
all_users = User.all expect(User.real.all).to match_array([user, unactivated, unactivated_old, unactivated_old_with_pm])
expect(all_users.include?(user)).to eq(true)
expect(all_users.include?(inactive)).to eq(true)
expect(all_users.include?(inactive_old)).to eq(true)
end end
end end