FIX: consider users with trashed topics/posts for inactive cleanup (#21678)
* FIX: consider users with trashed topics/posts for inactive cleanup * defer checking for missing associations
This commit is contained in:
parent
c3a3ede8cf
commit
efdfddf7fc
|
@ -8,7 +8,6 @@ module Jobs
|
|||
return if SiteSetting.clean_up_inactive_users_after_days <= 0
|
||||
|
||||
User
|
||||
.joins("LEFT JOIN posts ON posts.user_id = users.id")
|
||||
.where(
|
||||
last_posted_at: nil,
|
||||
trust_level: TrustLevel.levels[:newuser],
|
||||
|
@ -16,9 +15,11 @@ module Jobs
|
|||
moderator: false,
|
||||
)
|
||||
.where(
|
||||
"posts.user_id IS NULL AND users.last_seen_at < ?",
|
||||
"users.last_seen_at < ? OR users.last_seen_at IS NULL",
|
||||
SiteSetting.clean_up_inactive_users_after_days.days.ago,
|
||||
)
|
||||
.where
|
||||
.missing(:posts, :topics)
|
||||
.limit(1000)
|
||||
.pluck(:id)
|
||||
.each_slice(50) { |slice| destroy(slice) }
|
||||
|
|
|
@ -42,4 +42,34 @@ RSpec.describe Jobs::CleanUpInactiveUsers do
|
|||
expect { described_class.new.execute({}) }.to_not change { User.count }
|
||||
expect(User.exists?(moderator.id)).to eq(true)
|
||||
end
|
||||
|
||||
it "should clean up a user that has a deleted post" do
|
||||
SiteSetting.clean_up_inactive_users_after_days = 1
|
||||
|
||||
Fabricate(:active_user)
|
||||
|
||||
Fabricate(
|
||||
:post,
|
||||
user: Fabricate(:user, trust_level: TrustLevel.levels[:newuser], last_seen_at: 5.days.ago),
|
||||
# ensuring that topic author is a different user as the topic is non-deleted
|
||||
topic: Fabricate(:topic, user: Fabricate(:user)),
|
||||
deleted_at: Time.now,
|
||||
).user
|
||||
|
||||
expect { described_class.new.execute({}) }.to change { User.count }.by(-1)
|
||||
end
|
||||
|
||||
it "should clean up user that has a deleted topic" do
|
||||
SiteSetting.clean_up_inactive_users_after_days = 1
|
||||
|
||||
Fabricate(:active_user)
|
||||
|
||||
Fabricate(
|
||||
:topic,
|
||||
user: Fabricate(:user, trust_level: TrustLevel.levels[:newuser], last_seen_at: 5.days.ago),
|
||||
deleted_at: Time.now,
|
||||
).user
|
||||
|
||||
expect { described_class.new.execute({}) }.to change { User.count }.by(-1)
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue