From ed0e4582a1b227cbb663da89669484199b6fb396 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Fri, 19 Feb 2021 09:33:35 -0700 Subject: [PATCH] DEV: If disabled do not change setting after import (#12142) When running an import script there are many site settings that are changed but we reset them back to where they were originally before the import. However, there are two settings that we don't roll back: ``` purge_unactivated_users_grace_period_days purge_deleted_uploads_grace_period_days ``` which could have some unintended consequences. My first question is do we *really* have to change these settings? I'm not a huge fan of changing someones settings without them really knowing they were changed. If we really do have to change these settings here is my proposed PR where we don't alter the `purge_unactivated_users_grace_period_days` if it has been disabled. As I'm writing this another change we could make is that we don't change either of these site settings if we detect that they aren't set to the default values. The drive behind this PR is that there is a discourse instance which relies on staged users as part of their workflow and this setting was changed by accident via the import script causing users to be deleted that shouldn't have been. --- script/import_scripts/base.rb | 4 +++- spec/script/import_scripts/base_spec.rb | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/script/import_scripts/base.rb b/script/import_scripts/base.rb index aaef4efca4..7bb49bcde9 100644 --- a/script/import_scripts/base.rb +++ b/script/import_scripts/base.rb @@ -104,7 +104,9 @@ class ImportScripts::Base end # Some changes that should not be rolled back after the script is done - SiteSetting.purge_unactivated_users_grace_period_days = 60 + if SiteSetting.purge_unactivated_users_grace_period_days > 0 + SiteSetting.purge_unactivated_users_grace_period_days = 60 + end SiteSetting.purge_deleted_uploads_grace_period_days = 90 RateLimiter.disable diff --git a/spec/script/import_scripts/base_spec.rb b/spec/script/import_scripts/base_spec.rb index e7594d2fa5..a3b1199bb4 100644 --- a/spec/script/import_scripts/base_spec.rb +++ b/spec/script/import_scripts/base_spec.rb @@ -53,5 +53,12 @@ describe ImportScripts::Base do expect(Bookmark.count).to eq(5) expect(Post.count).to eq(5) expect(User.where('id > 0').count).to eq(1) + expect(SiteSetting.purge_unactivated_users_grace_period_days).to eq(60) + end + + it "does not change purge unactivated users setting if disabled" do + SiteSetting.purge_unactivated_users_grace_period_days = 0 + MockSpecImporter.new(import_data).perform + expect(SiteSetting.purge_unactivated_users_grace_period_days).to eq(0) end end