FEATURE: Remove deprecated uploads url site settings.

The site settings have been replaced with direct image upload since
Discourse 2.3.

app/jobs/onceoff/migrate_url_site_settings.rb

@@ -1,92 +0,0 @@
-# frozen_string_literal: true
-
-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)
-      SETTINGS.each do |old_setting, new_setting|
-        upload = SiteSetting.get(new_setting)
-
-        next if upload && upload.id >= Upload::SEEDED_ID_THRESHOLD
-
-        old_url = DB.query_single(
-          "SELECT value FROM site_settings WHERE name = '#{old_setting}'"
-        ).first
-
-        next if old_url.blank?
-
-        count = 0
-        file = nil
-        sleep_interval = 5
-
-        loop do
-          url = UrlHelper.absolute_without_cdn(old_url)
-
-          begin
-            file = FileHelper.download(
-              url,
-              max_file_size: [
-                SiteSetting.max_image_size_kb.kilobytes,
-                20.megabytes
-              ].max,
-              tmp_file_name: 'tmp_site_setting_logo',
-              skip_rate_limit: true,
-              follow_redirect: true
-            )
-          rescue OpenURI::HTTPError,
-                 OpenSSL::SSL::SSLError,
-                 Net::OpenTimeout,
-                 Net::ReadTimeout,
-                 Errno::ECONNREFUSED,
-                 EOFError,
-                 SocketError,
-                 Discourse::InvalidParameters => e
-
-            logger.warn(
-              "Error encountered when trying to download file " +
-              "for #{new_setting}.\n#{e.class}: #{e.message}\n#{e.backtrace.join("\n")}"
-            )
-          end
-
-          count += 1
-          break if file || (file.blank? && count >= 3)
-
-          logger.info(
-            "Failed to download upload from #{url} for #{new_setting}. Retrying..."
-          )
-
-          sleep(count * sleep_interval)
-        end
-
-        next if file.blank?
-
-        upload = UploadCreator.new(
-          file,
-          "#{new_setting}",
-          origin: UrlHelper.absolute(old_url),
-          for_site_setting: true
-        ).create_for(Discourse.system_user.id)
-
-        SiteSetting.set(new_setting, upload)
-      end
-    end
-
-    private
-
-    def logger
-      Rails.logger
-    end
-  end
-end

app/jobs/scheduled/clean_up_deprecated_url_site_settings.rb

@@ -1,16 +0,0 @@
-# frozen_string_literal: true
-
-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.set(old_setting, nil, warn: false)
-          SiteSetting.find_by(name: old_setting).destroy!
-        end
-      end
-    end
-  end
-end

app/jobs/scheduled/clean_up_uploads.rb

@@ -27,15 +27,6 @@ module Jobs
 
       # Any URLs in site settings are fair game
       ignore_urls = [
-        SiteSetting.logo_url(warn: false),
-        SiteSetting.logo_small_url(warn: false),
-        SiteSetting.digest_logo_url(warn: false),
-        SiteSetting.mobile_logo_url(warn: false),
-        SiteSetting.large_icon_url(warn: false),
-        SiteSetting.favicon_url(warn: false),
-        SiteSetting.default_opengraph_image_url(warn: false),
-        SiteSetting.twitter_summary_large_image_url(warn: false),
-        SiteSetting.apple_touch_icon_url(warn: false),
         *SiteSetting.selectable_avatars.split("\n"),
       ].flatten.map do |url|
         if url.present?

config/site_settings.yml

@@ -62,30 +62,18 @@ branding:
     default: -5
     client: true
     type: upload
-  logo_url:
-    hidden: true
-    default: "/images/d-logo-sketch.png"
   logo_small:
     default: -6
     client: true
     type: upload
-  logo_small_url:
-    hidden: true
-    default: "/images/d-logo-sketch-small.png"
   digest_logo:
     default: ""
     client: true
     type: upload
-  digest_logo_url:
-    hidden: true
-    default: ""
   mobile_logo:
     default: ""
     client: true
     type: upload
-  mobile_logo_url:
-    hidden: true
-    default: ""
   large_icon:
     default: ""
     client: true
@@ -93,35 +81,20 @@ branding:
   manifest_icon:
     default: ""
     type: upload
-  large_icon_url:
-    hidden: true
-    default: ""
   favicon:
     default: ""
     client: true
     type: upload
-  favicon_url:
-    hidden: true
-    default: "/images/default-favicon.ico"
   apple_touch_icon:
     default: ""
     client: true
     type: upload
-  apple_touch_icon_url:
-    hidden: true
-    default: "/images/default-apple-touch-icon.png"
   opengraph_image:
     default: ""
     type: upload
-  default_opengraph_image_url:
-    hidden: true
-    default: ""
   twitter_summary_large_image:
     default: ""
     type: upload
-  twitter_summary_large_image_url:
-    hidden: true
-    default: ""
 
 basic:
   display_local_time_in_user_card:
@@ -300,9 +273,6 @@ basic:
   push_notifications_icon:
     default: ""
     type: upload
-  push_notifications_icon_url:
-    hidden: true
-    default: ""
   short_title:
     default: ""
     max: 12

db/migrate/20190315025804_migrate_blank_override_for_upload_site_settings.rb

@@ -1,42 +0,0 @@
-# frozen_string_literal: true
-
-class MigrateBlankOverrideForUploadSiteSettings < ActiveRecord::Migration[5.2]
-  def up
-    {
-      '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_name, new_name|
-      if DB.query_single("SELECT 1 FROM site_settings WHERE name = '#{old_name}' AND value = ''").present? &&
-         DB.query_single("SELECT 1 FROM site_settings WHERE name = '#{new_name}'").empty?
-
-        ActiveRecord::Base.connection.execute <<~SQL
-        INSERT INTO site_settings (
-          name,
-          data_type,
-          value,
-          created_at,
-          updated_at
-        ) VALUES (
-          '#{new_name}',
-          18,
-          '',
-          CURRENT_TIMESTAMP,
-          CURRENT_TIMESTAMP
-        )
-        SQL
-      end
-    end
-  end
-
-  def down
-    raise ActiveRecord::IrreversibleMigration
-  end
-end

db/migrate/20190315055432_migrate_null_override_for_upload_site_settings.rb

@@ -1,42 +0,0 @@
-# frozen_string_literal: true
-
-class MigrateNullOverrideForUploadSiteSettings < ActiveRecord::Migration[5.2]
-  def up
-    {
-      '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_name, new_name|
-      if DB.query_single("SELECT 1 FROM site_settings WHERE name = '#{old_name}' AND value IS NULL").present? &&
-         DB.query_single("SELECT 1 FROM site_settings WHERE name = '#{new_name}'").empty?
-
-        ActiveRecord::Base.connection.execute <<~SQL
-        INSERT INTO site_settings (
-          name,
-          data_type,
-          value,
-          created_at,
-          updated_at
-        ) VALUES (
-          '#{new_name}',
-          18,
-          '',
-          CURRENT_TIMESTAMP,
-          CURRENT_TIMESTAMP
-        )
-        SQL
-      end
-    end
-  end
-
-  def down
-    raise ActiveRecord::IrreversibleMigration
-  end
-end

lib/site_settings/deprecated_settings.rb

@@ -4,16 +4,6 @@ module SiteSettings; end
 
 module SiteSettings::DeprecatedSettings
   SETTINGS = [
-    ['logo_url', 'logo', false, '2.3'],
-    ['logo_small_url', 'logo_small', false, '2.3'],
-    ['digest_logo_url', 'digest_logo', false, '2.3'],
-    ['mobile_logo_url', 'mobile_logo', false, '2.3'],
-    ['large_icon_url', 'large_icon', false, '2.3'],
-    ['favicon_url', 'favicon', false, '2.3'],
-    ['apple_touch_icon_url', 'apple_touch_icon', false, '2.3'],
-    ['default_opengraph_image_url', 'opengraph_image', false, '2.3'],
-    ['twitter_summary_large_image_url', 'twitter_summary_large_image', false, '2.3'],
-    ['push_notifications_icon_url', 'push_notifications_icon', false, '2.3'],
     ['show_email_on_profile', 'moderators_view_emails', true, '2.4'],
     ['allow_moderators_to_create_categories', 'moderators_create_categories', true, '2.4'],
     ['disable_edit_notifications', 'disable_system_edit_notifications', true, '2.4']

plugins/discourse-narrative-bot/spec/lib/certificate_generator_spec.rb

@@ -17,7 +17,6 @@ RSpec.describe DiscourseNarrativeBot::CertificateGenerator do
     describe 'when SiteSetting.site_logo_small_url is blank' do
       before do
         SiteSetting.logo_small = ''
-        SiteSetting.logo_small_url = ''
       end
 
       it 'should not try to fetch a image' do

spec/jobs/clean_up_deprecated_url_site_settings_spec.rb

@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-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.set("logo_url", '/test/some/url', warn: false)
-    SiteSetting.set("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.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
-end

spec/jobs/clean_up_uploads_spec.rb

@@ -127,60 +127,18 @@ describe Jobs::CleanUpUploads do
   end
 
   it "does not clean up uploads with URLs used in site settings" do
-    logo_upload = fabricate_upload
-    logo_small_upload = fabricate_upload
-    digest_logo_upload = fabricate_upload
-    mobile_logo_upload = fabricate_upload
-    large_icon_upload = fabricate_upload
-    default_opengraph_image_upload = fabricate_upload
-    twitter_summary_large_image_upload = fabricate_upload
-    favicon_upload = fabricate_upload
-    apple_touch_icon_upload = fabricate_upload
     avatar1_upload = fabricate_upload
     avatar2_upload = fabricate_upload
 
-    SiteSetting.logo_url = logo_upload.url
-    SiteSetting.logo_small_url = logo_small_upload.url
-    SiteSetting.digest_logo_url = digest_logo_upload.url
-    SiteSetting.mobile_logo_url = mobile_logo_upload.url
-    SiteSetting.large_icon_url = large_icon_upload.url
-    SiteSetting.default_opengraph_image_url = default_opengraph_image_upload.url
-
-    SiteSetting.twitter_summary_large_image_url =
-      twitter_summary_large_image_upload.url
-
-    SiteSetting.favicon_url = favicon_upload.url
-    SiteSetting.apple_touch_icon_url = apple_touch_icon_upload.url
     SiteSetting.selectable_avatars = [avatar1_upload.url, avatar2_upload.url].join("\n")
 
     Jobs::CleanUpUploads.new.execute(nil)
 
     expect(Upload.exists?(id: expired_upload.id)).to eq(false)
-    expect(Upload.exists?(id: logo_upload.id)).to eq(true)
-    expect(Upload.exists?(id: logo_small_upload.id)).to eq(true)
-    expect(Upload.exists?(id: digest_logo_upload.id)).to eq(true)
-    expect(Upload.exists?(id: mobile_logo_upload.id)).to eq(true)
-    expect(Upload.exists?(id: large_icon_upload.id)).to eq(true)
-    expect(Upload.exists?(id: default_opengraph_image_upload.id)).to eq(true)
-    expect(Upload.exists?(id: twitter_summary_large_image_upload.id)).to eq(true)
-    expect(Upload.exists?(id: favicon_upload.id)).to eq(true)
-    expect(Upload.exists?(id: apple_touch_icon_upload.id)).to eq(true)
     expect(Upload.exists?(id: avatar1_upload.id)).to eq(true)
     expect(Upload.exists?(id: avatar2_upload.id)).to eq(true)
   end
 
-  it "does not clean up uploads in site settings when they use the CDN" do
-    Discourse.stubs(:asset_host).returns("//my.awesome.cdn")
-
-    logo_small_upload = fabricate_upload
-    SiteSetting.logo_small_url = "#{Discourse.asset_host}#{logo_small_upload.url}"
-
-    Jobs::CleanUpUploads.new.execute(nil)
-
-    expect(Upload.exists?(id: expired_upload.id)).to eq(false)
-    expect(Upload.exists?(id: logo_small_upload.id)).to eq(true)
-  end
-
   it "does not delete profile background uploads" do
     profile_background_upload = fabricate_upload
     UserProfile.last.upload_profile_background(profile_background_upload)

spec/jobs/migrate_url_site_settings_spec.rb

@@ -1,74 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe Jobs::MigrateUrlSiteSettings do
-  before do
-    SiteSetting.authorized_extensions = ''
-  end
-
-  it 'should migrate to the new upload site settings correctly' do
-    [
-      %w{logo_url /test.png},
-      %w{logo_small_url https://test.discourse.awesome/test.png},
-      %w{favicon_url http://test.discourse.awesome/some.ico},
-      %w{digest_logo_url /test.png},
-      %w{mobile_logo_url /test.png},
-      %w{large_icon_url /test.png},
-      %w{apple_touch_icon_url /test.png},
-      %w{default_opengraph_image_url /test.png},
-      %w{twitter_summary_large_image_url //omg.aws.somestack/test.png},
-      %w{push_notifications_icon_url //omg.aws.somestack/test.png}
-    ].each do |name, value|
-
-      SiteSetting.create!(
-        name: name,
-        value: value,
-        data_type: SiteSettings::TypeSupervisor.types[:string]
-      )
-    end
-
-    %w{
-      http://test.localhost/test.png
-      https://omg.aws.somestack/test.png
-    }.each do |url|
-      stub_request(:get, url).to_return(
-        status: 200, body: file_from_fixtures("smallest.png").read
-      )
-    end
-
-    stub_request(:get, "https://test.discourse.awesome/test.png")
-      .to_return(status: 200, body: file_from_fixtures("downsized.png").read)
-
-    stub_request(:get, "http://test.discourse.awesome/some.ico")
-      .to_return(status: 200, body: file_from_fixtures("smallest.ico").read)
-
-    expect do
-      described_class.new.execute_onceoff({})
-    end.to change { Upload.count }.by(3)
-
-    upload = Upload.find_by(original_filename: "logo.png")
-    upload2 = Upload.find_by(original_filename: "logo_small.png")
-    upload3 = Upload.find_by(original_filename: "favicon.ico")
-
-    expect(SiteSetting.logo_small).to eq(upload2)
-    expect(SiteSetting.logo_small.is_a?(Upload)).to eq(true)
-
-    expect(SiteSetting.favicon).to eq(upload3)
-    expect(SiteSetting.favicon.is_a?(Upload)).to eq(true)
-
-    %i{
-      logo
-      digest_logo
-      mobile_logo
-      large_icon
-      apple_touch_icon
-      opengraph_image
-      twitter_summary_large_image
-      push_notifications_icon
-    }.each do |setting|
-      expect(SiteSetting.get(setting)).to eq(upload)
-      expect(SiteSetting.get(setting).is_a?(Upload)).to eq(true)
-    end
-  end
-end