FIX: Remove site settings override for deprecated url site settings

FIX: Remove site settings override for deprecated url site settings.

From 81b3bdaabde1599a6979e331e923489884073a58 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Tue, 20 Nov 2018 11:42:39 +0800
Subject: [PATCH] FIX: Remove site settings override for deprecated url site
 settings.


diff --git a/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb b/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb
index 0b23903..fd12fcb 100644
--- a/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb
+++ b/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb
@@ -6,6 +6,7 @@ module Jobs
       Jobs::MigrateUrlSiteSettings::SETTINGS.each do |old_setting, new_setting|
         if SiteSetting.where("name = ? AND value IS NOT NULL", new_setting).exists?
           SiteSetting.public_send("#{old_setting}=", nil, warn: false)
+          SiteSetting.find_by(name: old_setting).destroy!
         end
       end
     end
diff --git a/spec/jobs/clean_up_deprecated_url_site_settings_spec.rb b/spec/jobs/clean_up_deprecated_url_site_settings_spec.rb
index f21591d..677e9c5 100644
--- a/spec/jobs/clean_up_deprecated_url_site_settings_spec.rb
+++ b/spec/jobs/clean_up_deprecated_url_site_settings_spec.rb
@@ -21,6 +21,7 @@ RSpec.describe Jobs::CleanUpDeprecatedUrlSiteSettings do
       described_class.new.execute({})
     end.to change { SiteSetting.logo_url }.from("/test/some/url").to("")
 
+    expect(SiteSetting.exists?(name: "logo_url")).to eq(false)
     expect(SiteSetting.logo).to eq(logo_upload)
     expect(SiteSetting.logo_small_url).to eq('/test/another/url')
   end

GitHub

1 Like

Interesting, what broke when both existed?

When we set SiteSetting.logo_url = nil, it doesn’t actually remove the override but adds an override with a value of "". As a result, the site setting doesn’t fall back to the default value which is what we want.

2 Likes