From 9e86b425bcca08a2f0d096d0bdb9b9e17a46e316 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Fri, 16 Nov 2018 09:19:58 +0800
Subject: [PATCH] FIX: Job to clean up old URL settings when new setting has
 been set.

Related to https://github.com/discourse/discourse/commit/44391ee8ab7431ca572b6c340d6e770cdadd51b3
---
 app/jobs/onceoff/migrate_url_site_settings.rb | 26 +++++++++---------
 .../clean_up_deprecated_url_site_settings.rb  | 13 +++++++++
 ...an_up_deprecated_url_site_settings_spec.rb | 27 +++++++++++++++++++
 3 files changed, 54 insertions(+), 12 deletions(-)
 create mode 100644 app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb
 create mode 100644 spec/jobs/clean_up_deprecated_url_site_settings_spec.rb

diff --git a/app/jobs/onceoff/migrate_url_site_settings.rb b/app/jobs/onceoff/migrate_url_site_settings.rb
index a969dbceb95..dea74375cf0 100644
--- a/app/jobs/onceoff/migrate_url_site_settings.rb
+++ b/app/jobs/onceoff/migrate_url_site_settings.rb
@@ -1,18 +1,20 @@
 module Jobs
   class MigrateUrlSiteSettings < Jobs::Onceoff
+    SETTINGS = [
+      ['logo_url', 'logo'],
+      ['logo_small_url', 'logo_small'],
+      ['digest_logo_url', 'digest_logo'],
+      ['mobile_logo_url', 'mobile_logo'],
+      ['large_icon_url', 'large_icon'],
+      ['favicon_url', 'favicon'],
+      ['apple_touch_icon_url', 'apple_touch_icon'],
+      ['default_opengraph_image_url', 'opengraph_image'],
+      ['twitter_summary_large_image_url', 'twitter_summary_large_image'],
+      ['push_notifications_icon_url', 'push_notifications_icon'],
+    ]
+
     def execute_onceoff(args)
-      [
-        ['logo_url', 'logo'],
-        ['logo_small_url', 'logo_small'],
-        ['digest_logo_url', 'digest_logo'],
-        ['mobile_logo_url', 'mobile_logo'],
-        ['large_icon_url', 'large_icon'],
-        ['favicon_url', 'favicon'],
-        ['apple_touch_icon_url', 'apple_touch_icon'],
-        ['default_opengraph_image_url', 'opengraph_image'],
-        ['twitter_summary_large_image_url', 'twitter_summary_large_image'],
-        ['push_notifications_icon_url', 'push_notifications_icon'],
-      ].each do |old_setting, new_setting|
+      SETTINGS.each do |old_setting, new_setting|
         old_url = DB.query_single(
           "SELECT value FROM site_settings WHERE name = '#{old_setting}'"
         ).first
diff --git a/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb b/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb
new file mode 100644
index 00000000000..0b2390331dd
--- /dev/null
+++ b/app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb
@@ -0,0 +1,13 @@
+module Jobs
+  class CleanUpDeprecatedUrlSiteSettings < Jobs::Scheduled
+    every 1.day
+
+    def execute(args)
+      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)
+        end
+      end
+    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
new file mode 100644
index 00000000000..f21591d9b84
--- /dev/null
+++ b/spec/jobs/clean_up_deprecated_url_site_settings_spec.rb
@@ -0,0 +1,27 @@
+require 'rails_helper'
+
+RSpec.describe Jobs::CleanUpDeprecatedUrlSiteSettings do
+  before do
+    @original_provider = SiteSetting.provider
+    SiteSetting.provider = SiteSettings::DbProvider.new(SiteSetting)
+  end
+
+  after do
+    SiteSetting.delete_all
+    SiteSetting.provider = @original_provider
+  end
+
+  it 'should clean up the old deprecated site settings correctly' do
+    logo_upload = Fabricate(:upload)
+    SiteSetting.logo = logo_upload
+    SiteSetting.public_send("logo_url=", '/test/some/url', warn: false)
+    SiteSetting.public_send("logo_small_url=", '/test/another/url', warn: false)
+
+    expect do
+      described_class.new.execute({})
+    end.to change { SiteSetting.logo_url }.from("/test/some/url").to("")
+
+    expect(SiteSetting.logo).to eq(logo_upload)
+    expect(SiteSetting.logo_small_url).to eq('/test/another/url')
+  end
+end