FEATURE: Show themeable site settings in site setting lists (#34666)

This commit removes the filter that would remove themeable
site settings from all setting lists, so they are easier to
find for admins.

To do this, we show the value of the site's default theme
for that theme site setting, disable the setting component,
and provide a link to the site's default theme for quick
editing.

---------

Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>

AI-AGENTS.md

@@ -30,9 +30,10 @@ Discourse is large with long history. Understand context before changes.
 - Members: specify `@type`
 
 ## Testing
+- Do not write unnecessary comments in tests, every single assertion doesn't need a comment
 - Don't test functionality handled by other classes/components
 - Don't write obvious tests
-- Ruby: use `fab!()` over `let()`, system tests for UI (`spec/system`), page objects (`spec/system/page_objects`)
+- Ruby: use `fab!()` over `let()`, system tests for UI (`spec/system`), use page objects for system spec finders (`spec/system/page_objects`)
 
 ### Commands
 ```bash

app/assets/javascripts/admin/addon/components/site-setting.gjs

@@ -12,9 +12,11 @@ import { isNone } from "@ember/utils";
 import { and } from "truth-helpers";
 import DButton from "discourse/components/d-button";
 import JsonSchemaEditorModal from "discourse/components/modal/json-schema-editor";
+import basePath from "discourse/helpers/base-path";
 import icon from "discourse/helpers/d-icon";
 import { bind } from "discourse/lib/decorators";
 import { deepEqual } from "discourse/lib/object";
+import { sanitize } from "discourse/lib/text";
 import { splitString } from "discourse/lib/utilities";
 import { i18n } from "discourse-i18n";
 import SettingValidationMessage from "admin/components/setting-validation-message";
@@ -55,6 +57,7 @@ export default class SiteSettingComponent extends Component {
   @service router;
   @service siteSettingChangeTracker;
   @service messageBus;
+  @service site;
 
   @tracked isSecret = null;
   @tracked status = null;
@@ -92,6 +95,10 @@ export default class SiteSettingComponent extends Component {
     );
   }
 
+  get defaultTheme() {
+    return this.site.user_themes.find((theme) => theme.default);
+  }
+
   @bind
   async onMessage(membership) {
     this.status = membership.status;
@@ -131,6 +138,20 @@ export default class SiteSettingComponent extends Component {
     return this.componentType !== "bool";
   }
 
+  get showThemeSiteSettingWarning() {
+    return this.setting.themeable;
+  }
+
+  get themeSiteSettingWarningText() {
+    return htmlSafe(
+      i18n("admin.theme_site_settings.site_setting_warning", {
+        basePath,
+        defaultThemeName: sanitize(this.defaultTheme.name),
+        defaultThemeId: this.defaultTheme.theme_id,
+      })
+    );
+  }
+
   get dirty() {
     let bufferVal = this.buffered.get("value");
     let settingVal = this.setting?.value;
@@ -266,6 +287,10 @@ export default class SiteSettingComponent extends Component {
   }
 
   get canUpdate() {
+    if (this.setting.themeable) {
+      return false;
+    }
+
     if (!this.status || this.status === "completed") {
       return true;
     } else {
@@ -429,6 +454,7 @@ export default class SiteSettingComponent extends Component {
         {{else}}
           <this.resolvedComponent
             {{on "keydown" this._handleKeydown}}
+            @disabled={{this.setting.themeable}}
             @setting={{this.setting}}
             @value={{this.buffered.value}}
             @preview={{this.preview}}
@@ -444,6 +470,14 @@ export default class SiteSettingComponent extends Component {
             <Description @description={{this.setting.description}} />
             <JobStatus @status={{this.status}} @progress={{this.progress}} />
           {{/if}}
+          {{#if this.showThemeSiteSettingWarning}}
+            <div class="setting-theme-warning">
+              <p class="setting-theme-warning__text">
+                {{icon "paintbrush"}}
+                {{this.themeSiteSettingWarningText}}
+              </p>
+            </div>
+          {{/if}}
         {{/if}}
       </div>
 

app/assets/javascripts/admin/addon/components/site-settings/bool.gjs

@@ -26,6 +26,7 @@ export default class Bool extends Component {
         {{on "input" this.onToggle}}
         type="checkbox"
         checked={{this.enabled}}
+        disabled={{@disabled}}
       />
       <span>{{htmlSafe @setting.description}}</span>
     </label>

app/assets/javascripts/admin/addon/components/site-settings/enum.gjs

@@ -11,7 +11,11 @@ export default class Enum extends Component {
       @onChange={{fn (mut this.value)}}
       @valueProperty={{this.setting.computedValueProperty}}
       @nameProperty={{this.setting.computedNameProperty}}
-      @options={{hash castInteger=true allowAny=this.setting.allowsNone}}
+      @options={{hash
+        castInteger=true
+        allowAny=this.setting.allowsNone
+        disabled=@disabled
+      }}
     />
 
     {{this.preview}}

app/assets/javascripts/admin/addon/components/site-settings/host-list.gjs

@@ -28,7 +28,7 @@ export default class HostList extends Component {
       @settingName={{this.setting.setting}}
       @choices={{this.settingValue}}
       @onChange={{this.onChange}}
-      @options={{hash allowAny=this.allowAny}}
+      @options={{hash allowAny=this.allowAny disabled=@disabled}}
     />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/integer.gjs

@@ -34,6 +34,7 @@ export default class SiteSettingsInteger extends Component {
       max={{if @setting.max @setting.max null}}
       class="input-setting-integer"
       step="1"
+      disabled={{@disabled}}
     />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/list.gjs

@@ -8,6 +8,7 @@ export default class List extends Component {
       @values={{this.value}}
       @inputDelimiter="|"
       @choices={{this.setting.choices}}
+      @disabled={{@disabled}}
     />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/string.gjs

@@ -5,16 +5,25 @@ import TextField from "discourse/components/text-field";
 export default class String extends Component {
   <template>
     {{#if this.setting.textarea}}
-      <Textarea @value={{this.value}} class="input-setting-textarea" />
+      <Textarea
+        @value={{this.value}}
+        class="input-setting-textarea"
+        @disabled={{@disabled}}
+      />
     {{else if this.isSecret}}
       <Input
         @type="password"
         @value={{this.value}}
         class="input-setting-string"
         autocomplete="new-password"
+        @disabled={{@disabled}}
       />
     {{else}}
-      <TextField @value={{this.value}} @classNames="input-setting-string" />
+      <TextField
+        @value={{this.value}}
+        @classNames="input-setting-string"
+        @disabled={{@disabled}}
+      />
     {{/if}}
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/tag-group-list.gjs

@@ -22,6 +22,7 @@ export default class TagGroupList extends Component {
       @onChange={{this.onTagGroupChange}}
       @options={{hash
         filterPlaceholder="category.required_tag_group.placeholder"
+        disabled=@disabled
       }}
     />
   </template>

app/assets/javascripts/admin/addon/components/site-settings/tag-list.gjs

@@ -22,7 +22,7 @@ export default class TagList extends Component {
       @onChange={{this.changeSelectedTags}}
       @everyTag={{true}}
       @unlimitedTagCount={{true}}
-      @options={{hash allowAny=false}}
+      @options={{hash allowAny=false disabled=@disabled}}
     />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/upload.gjs

@@ -26,6 +26,7 @@ export default class Upload extends Component {
       @additionalParams={{hash for_site_setting=true}}
       @type="site_setting"
       @id={{concat "site-setting-image-uploader-" this.setting.setting}}
+      @disabled={{@disabled}}
     />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/uploaded-image-list.gjs

@@ -27,6 +27,7 @@ export default class UploadedImageList extends Component {
         this.showUploadModal
         (hash value=this.value setting=this.setting)
       }}
+      @disabled={{@disabled}}
     />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/url-list.gjs

@@ -4,6 +4,10 @@ import ValueList from "admin/components/value-list";
 
 export default class UrlList extends Component {
   <template>
-    <ValueList @values={{this.value}} @addKey="admin.site_settings.add_url" />
+    <ValueList
+      @disabled={{@disabled}}
+      @values={{this.value}}
+      @addKey="admin.site_settings.add_url"
+    />
   </template>
 }

app/assets/javascripts/admin/addon/components/site-settings/value-list.gjs

@@ -3,5 +3,7 @@ import Component from "@ember/component";
 import ValueList from "admin/components/value-list";
 
 export default class SiteSettingValueList extends Component {
-  <template><ValueList @values={{this.value}} /></template>
+  <template>
+    <ValueList @disabled={{@disabled}} @values={{this.value}} />
+  </template>
 }

app/assets/javascripts/admin/addon/components/value-list.gjs

@@ -187,7 +187,7 @@ export default class ValueList extends Component {
       @value={{this.newValue}}
       @content={{this.filteredChoices}}
       @onChange={{this.selectChoice}}
-      @options={{hash allowAny=true none=this.noneKey}}
+      @options={{hash allowAny=true none=this.noneKey disabled=@disabled}}
     />
   </template>
 }

app/assets/javascripts/discourse/tests/integration/components/site-setting-test.gjs

@@ -358,6 +358,83 @@ module("Integration | Component | SiteSetting", function (hooks) {
   });
 });
 
+module(
+  "Integration | Component | SiteSetting | Themeable Settings",
+  function (hooks) {
+    setupRenderingTest(hooks);
+
+    test("disables input for themeable site settings", async function (assert) {
+      const self = this;
+
+      this.site = this.container.lookup("service:site");
+      this.site.set("user_themes", [
+        { theme_id: 5, default: true, name: "Default Theme" },
+      ]);
+
+      this.set(
+        "setting",
+        SiteSetting.create({
+          setting: "test_themeable_setting",
+          value: "test value",
+          type: "string",
+          themeable: true,
+        })
+      );
+
+      await render(
+        <template><SiteSettingComponent @setting={{self.setting}} /></template>
+      );
+
+      assert.dom(".input-setting-string").hasAttribute("disabled", "");
+      assert
+        .dom(".setting-controls__ok")
+        .doesNotExist("save button is not shown");
+    });
+
+    test("shows warning text for themeable site settings", async function (assert) {
+      const self = this;
+
+      this.site = this.container.lookup("service:site");
+      this.site.set("user_themes", [
+        { theme_id: 5, default: true, name: "Default Theme" },
+      ]);
+
+      this.set(
+        "setting",
+        SiteSetting.create({
+          setting: "test_themeable_setting",
+          value: "test value",
+          type: "string",
+          themeable: true,
+        })
+      );
+
+      await render(
+        <template><SiteSettingComponent @setting={{self.setting}} /></template>
+      );
+
+      assert
+        .dom(".setting-theme-warning")
+        .exists("warning wrapper is displayed");
+
+      assert
+        .dom(".setting-theme-warning__text")
+        .exists("warning text element is displayed");
+
+      const expectedText = i18n(
+        "admin.theme_site_settings.site_setting_warning",
+        {
+          basePath: "",
+          defaultThemeName: "Default Theme",
+          defaultThemeId: 5,
+        }
+      );
+
+      assert.dom(".setting-theme-warning__text").includesHtml(expectedText);
+    });
+  }
+);
+
 module(
   "Integration | Component | SiteSetting | file_size_restriction type",
   function (hooks) {

app/assets/stylesheets/admin/settings.scss

@@ -73,6 +73,16 @@
       }
     }
 
+    .setting-theme-warning {
+      font-size: var(--font-down-1);
+      color: var(--primary-medium);
+
+      .d-icon {
+        font-size: var(--font-down-1);
+        vertical-align: baseline;
+      }
+    }
+
     .setting-controls {
       float: left;
     }

app/controllers/admin/site_settings_controller.rb

@@ -14,6 +14,8 @@ class Admin::SiteSettingsController < Admin::AdminController
           filter_plugin: params[:plugin],
           filter_names: params[:names],
         ),
+      default_theme:
+        BasicThemeSerializer.new(Theme.find_default, scope: guardian, root: false).as_json,
     )
   end
 

config/locales/client.en.yml

@@ -5964,6 +5964,7 @@ en:
         help: "The theme you are currently using is <a href='%{basePath}/admin/customize/themes/%{currentThemeId}'>%{currentTheme}</a>. Go to the <a href='%{basePath}/admin/customize/themes/%{currentThemeId}'>theme config page</a> to alter theme site settings, or click on a linked theme in the table below to edit its settings."
         filter: "Filter theme site settings by setting name, description, or theme name..."
         filter_no_results: "No theme site settings match your filter"
+        site_setting_warning: 'This setting is managed by the default theme for your site (%{defaultThemeName}). You can modify it from <a href="%{basePath}/admin/customize/themes/%{defaultThemeId}">the theme''s edit page</a>.'
 
       search:
         modal_title: "Search everything in admin"

config/locales/server.en.yml

@@ -2761,7 +2761,6 @@ en:
     default_composition_mode: "Set the default mode for your community's composer. Rich text mode provides a more modern, familiar writing experience for most users, while Markdown mode may be suitable for more technical audiences. Members can use a toggle in the composer toolbar to switch between modes."
     viewport_based_mobile_mode: "EXPERIMENTAL: Disable the user-agent-based mobile/desktop modes and use viewport width instead."
     reviewable_ui_refresh: "Groups that can use the experimental new UI in the review queue."
-
     content_localization_enabled: "Displays localized content for users based on their browser or user language preferences. Such content may include categories, tags, posts, and topics. Supported locales are set in 'content localization supported locales'."
     content_localization_supported_locales: "List of supported locales that user content can be translated to. Requires 'content localization enabled'."
     content_localization_allowed_groups: "Groups allowed to update localized content. Requires 'content localization enabled'."

lib/site_setting_extension.rb

@@ -339,16 +339,16 @@ module SiteSettingExtension
           true
         end
       end
-      .reject do |setting_name, _|
-        # Do not show themeable site settings all_settings list or in the UI, they
-        # are managed separately via the ThemeSiteSetting model.
-        themeable[setting_name]
-      end
       .map do |s, v|
         type_hash = type_supervisor.type_hash(s)
         default = defaults.get(s, default_locale).to_s
 
+        if themeable[s]
+          value = public_send(s, { theme_id: SiteSetting.default_theme_id })
+        else
           value = public_send(s)
+        end
+
         value = value.map(&:to_s).join("|") if type_hash[:type].to_s == "uploaded_image_list"
 
         if type_hash[:type].to_s == "upload" && default.to_i < Upload::SEEDED_ID_THRESHOLD

spec/system/admin_config_theme_site_settings_spec.rb

@@ -44,4 +44,20 @@ describe "Admin Theme Site Settings", type: :system do
       "search_field",
     )
   end
+
+  describe "all site setting list" do
+    fab!(:default_theme) { Theme.find_default }
+    let(:site_settings_page) { PageObjects::Pages::AdminSiteSettings.new }
+
+    it "shows warning and disabled state for themeable site settings" do
+      site_settings_page.visit("enable_welcome_banner")
+
+      expect(site_settings_page).to have_disabled_input("enable_welcome_banner")
+      expect(site_settings_page).to have_theme_warning(
+        "enable_welcome_banner",
+        default_theme.name,
+        default_theme.id,
+      )
+    end
+  end
 end

spec/system/page_objects/pages/admin_site_settings.rb

@@ -141,6 +141,17 @@ module PageObjects
         setting = find_setting(setting_name)
         setting.find(".setting-value .validation-error").text
       end
+
+      def has_theme_warning?(setting_name, theme_name, theme_id)
+        find_setting(setting_name).find(".setting-theme-warning__text").has_text?(theme_name) &&
+          find_setting(setting_name).find(".setting-theme-warning__text").has_link?(
+            href: "/admin/customize/themes/#{theme_id}",
+          )
+      end
+
+      def has_disabled_input?(setting_name)
+        find_setting(setting_name).has_css?("input[disabled]")
+      end
     end
   end
 end