FIX: consider users.created_at for inactive cleanup (#21688)

This commit is contained in:
Faizaan Gagan 2023-05-23 13:41:23 +05:30 committed by GitHub
parent 54db01d156
commit d1334a7aaf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 49 additions and 8 deletions

View File

@ -14,6 +14,7 @@ module Jobs
admin: false, admin: false,
moderator: false, moderator: false,
) )
.where("users.created_at < ?", SiteSetting.clean_up_inactive_users_after_days.days.ago)
.where( .where(
"users.last_seen_at < ? OR users.last_seen_at IS NULL", "users.last_seen_at < ? OR users.last_seen_at IS NULL",
SiteSetting.clean_up_inactive_users_after_days.days.ago, SiteSetting.clean_up_inactive_users_after_days.days.ago,

View File

@ -4,18 +4,35 @@ RSpec.describe Jobs::CleanUpInactiveUsers do
it "should clean up new users that have been inactive" do it "should clean up new users that have been inactive" do
SiteSetting.clean_up_inactive_users_after_days = 0 SiteSetting.clean_up_inactive_users_after_days = 0
user = Fabricate(:user, last_seen_at: 5.days.ago, trust_level: TrustLevel.levels[:newuser]) user =
Fabricate(
:user,
created_at: 5.days.ago,
last_seen_at: 5.days.ago,
trust_level: TrustLevel.levels[:newuser],
)
Fabricate(:active_user) Fabricate(:active_user)
Fabricate( Fabricate(
:post, :post,
user: Fabricate(:user, trust_level: TrustLevel.levels[:newuser], last_seen_at: 5.days.ago), user:
Fabricate(
:user,
trust_level: TrustLevel.levels[:newuser],
created_at: 5.days.ago,
last_seen_at: 5.days.ago,
),
).user ).user
Fabricate(:user, trust_level: TrustLevel.levels[:newuser], last_seen_at: 2.days.ago) Fabricate(
:user,
trust_level: TrustLevel.levels[:newuser],
created_at: 5.days.ago,
last_seen_at: 2.days.ago,
)
Fabricate(:user, trust_level: TrustLevel.levels[:basic]) Fabricate(:user, trust_level: TrustLevel.levels[:basic], created_at: 5.days.ago)
expect { described_class.new.execute({}) }.to_not change { User.count } expect { described_class.new.execute({}) }.to_not change { User.count }
@ -28,7 +45,13 @@ RSpec.describe Jobs::CleanUpInactiveUsers do
it "doesn't delete inactive admins" do it "doesn't delete inactive admins" do
SiteSetting.clean_up_inactive_users_after_days = 4 SiteSetting.clean_up_inactive_users_after_days = 4
admin = Fabricate(:admin, last_seen_at: 5.days.ago, trust_level: TrustLevel.levels[:newuser]) admin =
Fabricate(
:admin,
created_at: 5.days.ago,
last_seen_at: 5.days.ago,
trust_level: TrustLevel.levels[:newuser],
)
expect { described_class.new.execute({}) }.to_not change { User.count } expect { described_class.new.execute({}) }.to_not change { User.count }
expect(User.exists?(admin.id)).to eq(true) expect(User.exists?(admin.id)).to eq(true)
@ -37,7 +60,12 @@ RSpec.describe Jobs::CleanUpInactiveUsers do
it "doesn't delete inactive mods" do it "doesn't delete inactive mods" do
SiteSetting.clean_up_inactive_users_after_days = 4 SiteSetting.clean_up_inactive_users_after_days = 4
moderator = moderator =
Fabricate(:moderator, last_seen_at: 5.days.ago, trust_level: TrustLevel.levels[:newuser]) Fabricate(
:moderator,
created_at: 5.days.ago,
last_seen_at: 5.days.ago,
trust_level: TrustLevel.levels[:newuser],
)
expect { described_class.new.execute({}) }.to_not change { User.count } expect { described_class.new.execute({}) }.to_not change { User.count }
expect(User.exists?(moderator.id)).to eq(true) expect(User.exists?(moderator.id)).to eq(true)
@ -50,7 +78,13 @@ RSpec.describe Jobs::CleanUpInactiveUsers do
Fabricate( Fabricate(
:post, :post,
user: Fabricate(:user, trust_level: TrustLevel.levels[:newuser], last_seen_at: 5.days.ago), user:
Fabricate(
:user,
trust_level: TrustLevel.levels[:newuser],
created_at: 5.days.ago,
last_seen_at: 2.days.ago,
),
# ensuring that topic author is a different user as the topic is non-deleted # ensuring that topic author is a different user as the topic is non-deleted
topic: Fabricate(:topic, user: Fabricate(:user)), topic: Fabricate(:topic, user: Fabricate(:user)),
deleted_at: Time.now, deleted_at: Time.now,
@ -66,7 +100,13 @@ RSpec.describe Jobs::CleanUpInactiveUsers do
Fabricate( Fabricate(
:topic, :topic,
user: Fabricate(:user, trust_level: TrustLevel.levels[:newuser], last_seen_at: 5.days.ago), user:
Fabricate(
:user,
trust_level: TrustLevel.levels[:newuser],
created_at: 2.days.ago,
last_seen_at: 2.days.ago,
),
deleted_at: Time.now, deleted_at: Time.now,
).user ).user