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.
This commit is contained in:
Blake Erickson 2021-02-19 09:33:35 -07:00 committed by GitHub
parent 5604ce70d4
commit ed0e4582a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 1 deletions

View File

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

View File

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