FIX: Use a valid value for disabling backups using backup_frequency (#34245)

Drenmi · web-flow · commit 71ea236a7963 · 2025-08-12T13:19:56.000+08:00
In #33558 we removed automatic_backups_enabled setting, and instead rely on backup_frequency being blank to disable. There was a big oversight there with the site setting type system, which will coerce the value to an integer. It also makes it so you can't blank the value out in the UI. This is a "fix forward" solution where instead of "set to blank to disable" we do "set to 0 to disable". This works along the grain of the site setting type system for a workable fix where we don't have to deal with the irreversible migration in the previous change. We can potentially go and add in "nullable" to the type system at a later point.
diff --git a/app/jobs/scheduled/schedule_backup.rb b/app/jobs/scheduled/schedule_backup.rb
@@ -8,7 +8,7 @@ class ScheduleBackup < ::Jobs::Scheduled
     def execute(args)
       delete_prior_to_n_days
       return if !SiteSetting.enable_backups?
-      return if SiteSetting.backup_frequency.blank?
+      return if SiteSetting.backup_frequency.zero?
 
       store = BackupRestore::BackupStore.create
       if latest_backup = store.latest_file
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
@@ -2026,7 +2026,7 @@ en:
     allow_restore: "Allow restore, which can replace ALL site data! Leave disabled unless you plan to restore a backup"
     maximum_backups: "The maximum amount of backups to keep. Older backups are automatically deleted"
     remove_older_backups: "Remove backups older than the specified number of days. Leave blank to disable."
-    backup_frequency: "Specifies the interval, in days, at which automatic backups of the site are created. If set to 7, for example, a new backup will be generated every week. Leave blank to disable."
+    backup_frequency: "Specifies the interval, in days, at which automatic backups of the site are created. If set to 7, for example, a new backup will be generated every week. Set to 0 to disable."
     s3_backup_bucket: "The remote bucket to hold backups. WARNING: Make sure it is a private bucket."
     s3_endpoint: "The endpoint can be modified to backup to an S3 compatible service like DigitalOcean Spaces or Minio. WARNING: Leave blank if using AWS S3."
     s3_configure_tombstone_policy: "Enable automatic deletion policy for tombstone uploads. IMPORTANT: If disabled, no space will be reclaimed after uploads are deleted."
diff --git a/config/site_settings.yml b/config/site_settings.yml
@@ -3126,7 +3126,7 @@ backups:
     client: true
     default: ""
   backup_frequency:
-    min: 1
+    min: 0
     max: 30
     default: 7
   s3_backup_bucket:
diff --git a/db/post_migrate/20250812033430_replace_null_with_zero_in_backup_frequency.rb b/db/post_migrate/20250812033430_replace_null_with_zero_in_backup_frequency.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+class ReplaceNullWithZeroInBackupFrequency < ActiveRecord::Migration[8.0]
+  def change
+    execute <<~SQL
+      UPDATE site_settings
+      SET value = 0
+      WHERE name = 'backup_frequency'
+      AND value IS NULL
+    SQL
+  end
+end
diff --git a/script/bulk_import/vanilla.rb b/script/bulk_import/vanilla.rb
@@ -63,7 +63,7 @@ def execute
 
     # SiteSetting.port = 3000
     # SiteSetting.permalink_normalizations = "/discussion\/(\d+)\/.*/discussion/\1"
-    # SiteSetting.backup_frequency = nil
+    # SiteSetting.backup_frequency = 0
     # SiteSetting.disable_emails = "non-staff"
     # SiteSetting.authorized_extensions = '*'
     # SiteSetting.max_image_size_kb = 102400
diff --git a/script/bulk_import/vbulletin.rb b/script/bulk_import/vbulletin.rb
@@ -48,7 +48,7 @@ def initialize
 
   def execute
     # enable as per requirement:
-    # SiteSetting.backup_frequency = nil
+    # SiteSetting.backup_frequency = 0
     # SiteSetting.disable_emails = "non-staff"
     # SiteSetting.authorized_extensions = '*'
     # SiteSetting.max_image_size_kb = 102400
diff --git a/script/bulk_import/vbulletin5.rb b/script/bulk_import/vbulletin5.rb
@@ -58,7 +58,7 @@ def initialize
 
   def execute
     # enable as per requirement:
-    #SiteSetting.backup_frequency = nil
+    #SiteSetting.backup_frequency = 0
     #SiteSetting.disable_emails = "non-staff"
     #SiteSetting.authorized_extensions = '*'
     #SiteSetting.max_image_size_kb = 102400
diff --git a/spec/lib/backup_restore/shared_examples_for_backup_store.rb b/spec/lib/backup_restore/shared_examples_for_backup_store.rb
@@ -185,6 +185,22 @@
         scheduleBackup.expects(:delete_prior_to_n_days)
         scheduleBackup.perform
       end
+
+      it "doesn't run if SiteSetting.backup_frequency is set to 0" do
+        base_backup_s3_url = "https://s3-backup-bucket.s3.dualstack.us-west-1.amazonaws.com"
+        stub_request(:get, "#{base_backup_s3_url}/?list-type=2&prefix=default/").to_return(
+          status: 200,
+          body: "",
+          headers: {
+          },
+        )
+        stub_request(:head, "#{base_backup_s3_url}/").to_return(status: 200, body: "", headers: {})
+
+        SiteSetting.backup_frequency = 0
+        scheduleBackup = Jobs::ScheduleBackup.new
+        scheduleBackup.expects(:delete_prior_to_n_days)
+        scheduleBackup.perform
+      end
     end
 
     describe "#file" do
