From 3f908c047dfcee97078bab79f2cde54d4e98c15c Mon Sep 17 00:00:00 2001
From: Osama Sayegh <asooomaasoooma90@gmail.com>
Date: Mon, 6 Mar 2023 11:41:47 +0300
Subject: [PATCH] FIX: Use the default value correctly for theme settings of
 type uploads (#20541)

When a theme setting of type `upload` has a default upload, it should return the URL of the specified default upload until a custom upload is used for the setting. However, currently this isn't the case and we get null instead of the default upload URL.

The reason for this is because the `super` method of `#value` already returns the default upload URL (if there's one), so we can't pass that to `cdn_url` which expects an upload ID:

https://github.com/discourse/discourse/blob/c961dcc757911901b213b26c97a9972d7291c8c6/lib/theme_settings_manager.rb#L212

This commit fixes the bug by skipping the call to `cdn_url` when we fallback to the default upload for the setting value.
---
 lib/theme_settings_manager.rb                 |  2 +-
 .../theme_settings/valid_settings.yaml        |  2 +-
 spec/lib/theme_settings_manager_spec.rb       | 28 +++++++++++++++----
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/lib/theme_settings_manager.rb b/lib/theme_settings_manager.rb
index 1c353059f6..7a5f987ab8 100644
--- a/lib/theme_settings_manager.rb
+++ b/lib/theme_settings_manager.rb
@@ -184,7 +184,7 @@ class ThemeSettingsManager
 
   class Upload < self
     def value
-      cdn_url(super)
+      has_record? ? cdn_url(db_record.value) : default
     end
 
     def default
diff --git a/spec/fixtures/theme_settings/valid_settings.yaml b/spec/fixtures/theme_settings/valid_settings.yaml
index a6e8e9b6fa..4c3e3b480d 100644
--- a/spec/fixtures/theme_settings/valid_settings.yaml
+++ b/spec/fixtures/theme_settings/valid_settings.yaml
@@ -71,7 +71,7 @@ enum_setting_03:
 
 upload_setting:
   type: upload
-  default: ""
+  default: "default-upload"
 
 invalid_json_schema_setting:
   default: ""
diff --git a/spec/lib/theme_settings_manager_spec.rb b/spec/lib/theme_settings_manager_spec.rb
index f1ec48b6dd..ac507ad146 100644
--- a/spec/lib/theme_settings_manager_spec.rb
+++ b/spec/lib/theme_settings_manager_spec.rb
@@ -161,12 +161,30 @@ RSpec.describe ThemeSettingsManager do
       ).to be_truthy
     end
 
-    it "returns the CDN URL" do
-      upload_setting = find_by_name(:upload_setting)
-      upload_setting.value = upload.url
-      theme.reload
+    describe "#value" do
+      context "when it's changed to a custom upload" do
+        it "returns 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))
+          expect(upload_setting.value).to eq(Discourse.store.cdn_url(upload.url))
+        end
+      end
+
+      context "when there's a default upload" do
+        it "returns CDN URL" do
+          theme.set_field(
+            target: :common,
+            name: "default-upload",
+            type: :theme_upload_var,
+            upload_id: upload.id,
+          )
+          theme.save!
+          upload_setting = find_by_name(:upload_setting)
+          expect(upload_setting.value).to eq(Discourse.store.cdn_url(upload.url))
+        end
+      end
     end
   end
 end