From a6de4a5ce98cc4b2d39999f1e17073ee2a8cb95f Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Thu, 16 Sep 2021 07:58:53 +0530 Subject: [PATCH] DEV: use upload id to save in theme setting instead of URL. (#14341) When we use URL instead it creates the problem while changing the CDN hostname. --- app/jobs/scheduled/clean_up_uploads.rb | 1 - app/models/upload.rb | 6 +++ ..._value_on_theme_setting_for_upload_type.rb | 19 ++++++++ lib/theme_settings_manager.rb | 45 +++++++++++++++++-- .../components/theme_settings_manager_spec.rb | 20 +++++++++ spec/jobs/clean_up_uploads_spec.rb | 2 +- spec/models/theme_spec.rb | 2 +- 7 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20210914152002_update_value_on_theme_setting_for_upload_type.rb diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index a3d9f1e55e7..770ff7144b8 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -38,7 +38,6 @@ module Jobs encoded_sha = Base62.encode(upload.sha1.hex) next if ReviewableQueuedPost.pending.where("payload->>'raw' LIKE '%#{upload.sha1}%' OR payload->>'raw' LIKE '%#{encoded_sha}%'").exists? next if Draft.where("data LIKE '%#{upload.sha1}%' OR data LIKE '%#{encoded_sha}%'").exists? - next if ThemeSetting.where(data_type: ThemeSetting.types[:upload]).where("value LIKE ?", "%#{upload.sha1}%").exists? if defined?(ChatMessage) && ChatMessage.where("message LIKE ? OR message LIKE ?", "%#{upload.sha1}%", "%#{encoded_sha}%").exists? next diff --git a/app/models/upload.rb b/app/models/upload.rb index f03883783cb..d0611ba84af 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -90,6 +90,12 @@ class Upload < ActiveRecord::Base .where("g.flair_upload_id IS NULL") .joins("LEFT JOIN badges b ON b.image_upload_id = uploads.id") .where("b.image_upload_id IS NULL") + .joins(<<~SQL) + LEFT JOIN theme_settings ts + ON NULLIF(ts.value, '')::integer = uploads.id + AND ts.data_type = #{ThemeSetting.types[:upload].to_i} + SQL + .where("ts.value IS NULL") if SiteSetting.selectable_avatars.present? scope = scope.where.not(id: SiteSetting.selectable_avatars.map(&:id)) diff --git a/db/migrate/20210914152002_update_value_on_theme_setting_for_upload_type.rb b/db/migrate/20210914152002_update_value_on_theme_setting_for_upload_type.rb new file mode 100644 index 00000000000..00807694c3c --- /dev/null +++ b/db/migrate/20210914152002_update_value_on_theme_setting_for_upload_type.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class UpdateValueOnThemeSettingForUploadType < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + UPDATE theme_settings + SET value = (SELECT id FROM uploads WHERE uploads.url = theme_settings.value) + WHERE data_type = 6 + SQL + end + + def down + execute <<~SQL + UPDATE theme_settings + SET value = (SELECT url FROM uploads WHERE uploads.id = theme_settings.value) + WHERE data_type = 6 + SQL + end +end diff --git a/lib/theme_settings_manager.rb b/lib/theme_settings_manager.rb index 7ebbcc21418..2e905717b7c 100644 --- a/lib/theme_settings_manager.rb +++ b/lib/theme_settings_manager.rb @@ -22,7 +22,7 @@ class ThemeSettingsManager end def value - has_record? ? db_record.value : @default + has_record? ? db_record.value : default end def type_name @@ -174,9 +174,48 @@ class ThemeSettingsManager class Upload < self def value - val = super - Discourse.store.cdn_url(val) + cdn_url(super) end + def default + upload_id = default_upload_id + return if upload_id.blank? + + cdn_url(upload_id) + end + + def value=(new_value) + if new_value.present? + if new_value == default + new_value = default_upload_id + else + upload = ::Upload.find_by(url: new_value) + new_value = upload.id if upload.present? + end + end + + super(new_value) + end + + private + + def cdn_url(upload_id) + return if upload_id.blank? + + upload = ::Upload.find_by_id(upload_id.to_i) + return if upload.blank? + + Discourse.store.cdn_url(upload.url) + end + + def default_upload_id + theme_field = theme.theme_fields.find_by( + name: @default, + type_id: ThemeField.types[:theme_upload_var] + ) + return if theme_field.blank? + + theme_field.upload_id + end end end diff --git a/spec/components/theme_settings_manager_spec.rb b/spec/components/theme_settings_manager_spec.rb index 2659883fd40..70cde6bc0c3 100644 --- a/spec/components/theme_settings_manager_spec.rb +++ b/spec/components/theme_settings_manager_spec.rb @@ -149,4 +149,24 @@ describe ThemeSettingsManager do expect(list_setting.list_type).to eq("compact") end end + + context "Upload" do + let!(:upload) { Fabricate(:upload) } + + it "saves the upload id" do + upload_setting = find_by_name(:upload_setting) + upload_setting.value = upload.url + theme.reload + + expect(ThemeSetting.exists?(theme_id: theme.id, name: "upload_setting", value: upload.id.to_s)).to be_truthy + end + + it "returns the CDN URL" do + upload_setting = find_by_name(:upload_setting) + upload_setting.value = upload.url + theme.reload + + expect(upload_setting.value).to eq(Discourse.store.cdn_url(upload.url)) + end + end end diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index 219b2f4c19e..093a760cc3d 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -288,7 +288,7 @@ describe Jobs::CleanUpUploads do it "does not delete theme setting uploads" do theme = Fabricate(:theme) theme_upload = fabricate_upload - ThemeSetting.create!(theme: theme, data_type: ThemeSetting.types[:upload], value: theme_upload.url, name: "my_setting_name") + ThemeSetting.create!(theme: theme, data_type: ThemeSetting.types[:upload], value: theme_upload.id.to_s, name: "my_setting_name") Jobs::CleanUpUploads.new.execute(nil) diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 90550ff2ce7..a47571e444b 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -542,7 +542,7 @@ HTML default: "" YAML - ThemeSetting.create!(theme: theme, data_type: ThemeSetting.types[:upload], value: upload.url, name: "my_upload") + ThemeSetting.create!(theme: theme, data_type: ThemeSetting.types[:upload], value: upload.id.to_s, name: "my_upload") theme.save! json = JSON.parse(cached_settings(theme.id))