diff --git a/app/assets/javascripts/admin/addon/components/admin-config-area-empty-list.gjs b/app/assets/javascripts/admin/addon/components/admin-config-area-empty-list.gjs index bb4b8a2c1e9..f6bd0d6c761 100644 --- a/app/assets/javascripts/admin/addon/components/admin-config-area-empty-list.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-config-area-empty-list.gjs @@ -26,6 +26,7 @@ export default class AdminConfigAreaEmptyList extends Component { }} @action={{@ctaAction}} @route={{@ctaRoute}} + @routeModels={{@ctaRouteModels}} /> {{/if}} {{yield}} diff --git a/app/assets/javascripts/admin/addon/components/admin-config-areas/themes.gjs b/app/assets/javascripts/admin/addon/components/admin-config-areas/themes.gjs index ffef0bf9593..a49b730d82a 100644 --- a/app/assets/javascripts/admin/addon/components/admin-config-areas/themes.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-config-areas/themes.gjs @@ -5,6 +5,7 @@ import { service } from "@ember/service"; import { isPresent } from "@ember/utils"; import DPageSubheader from "discourse/components/d-page-subheader"; import PluginOutlet from "discourse/components/plugin-outlet"; +import getURL from "discourse/helpers/get-url"; import lazyHash from "discourse/helpers/lazy-hash"; import { i18n } from "discourse-i18n"; import InstallThemeModal from "admin/components/modal/install-theme"; @@ -90,6 +91,7 @@ export default class AdminConfigAreasThemes extends Component { }} @descriptionLabel={{i18n "admin.config_areas.themes_and_components.themes.description" + themeSiteSettingsUrl=(getURL "/admin/config/theme-site-settings") }} > <:actions as |actions|> diff --git a/app/assets/javascripts/admin/addon/components/theme-setting-editor.js b/app/assets/javascripts/admin/addon/components/theme-setting-editor.js index a6aabc6c5a9..d4052840ff6 100644 --- a/app/assets/javascripts/admin/addon/components/theme-setting-editor.js +++ b/app/assets/javascripts/admin/addon/components/theme-setting-editor.js @@ -1,10 +1,20 @@ +import { service } from "@ember/service"; +import { i18n } from "discourse-i18n"; import SiteSettingComponent from "./site-setting"; -export default class extends SiteSettingComponent { +export default class ThemeSettingEditor extends SiteSettingComponent { + @service toasts; + _save() { - return this.setting.updateSetting( - this.args.model.id, - this.buffered.get("value") - ); + return this.setting + .updateSetting(this.args.model.id, this.buffered.get("value")) + .then(() => { + this.toasts.success({ + data: { + message: i18n("admin.customize.theme.theme_setting_saved"), + }, + duration: "short", + }); + }); } } diff --git a/app/assets/javascripts/admin/addon/components/theme-site-setting-editor.gjs b/app/assets/javascripts/admin/addon/components/theme-site-setting-editor.gjs new file mode 100644 index 00000000000..c2170c1b66b --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/theme-site-setting-editor.gjs @@ -0,0 +1,20 @@ +import { service } from "@ember/service"; +import { i18n } from "discourse-i18n"; +import SiteSettingComponent from "./site-setting"; + +export default class ThemeSiteSettingEditor extends SiteSettingComponent { + @service toasts; + + _save() { + return this.setting + .updateSetting(this.args.model.id, this.buffered.get("value")) + .then(() => { + this.toasts.success({ + data: { + message: i18n("admin.customize.theme.theme_site_setting_saved"), + }, + duration: "short", + }); + }); + } +} diff --git a/app/assets/javascripts/admin/addon/components/theme-site-settings.gjs b/app/assets/javascripts/admin/addon/components/theme-site-settings.gjs new file mode 100644 index 00000000000..60806fa04d2 --- /dev/null +++ b/app/assets/javascripts/admin/addon/components/theme-site-settings.gjs @@ -0,0 +1,133 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { array } from "@ember/helper"; +import { action } from "@ember/object"; +import { LinkTo } from "@ember/routing"; +import { service } from "@ember/service"; +import { eq } from "truth-helpers"; +import AsyncContent from "discourse/components/async-content"; +import DPageSubheader from "discourse/components/d-page-subheader"; +import basePath from "discourse/helpers/base-path"; +import { ajax } from "discourse/lib/ajax"; +import { currentThemeId, listThemes } from "discourse/lib/theme-selector"; +import { i18n } from "discourse-i18n"; +import DTooltip from "float-kit/components/d-tooltip"; + +export default class ThemeSiteSettings extends Component { + @service site; + @service router; + + @tracked themesWithSiteSettingOverrides = null; + @tracked themeableSiteSettings = null; + + get themes() { + return listThemes(this.site); + } + + get currentThemeIdValue() { + return currentThemeId(); + } + + get currentTheme() { + return this.themes.find((theme) => { + return eq(theme.id, this.currentThemeIdValue); + }); + } + + isLastThemeSettingOverride(overrides, theme) { + return theme === overrides.themes.at(-1); + } + + @action + async loadThemeSiteSettings() { + let url = "/admin/config/theme-site-settings.json"; + const response = await ajax(url, { + method: "GET", + }); + this.themeableSiteSettings = response.themeable_site_settings.map( + (setting) => { + return { + name: setting.humanized_name, + value: setting, + }; + } + ); + this.themesWithSiteSettingOverrides = + response.themes_with_site_setting_overrides; + return this.themesWithSiteSettingOverrides; + } + + +} diff --git a/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show-index.js b/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show-index.js index eedeb1835f2..1a61b9a7312 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show-index.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show-index.js @@ -47,7 +47,9 @@ export default class AdminCustomizeThemesShowIndexController extends Controller @filterBy("model.theme_fields", "target", "extra_js") extraFiles; @notEmpty("settings") hasSettings; @notEmpty("translations") hasTranslations; + @notEmpty("model.themeable_site_settings") hasThemeableSiteSettings; @readOnly("model.settings") settings; + @readOnly("model.themeable_site_settings") themeSiteSettings; @discourseComputed("model.component", "model.remote_theme") showCheckboxes() { diff --git a/app/assets/javascripts/admin/addon/models/theme-settings.js b/app/assets/javascripts/admin/addon/models/theme-settings.js index 040943c2771..cb2b3709bae 100644 --- a/app/assets/javascripts/admin/addon/models/theme-settings.js +++ b/app/assets/javascripts/admin/addon/models/theme-settings.js @@ -3,18 +3,22 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import SiteSetting from "admin/models/site-setting"; export default class ThemeSettings extends SiteSetting { - updateSetting(themeId, newValue) { + async updateSetting(themeId, newValue) { if (this.objects_schema) { newValue = JSON.stringify(newValue); } - return ajax(`/admin/themes/${themeId}/setting`, { - type: "PUT", - data: { - name: this.setting, - value: newValue, - }, - }); + try { + return ajax(`/admin/themes/${themeId}/setting`, { + type: "PUT", + data: { + name: this.setting, + value: newValue, + }, + }); + } catch (error) { + popupAjaxError(error); + } } loadMetadata(themeId) { diff --git a/app/assets/javascripts/admin/addon/models/theme-site-settings.js b/app/assets/javascripts/admin/addon/models/theme-site-settings.js new file mode 100644 index 00000000000..6da488bc8be --- /dev/null +++ b/app/assets/javascripts/admin/addon/models/theme-site-settings.js @@ -0,0 +1,19 @@ +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import SiteSetting from "admin/models/site-setting"; + +export default class ThemeSiteSettings extends SiteSetting { + async updateSetting(themeId, newValue) { + try { + return ajax(`/admin/themes/${themeId}/site-setting`, { + type: "PUT", + data: { + name: this.setting, + value: newValue, + }, + }); + } catch (error) { + popupAjaxError(error); + } + } +} diff --git a/app/assets/javascripts/admin/addon/models/theme.js b/app/assets/javascripts/admin/addon/models/theme.js index c93374cc33a..67b8404b4f7 100644 --- a/app/assets/javascripts/admin/addon/models/theme.js +++ b/app/assets/javascripts/admin/addon/models/theme.js @@ -9,6 +9,7 @@ import RestModel from "discourse/models/rest"; import { i18n } from "discourse-i18n"; import ColorScheme from "admin/models/color-scheme"; import ThemeSettings from "admin/models/theme-settings"; +import ThemeSiteSettings from "admin/models/theme-site-settings"; const THEME_UPLOAD_VAR = 2; const FIELDS_IDS = [0, 1, 5, 6]; @@ -27,6 +28,12 @@ class Theme extends RestModel { ); } + if (json.themeable_site_settings) { + json.themeable_site_settings = json.themeable_site_settings.map( + (setting) => ThemeSiteSettings.create(setting) + ); + } + const palette = json.owned_color_palette || json.color_scheme || json.base_palette; if (palette) { diff --git a/app/assets/javascripts/admin/addon/routes/admin-config-theme-site-settings.js b/app/assets/javascripts/admin/addon/routes/admin-config-theme-site-settings.js new file mode 100644 index 00000000000..297f8f70757 --- /dev/null +++ b/app/assets/javascripts/admin/addon/routes/admin-config-theme-site-settings.js @@ -0,0 +1,8 @@ +import DiscourseRoute from "discourse/routes/discourse"; +import { i18n } from "discourse-i18n"; + +export default class AdminConfigThemeSiteSettings extends DiscourseRoute { + titleToken() { + return i18n("admin.config.theme_site_settings.title"); + } +} diff --git a/app/assets/javascripts/admin/addon/routes/admin-route-map.js b/app/assets/javascripts/admin/addon/routes/admin-route-map.js index add9fc85710..79a5ad7f5ba 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-route-map.js +++ b/app/assets/javascripts/admin/addon/routes/admin-route-map.js @@ -381,6 +381,7 @@ export default function () { this.route("spam", function () { this.route("settings", { path: "/" }); }); + this.route("theme-site-settings"); this.route( "colorPalettes", diff --git a/app/assets/javascripts/admin/addon/templates/config-theme-site-settings.gjs b/app/assets/javascripts/admin/addon/templates/config-theme-site-settings.gjs new file mode 100644 index 00000000000..f7d26c21c68 --- /dev/null +++ b/app/assets/javascripts/admin/addon/templates/config-theme-site-settings.gjs @@ -0,0 +1,29 @@ +import RouteTemplate from "ember-route-template"; +import DBreadcrumbsItem from "discourse/components/d-breadcrumbs-item"; +import DPageHeader from "discourse/components/d-page-header"; +import { i18n } from "discourse-i18n"; +import ThemeSiteSettings from "admin/components/theme-site-settings"; + +export default RouteTemplate( + +); diff --git a/app/assets/javascripts/admin/addon/templates/customize-themes-show-index.gjs b/app/assets/javascripts/admin/addon/templates/customize-themes-show-index.gjs index a81874a67fa..77e49a68b56 100644 --- a/app/assets/javascripts/admin/addon/templates/customize-themes-show-index.gjs +++ b/app/assets/javascripts/admin/addon/templates/customize-themes-show-index.gjs @@ -1,4 +1,5 @@ import { fn, hash } from "@ember/helper"; +import { htmlSafe } from "@ember/template"; import RouteTemplate from "ember-route-template"; import { and, not } from "truth-helpers"; import ConditionalLoadingSpinner from "discourse/components/conditional-loading-spinner"; @@ -9,10 +10,12 @@ import icon from "discourse/helpers/d-icon"; import formatDate from "discourse/helpers/format-date"; import formatUsername from "discourse/helpers/format-username"; import lazyHash from "discourse/helpers/lazy-hash"; +import getURL from "discourse/lib/get-url"; import { i18n } from "discourse-i18n"; import InlineEditCheckbox from "admin/components/inline-edit-checkbox"; import ThemeSettingEditor from "admin/components/theme-setting-editor"; import ThemeSettingRelativesSelector from "admin/components/theme-setting-relatives-selector"; +import ThemeSiteSettingEditor from "admin/components/theme-site-setting-editor"; import ThemeTranslation from "admin/components/theme-translation"; import ColorPalettes from "select-kit/components/color-palettes"; import ComboBox from "select-kit/components/combo-box"; @@ -289,6 +292,33 @@ export default RouteTemplate( {{/if}} + {{#if @controller.hasThemeableSiteSettings}} +
+
{{i18n + "admin.customize.theme.theme_site_settings" + }}
+

{{htmlSafe + (i18n + "admin.customize.theme.overriden_site_settings_explanation" + themeSiteSettingsConfigUrl=(getURL + "/admin/config/theme-site-settings" + ) + ) + }}

+
+ {{#each @controller.themeSiteSettings as |setting|}} + + {{/each}} +
+
+ {{/if}} + {{#if @controller.hasSettings}}
{{i18n diff --git a/app/assets/javascripts/discourse/app/instance-initializers/subscribe-user-notifications.js b/app/assets/javascripts/discourse/app/instance-initializers/subscribe-user-notifications.js index c6214168b5d..18310ede4fe 100644 --- a/app/assets/javascripts/discourse/app/instance-initializers/subscribe-user-notifications.js +++ b/app/assets/javascripts/discourse/app/instance-initializers/subscribe-user-notifications.js @@ -14,6 +14,7 @@ import { register as registerPushNotifications, unsubscribe as unsubscribePushNotifications, } from "discourse/lib/push-notifications"; +import { currentThemeId } from "discourse/lib/theme-selector"; import Notification from "discourse/models/notification"; class SubscribeUserNotificationsInit { @@ -255,6 +256,14 @@ class SubscribeUserNotificationsInit { @bind onClientSettings(data) { + // Theme site setting changes for client settings should only affect users + // currently using the same theme. + if (data.scoped_to?.theme_id) { + if (currentThemeId() !== data.scoped_to.theme_id) { + return; + } + } + this.siteSettings[data.name] = data.value; } diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js b/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js index 587a1fac0cc..bb2d63507f2 100644 --- a/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js +++ b/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js @@ -272,6 +272,15 @@ export const ADMIN_NAV_MAP = [ description: "admin.config.themes_and_components.header_description", icon: "paintbrush", keywords: "admin.config.themes_and_components.keywords", + links: [ + { + name: "admin_theme_site_settings", + route: "adminConfig.theme-site-settings", + label: "admin.config.theme_site_settings.title", + description: "admin.config.theme_site_settings.header_description", + icon: "gear", + }, + ], }, { name: "admin_customize_site_texts", diff --git a/app/assets/javascripts/discourse/app/services/site-settings.js b/app/assets/javascripts/discourse/app/services/site-settings.js index 8ed8112238f..3b2198eebe5 100644 --- a/app/assets/javascripts/discourse/app/services/site-settings.js +++ b/app/assets/javascripts/discourse/app/services/site-settings.js @@ -2,8 +2,22 @@ import { TrackedObject } from "@ember-compat/tracked-built-ins"; import { disableImplicitInjections } from "discourse/lib/implicit-injections"; import PreloadStore from "discourse/lib/preload-store"; -export function createSiteSettingsFromPreloaded(data) { - const settings = new TrackedObject(data); +export function createSiteSettingsFromPreloaded( + siteSettings, + themeSiteSettingOverrides +) { + const settings = new TrackedObject(siteSettings); + + if (themeSiteSettingOverrides) { + for (const [key, value] of Object.entries(themeSiteSettingOverrides)) { + settings[key] = value; + // eslint-disable-next-line no-console + console.info( + `[Discourse] Overriding site setting ${key} with theme site setting value: ${value}` + ); + } + settings.themeSiteSettingOverrides = themeSiteSettingOverrides; + } settings.groupSettingArray = (groupSetting) => { const setting = settings[groupSetting]; @@ -26,6 +40,9 @@ export default class SiteSettingsService { static isServiceFactory = true; static create() { - return createSiteSettingsFromPreloaded(PreloadStore.get("siteSettings")); + return createSiteSettingsFromPreloaded( + PreloadStore.get("siteSettings"), + PreloadStore.get("themeSiteSettingOverrides") + ); } } diff --git a/app/assets/stylesheets/admin/site-settings.scss b/app/assets/stylesheets/admin/site-settings.scss index 41d3a15b405..101980108e0 100644 --- a/app/assets/stylesheets/admin/site-settings.scss +++ b/app/assets/stylesheets/admin/site-settings.scss @@ -53,3 +53,30 @@ font-weight: 700; } } + +.admin-theme-site-settings-row { + &__setting { + .setting-label { + margin: 0; + } + + .setting-description { + color: var(--primary-medium); + font-size: var(--font-down-1); + } + } + + @include viewport.from(sm) { + &__setting { + width: 40%; + } + + &__default { + width: 10%; + } + + &__overridden { + width: 50%; + } + } +} diff --git a/app/controllers/admin/config/theme_site_settings_controller.rb b/app/controllers/admin/config/theme_site_settings_controller.rb new file mode 100644 index 00000000000..9b61323d8b7 --- /dev/null +++ b/app/controllers/admin/config/theme_site_settings_controller.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class Admin::Config::ThemeSiteSettingsController < Admin::AdminController + def index + respond_to do |format| + format.json do + themes_with_site_setting_overrides = {} + + SiteSetting.themeable_site_settings.each do |setting_name| + themes_with_site_setting_overrides[setting_name] = SiteSetting.setting_metadata_hash( + setting_name, + ).merge(themes: []) + end + + ThemeSiteSetting.themes_with_overridden_settings.each do |row| + themes_with_site_setting_overrides[row.setting_name][:themes] << { + theme_id: row.theme_id, + theme_name: row.theme_name, + value: row.value, + } + end + + render_json_dump( + themeable_site_settings: SiteSetting.themeable_site_settings, + themes_with_site_setting_overrides: themes_with_site_setting_overrides, + ) + end + end + end +end diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index 4f44175e50c..2e4271ac983 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -350,6 +350,30 @@ class Admin::ThemesController < Admin::AdminController render json: updated_setting, status: :ok end + def update_theme_site_setting + Themes::ThemeSiteSettingManager.call( + params: { + theme_id: params[:id], + name: params[:name], + value: params[:value], + }, + guardian:, + ) do + on_success do |theme_site_setting:| + if theme_site_setting.present? + render json: success_json.merge(theme_site_setting.as_json(only: %i[name value theme_id])) + else + render json: success_json + end + end + on_failed_policy(:current_user_is_admin) { raise Discourse::InvalidAccess } + on_failed_policy(:ensure_setting_is_themeable) do + render_json_error(I18n.t("themes.setting_not_themeable", name: params[:name]), status: 400) + end + on_model_not_found(:theme) { raise Discourse::NotFound } + end + end + def schema end diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 49045c67b52..7fbe5e62b2b 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -386,6 +386,8 @@ class RemoteTheme < ActiveRecord::Base raise ActiveRecord::Rollback if !theme.save end + create_theme_site_settings(theme, theme_info["theme_site_settings"]) + theme.migrate_settings(start_transaction: false) if run_migrations end @@ -457,6 +459,35 @@ class RemoteTheme < ActiveRecord::Base theme.color_scheme = ordered_schemes.first if theme.new_record? end + def create_theme_site_settings(theme, theme_site_settings) + theme_site_settings ||= {} + + existing_theme_site_settings = + theme.theme_site_settings.where(name: theme_site_settings.keys).to_a + theme_site_settings.each do |setting, value| + next if !SiteSetting.themeable[setting.to_sym] + + # If there is an existing theme site setting, then don't touch it, + # we don't want to mess with site owner's changes. + existing_theme_site_setting = + existing_theme_site_settings.find do |theme_site_setting| + theme_site_setting.name == setting + end + next if existing_theme_site_setting.present? + + # The manager handles creating the theme site setting record + # if it does not exist. + Themes::ThemeSiteSettingManager.call( + params: { + theme_id: theme.id, + name: setting, + value: value, + }, + guardian: Discourse.system_user.guardian, + ) { |result| } + end + end + def github_diff_link if github_repo_url.present? && local_version != remote_version "#{github_repo_url.gsub(/\.git\z/, "")}/compare/#{local_version}...#{remote_version}" diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 18c3fe5e13c..5fb26dd5d9c 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -337,8 +337,8 @@ class SiteSetting < ActiveRecord::Base protected - def self.clear_cache! - super + def self.clear_cache!(expire_theme_site_setting_cache: false) + super(expire_theme_site_setting_cache:) @blocked_attachment_content_types_regex = nil @blocked_attachment_filenames_regex = nil diff --git a/app/models/theme.rb b/app/models/theme.rb index 4ce9670b1d5..6fa937d78e2 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -93,6 +93,7 @@ class Theme < ActiveRecord::Base has_many :migration_fields, -> { where(target_id: Theme.targets[:migrations]) }, class_name: "ThemeField" + has_many :theme_site_settings, dependent: :destroy validate :component_validations validate :validate_theme_fields @@ -107,6 +108,7 @@ class Theme < ActiveRecord::Base -> do include_basic_relations.includes( :theme_settings, + :theme_site_settings, :settings_field, theme_fields: %i[upload theme_settings_migration], child_themes: %i[color_scheme locale_fields theme_translation_overrides], @@ -126,6 +128,7 @@ class Theme < ActiveRecord::Base ) end + scope :not_components, -> { where(component: false) } scope :not_system, -> { where("id > 0") } scope :system, -> { where("id < 0") } @@ -317,6 +320,15 @@ class Theme < ActiveRecord::Base SvgSprite.expire_cache end + def self.expire_site_setting_cache! + Theme + .not_components + .pluck(:id) + .each do |theme_id| + Discourse.cache.delete(SiteSettingExtension.theme_site_settings_cache_key(theme_id)) + end + end + def self.clear_default! SiteSetting.default_theme_id = -1 expire_site_cache! @@ -1063,7 +1075,12 @@ class Theme < ActiveRecord::Base end def user_selectable_count - UserOption.where(theme_ids: [id]).count + UserOption.where(theme_ids: [self.id]).count + end + + def themeable_site_settings + return [] if self.component? + ThemeSiteSettingResolver.new(theme: self).resolved_theme_site_settings end def find_or_create_owned_color_palette diff --git a/app/models/theme_site_setting.rb b/app/models/theme_site_setting.rb new file mode 100644 index 00000000000..7df81dd079e --- /dev/null +++ b/app/models/theme_site_setting.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +# Copies the same schema as SiteSetting, since the values and +# data types are identical. This table is used as a way for themes +# to overrride specific site settings that we make available in +# core based on the `themeable` designation on a site setting. +# +# Creation, updating, and deletion of theme site settings is done +# via the `Themes::ThemeSiteSettingManager` service. +class ThemeSiteSetting < ActiveRecord::Base + belongs_to :theme + + # Gets a list of themes that have theme site setting records + # and the associated values for those settings, where the + # value is different from the default site setting value. + # + # @return [Array] an array of hashes where each hash contains: + # - :theme_id [Integer] the ID of the theme + # - :theme_name [String] the name of the theme + # - :setting_name [Symbol] the name of the setting + # - :value [String] the value of the setting + # - :data_type [Integer] the data type of the setting + def self.themes_with_overridden_settings + sql = <<~SQL + SELECT theme.id AS theme_id, theme.name AS theme_name, + tss.name AS setting_name, tss.value, tss.data_type + FROM themes theme + INNER JOIN theme_site_settings tss ON theme.id = tss.theme_id + WHERE theme.component = false AND tss.name IN (:setting_names) + ORDER BY tss.name, theme.name + SQL + + DB + .query(sql, setting_names: SiteSetting.themeable_site_settings) + .each { |row| row.setting_name = row.setting_name.to_sym } + .select do |row| + # Do not consider this as an "overridden" setting if the value + # is the same as the default site setting value. + row.value != + SiteSetting + .type_supervisor + .to_db_value(row.setting_name, SiteSetting.defaults[row.setting_name]) + .first + end + .each do |row| + row.value = + SiteSetting.type_supervisor.to_rb_value(row.setting_name, row.value, row.data_type) + end + end + + def self.can_access_db? + !GlobalSetting.skip_redis? && !GlobalSetting.skip_db? && + ActiveRecord::Base.connection.table_exists?(self.table_name) + end + + # Generates a map of theme IDs to their site setting values. When + # there is no theme site setting for a given theme, the default + # site setting value is used. + # + # @return [Hash] a map where keys are theme IDs and values are hashes: + # + # { + # 123 => { + # setting_name_1 => value_1, + # setting_name_2 => value_2, + # ... + # } + # } + def self.generate_theme_map + # Similar to what SiteSettings::DbProvider and SiteSettings::LocalProcessProvider do + # for their #all method, we can't try to load settings if the DB is not available, + # since this method is called within SiteSetting.refresh! which is called on boot. + return {} if !can_access_db? + + theme_site_setting_values_map = {} + Theme + .includes(:theme_site_settings) + .not_components + .each do |theme| + SiteSetting.themeable_site_settings.each do |setting_name| + setting = theme.theme_site_settings.find { |s| s.name == setting_name.to_s } + + value = + if setting.nil? + SiteSetting.defaults[setting_name] + else + SiteSetting.type_supervisor.to_rb_value( + setting.name.to_sym, + setting.value, + setting.data_type, + ) + end + + theme_site_setting_values_map[theme.id] ||= {} + theme_site_setting_values_map[theme.id][setting_name] = value + end + end + + theme_site_setting_values_map + end + + def setting_rb_value + SiteSetting.type_supervisor.to_rb_value(self.name, self.value, self.data_type) + end +end + +# == Schema Information +# +# Table name: theme_site_settings +# +# id :bigint not null, primary key +# theme_id :integer not null +# name :string not null +# data_type :integer not null +# value :text +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_theme_site_settings_on_theme_id (theme_id) +# index_theme_site_settings_on_theme_id_and_name (theme_id,name) UNIQUE +# diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 4193717dd73..d345de40823 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -156,6 +156,7 @@ class UserHistory < ActiveRecord::Base tag_group_destroy: 117, tag_group_change: 118, delete_associated_accounts: 119, + change_theme_site_setting: 120, ) end @@ -278,6 +279,7 @@ class UserHistory < ActiveRecord::Base delete_flag update_flag create_flag + change_theme_site_setting ] end diff --git a/app/serializers/theme_serializer.rb b/app/serializers/theme_serializer.rb index 68cf6c90083..9587a94b9d7 100644 --- a/app/serializers/theme_serializer.rb +++ b/app/serializers/theme_serializer.rb @@ -8,6 +8,7 @@ class ThemeSerializer < BasicThemeSerializer :auto_update, :remote_theme_id, :settings, + :themeable_site_settings, :errors, :supported?, :enabled?, @@ -75,6 +76,15 @@ class ThemeSerializer < BasicThemeSerializer nil end + # Components always return an empty array here + def themeable_site_settings + object.themeable_site_settings + end + + def include_themeable_site_settings? + !object.component? + end + def include_child_themes? !object.component? end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 9bf89155803..d98204e1b5b 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -340,6 +340,19 @@ class StaffActionLogger ) end + def log_theme_site_setting_change(setting_name, previous_value, new_value, theme, opts = {}) + raise Discourse::InvalidParameters.new(:theme) if !theme + + UserHistory.create!( + params(opts).merge( + action: UserHistory.actions[:change_theme_site_setting], + subject: "#{theme.name}: #{setting_name}", + previous_value: previous_value, + new_value: new_value, + ), + ) + end + def log_site_text_change(subject, new_text = nil, old_text = nil, opts = {}) raise Discourse::InvalidParameters.new(:subject) if subject.blank? UserHistory.create!( diff --git a/app/services/themes/create.rb b/app/services/themes/create.rb index 168233db30e..3270e250587 100644 --- a/app/services/themes/create.rb +++ b/app/services/themes/create.rb @@ -63,6 +63,8 @@ class Themes::Create step :log_theme_change end + step :refresh_site_setting_cache + private def ensure_remote_themes_are_not_allowlisted @@ -82,4 +84,8 @@ class Themes::Create def log_theme_change(theme:, guardian:) StaffActionLogger.new(guardian.user).log_theme_change(nil, theme) end + + def refresh_site_setting_cache + SiteSetting.refresh!(refresh_site_settings: false, refresh_theme_site_settings: true) + end end diff --git a/app/services/themes/theme_site_setting_manager.rb b/app/services/themes/theme_site_setting_manager.rb new file mode 100644 index 00000000000..2588c8ffc5e --- /dev/null +++ b/app/services/themes/theme_site_setting_manager.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +# Responsible for creating or updating theme site settings, and in the case +# where the value is set to nil or is the same as the site setting default, +# deleting the theme site setting override. +# +# Theme site settings are used to override specific site settings that are +# marked as themeable in site_settings.yml. This allows themes to have a greater +# control over the full site experience. +# +# Theme site settings have an identical schema to SiteSetting. +class Themes::ThemeSiteSettingManager + include Service::Base + + params do + attribute :theme_id, :integer + attribute :name + attribute :value + + validates :theme_id, presence: true + validates :name, presence: true + + after_validation { self.name = self.name.to_sym if self.name.present? } + end + + policy :current_user_is_admin + policy :ensure_setting_is_themeable + model :theme + model :existing_theme_site_setting, optional: true + + transaction do + step :convert_new_value_to_site_setting_values + step :upsert + step :log_change + end + + step :update_site_setting_cache + + private + + def current_user_is_admin(guardian:) + guardian.is_admin? + end + + def ensure_setting_is_themeable(params:) + SiteSetting.themeable[params.name] + end + + def fetch_theme(params:) + Theme.find_by(id: params.theme_id) + end + + def fetch_existing_theme_site_setting(params:, theme:) + theme.theme_site_settings.find_by(name: params.name) + end + + def convert_new_value_to_site_setting_values(params:) + if params.value.nil? + context[:setting_db_value] = nil + context[:setting_data_type] = nil + context[:setting_ruby_value] = nil + return + end + + # This must be done because we want the schema and data of ThemeSiteSetting to reflect + # that of SiteSetting, since they are the same data types and values. + setting_db_value, setting_data_type = + SiteSetting.type_supervisor.to_db_value(params.name, params.value) + setting_ruby_value = + SiteSetting.type_supervisor.to_rb_value(params.name, params.value, setting_data_type) + + context[:setting_db_value] = setting_db_value + context[:setting_data_type] = setting_data_type + context[:setting_ruby_value] = setting_ruby_value + end + + def upsert( + params:, + existing_theme_site_setting:, + theme:, + setting_db_value:, + setting_data_type:, + setting_ruby_value: + ) + setting_record = nil + context[:previous_value] = nil + context[:new_value] = nil + + if existing_theme_site_setting + context[:previous_value] = existing_theme_site_setting.setting_rb_value + setting_record = existing_theme_site_setting + + # If the setting is nil or matches the site setting default, + # then we just update the existing theme site setting to reflect + # this, as insurance against further changes to the site setting + # default value. + if params.value.nil? || setting_ruby_value == SiteSetting.defaults[params.name] + new_db_value, _ = + SiteSetting.type_supervisor.to_db_value(params.name, SiteSetting.defaults[params.name]) + new_ruby_value = SiteSetting.defaults[params.name] + else + new_db_value = setting_db_value + new_ruby_value = setting_ruby_value + end + + existing_theme_site_setting.update!(value: new_db_value) + context[:new_value] = new_ruby_value + else + # If the setting is nil or matches the site setting default, + # then we make a record using the site setting default as + # insurance against further changes to the default value for + # the site setting. + if params.value.nil? || setting_ruby_value == SiteSetting.defaults[params.name] + new_db_value, _ = + SiteSetting.type_supervisor.to_db_value(params.name, SiteSetting.defaults[params.name]) + new_ruby_value = SiteSetting.defaults[params.name] + else + new_db_value = setting_db_value + new_ruby_value = setting_ruby_value + end + + setting_record = + theme.theme_site_settings.create!( + name: params.name, + value: new_db_value, + data_type: setting_data_type, + ) + context[:new_value] = new_ruby_value + end + + context[:theme_site_setting] = setting_record + end + + def log_change(params:, new_value:, previous_value:, theme:, guardian:) + StaffActionLogger.new(guardian.user).log_theme_site_setting_change( + params.name, + previous_value, + new_value, + theme, + ) + end + + def update_site_setting_cache(theme:, params:, new_value:) + # This also sends a MessageBus message to the client for client site settings, + # and a DiscourseEvent for the change. + SiteSetting.change_themeable_site_setting(theme.id, params.name, new_value) + end +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 52269977f38..52f74180c82 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5510,6 +5510,9 @@ en: legal: title: "Legal" header_description: "Configure legal settings, such as terms of service, privacy policy, contact details, and EU-specific considerations" + theme_site_settings: + title: "Theme site settings" + header_description: "Shows all theme site settings and the themes that are currently overriding them" localization: title: "Localization" header_description: "Configure your community’s interface language and other localization options for your members" @@ -5800,6 +5803,15 @@ en: title: "Embedding" header_description: "Discourse has the ability to embed the comments from a topic in a remote site using a Javascript API that creates an IFRAME" + theme_site_settings: + setting: "Setting" + overridden_by: "Overridden by" + default_value: "Default" + overridden_value: "Overridden value: %{value}" + select_setting: "Select a site setting to see current theme values" + add: "Go to theme site settings" + help: "The theme you are currently using is %{currentTheme}. Go to the theme config page to alter theme site settings, or click on a linked theme in the table below to edit its settings." + search: modal_title: "Search everything in admin" title: "Search" @@ -6485,7 +6497,7 @@ en: install: "Install" themes: title: "Themes" - description: "Site-wide themes that change the overall look and feel of your site for all users." + description: "Site-wide themes that change the overall look and feel of your site for all users. Want to check which site settings your themes override? View theme site settings" themes_intro: "Install a new theme to get started, or create your own from scratch using these resources." new_theme: "New theme" back: "Back to themes" @@ -6848,11 +6860,13 @@ en: up_to_date: "Theme is up-to-date, last checked:" has_overwritten_history: "Current theme version no longer exists because the Git history has been overwritten by a force push." add: "Add" - theme_settings: "Theme Settings" - edit_objects_theme_setting: "Objects Setting Editor" - overriden_settings_explanation: "Overridden settings are marked with a dot and have a highlighted color. To reset these settings to the default value, press the reset button next to them." + theme_settings: "Custom theme settings" + theme_site_settings: "Settings the theme can override" + edit_objects_theme_setting: "Objects setting editor" + overriden_settings_explanation: "Changes from the default are marked with a dot and highlight. Click Reset to reset to the default." + overriden_site_settings_explanation: "Site settings the theme can customize. Click Reset to restore the theme’s default. If none is set, the site setting default is used. You can see all site settings being overridden by themes at the theme site settings config page." no_settings: "This theme has no settings." - theme_translations: "Theme Translations" + theme_translations: "Theme translations" empty: "No items" commits_behind: one: "Theme is %{count} commit behind!" @@ -6916,6 +6930,8 @@ en: active_filter: "Active" inactive_filter: "Inactive" updates_available_filter: "Updates Available" + theme_setting_saved: "Theme setting saved!" + theme_site_setting_saved: "Theme site setting saved!" schema: title: "Edit %{name} setting" @@ -7265,6 +7281,7 @@ en: tag_group_destroy: "delete tag group" tag_group_change: "change tag group" delete_associated_accounts: "delete associated accounts" + change_theme_site_setting: "change theme site setting" screened_emails: title: "Screened emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3841b2ce558..91f1e64a0fe 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -84,6 +84,7 @@ en: bad_color_scheme: "Can not update theme, invalid color palette" other_error: "Something went wrong updating theme" ember_selector_error: "Sorry – using #ember or .ember-view CSS selectors is not permitted, because these names are dynamically generated at runtime and will change over time, eventually resulting in broken CSS. Try a different selector." + setting_not_themeable: "The setting '%{name}' is not themeable." import_error: generic: An error occurred while importing that theme upload: "Error creating upload asset: %{name}. %{errors}" diff --git a/config/routes.rb b/config/routes.rb index 1a7088b4640..e1ae12947aa 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -249,6 +249,7 @@ Discourse::Application.routes.draw do get "preview" => "themes#preview" get "translations/:locale" => "themes#get_translations" put "setting" => "themes#update_single_setting" + put "site-setting" => "themes#update_theme_site_setting" get "objects_setting_metadata/:setting_name" => "themes#objects_setting_metadata" put "change-colors" => "themes#change_colors" end @@ -437,6 +438,7 @@ Discourse::Application.routes.draw do put "/logo" => "logo#update" put "/fonts" => "fonts#update" get "colors/:id" => "color_palettes#show" + get "theme-site-settings" => "theme_site_settings#index" get "colors" => "color_palettes#index" resources :flags, only: %i[index new create update destroy] do diff --git a/config/site_settings.yml b/config/site_settings.yml index d7d667c4288..ae6e225793a 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1,18 +1,26 @@ # Available options: # -# default - The default value of the setting. For upload site settings, use the id of the upload seeded in db/fixtures/010_uploads.rb. -# client - Set to true if the javascript should have access to this setting's value. -# refresh - Set to true if clients should refresh when the setting is changed. -# min - For a string setting, the minimum length. For an integer setting, the minimum value. -# max - For a string setting, the maximum length. For an integer setting, the maximum value. -# regex - A regex that the value must match. -# validator - The name of the class that will be use to validate the value of the setting. -# allow_any - For choice settings allow items not specified in the choice list (default true) -# secret - Set to true if input type should be password and value needs to be scrubbed from logs (default false). -# enum - The setting has a fixed set of allowed values, and only one can be chosen. -# Set to the class name that defines the set. -# locale_default - A hash which overrides according to `SiteSetting.default_locale`. -# The key should be as the same as possible value of default_locale. +# default - The default value of the setting. For upload site settings, use the id of the upload seeded +# in db/fixtures/010_uploads.rb. +# client - Set to true if the javascript should have access to this setting's value. +# refresh - Set to true if clients should refresh when the setting is changed. +# min - For a string setting, the minimum length. For an integer setting, the minimum value. +# max - For a string setting, the maximum length. For an integer setting, the maximum value. +# regex - A regex that the value must match. +# validator - The name of the class that will be use to validate the value of the setting. +# allow_any - For choice settings allow items not specified in the choice list (default true) +# secret - Set to true if input type should be password and value needs to be scrubbed from logs (default false). +# enum - The setting has a fixed set of allowed values, and only one can be chosen. +# Set to the class name that defines the set. +# locale_default - A hash which overrides according to `SiteSetting.default_locale`. +# The key should be as the same as possible value of default_locale. +# mandatory_values - A list of mandatory values that must be included in the setting, these cannot be changed or removed +# in the UI. +# requires_confirmation - A string that indicates if the setting requires confirmation before it can be changed. +# Only valid value here is "simple" which will display a confirmation dialog when the setting +# is changed. +# themeable - Set to true if themes can override this site setting. Generally, only client side affecting settings +# that change the UI should be themeable, and try limit to simple setting types like bool, list, integer, enum. # # # type: email - Must be a valid email address. @@ -3218,6 +3226,7 @@ search: client: true type: enum enum: "SearchExperienceSiteSetting" + themeable: true uncategorized: version_checks: @@ -3636,6 +3645,7 @@ uncategorized: enable_welcome_banner: client: true default: true + themeable: true area: "interface" user_preferences: diff --git a/db/migrate/20250409035119_add_theme_site_setting_table.rb b/db/migrate/20250409035119_add_theme_site_setting_table.rb new file mode 100644 index 00000000000..1d4c56c7629 --- /dev/null +++ b/db/migrate/20250409035119_add_theme_site_setting_table.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +class AddThemeSiteSettingTable < ActiveRecord::Migration[7.2] + def change + create_table :theme_site_settings do |t| + t.integer :theme_id, null: false + t.string :name, null: false + t.integer :data_type, null: false + t.text :value + t.timestamps + + t.index :theme_id + t.index %i[theme_id name], unique: true + end + end +end diff --git a/db/migrate/20250714010001_backfill_themeable_site_settings.rb b/db/migrate/20250714010001_backfill_themeable_site_settings.rb new file mode 100644 index 00000000000..e0ba7e0ca56 --- /dev/null +++ b/db/migrate/20250714010001_backfill_themeable_site_settings.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true +class BackfillThemeableSiteSettings < ActiveRecord::Migration[7.2] + def up + initial_themeable_site_settings = %w[enable_welcome_banner search_experience] + + initial_themeable_site_settings.each do |setting| + db_data_type, db_value = + DB.query_single("SELECT data_type, value FROM site_settings WHERE name = ?", setting) + + # If there is no value in the DB, it means the admin hasn't changed it from the default, + # and theme site settings will just use the default value. + next if db_value.nil? + + theme_ids = DB.query_single("SELECT id FROM themes WHERE NOT component") + + theme_ids.each do |theme_id| + # ThemeSiteSetting has an identical schema to SiteSetting, so we can use the same values + # and data types. + DB.exec( + "INSERT INTO theme_site_settings (name, data_type, value, theme_id, created_at, updated_at) + VALUES (:setting, :data_type, :value, :theme_id, NOW(), NOW()) + ON CONFLICT (name, theme_id) DO NOTHING", + setting:, + data_type: db_data_type, + value: db_value, + theme_id:, + ) + end + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/application_layout_preloader.rb b/lib/application_layout_preloader.rb index f5a659581af..00c62eaa8e7 100644 --- a/lib/application_layout_preloader.rb +++ b/lib/application_layout_preloader.rb @@ -112,6 +112,7 @@ class ApplicationLayoutPreloader check_readonly_mode if @readonly_mode.nil? @preloaded["site"] = Site.json_for(@guardian) @preloaded["siteSettings"] = SiteSetting.client_settings_json + @preloaded["themeSiteSettingOverrides"] = SiteSetting.theme_site_settings_json(@theme_id) @preloaded["customHTML"] = custom_html_json @preloaded["banner"] = banner_json @preloaded["customEmoji"] = custom_emoji diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index b27e610c156..31de7f402ba 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -5,6 +5,7 @@ module SiteSettingExtension include HasSanitizableFields SiteSettingChangeResult = Struct.new(:previous_value, :new_value) + InvalidSettingAccess = Class.new(StandardError) delegate :description, :keywords, :placeholder, :humanized_name, to: SiteSettings::LabelFormatter @@ -79,6 +80,11 @@ module SiteSettingExtension @containers[provider.current_site] ||= {} end + def theme_site_settings + @theme_site_settings ||= {} + @theme_site_settings[provider.current_site] ||= {} + end + def defaults @defaults ||= SiteSettings::DefaultsProvider.new(self) end @@ -91,6 +97,10 @@ module SiteSettingExtension @categories ||= {} end + def themeable + @themeable ||= {} + end + def areas @areas ||= {} end @@ -153,18 +163,30 @@ module SiteSettingExtension &.first end - def settings_hash - result = {} + def theme_site_settings_json(theme_id) + key = SiteSettingExtension.theme_site_settings_cache_key(theme_id) + json = + Discourse + .cache + .fetch(key, expires_in: 30.minutes) { theme_site_settings_json_uncached(theme_id) } + Rails.logger.error("Nil theme_site_settings_json from the cache for '#{key}'") if json.nil? + json || "" + rescue => e + Rails.logger.error("Error while retrieving theme_site_settings_json: #{e.message}") + "" + end - defaults.all.keys.each do |s| - result[s] = if deprecated_settings.include?(s.to_s) - public_send(s, warn: false).to_s - else - public_send(s).to_s - end - end + def setting_metadata_hash(setting) + setting_hash = { + setting:, + default: SiteSetting.defaults[setting], + description: SiteSetting.description(setting), + humanized_name: SiteSetting.humanized_name(setting), + }.merge(type_supervisor.type_hash(setting)) + end - result + def themeable_site_settings + themeable.select { |_, value| value }.keys end def client_settings_json @@ -180,19 +202,30 @@ module SiteSettingExtension def client_settings_json_uncached MultiJson.dump( Hash[ - *@client_settings.flat_map do |name| - value = - if deprecated_settings.include?(name.to_s) - public_send(name, warn: false) - else - public_send(name) - end - type = type_supervisor.get_type(name) - value = value.to_s if type == :upload - value = value.map(&:to_s).join("|") if type == :uploaded_image_list + *@client_settings + .flat_map do |name| + # Themeable site settings require a theme ID, which we do not always + # have when loading client site settings. They are excluded here, + # to get them use theme_site_settings_json(:theme_id) + next if themeable[name] - [name, value] - end + value = + if deprecated_settings.include?(name.to_s) + public_send(name, warn: false) + else + public_send(name) + end + + type = type_supervisor.get_type(name) + if type == :upload + value = value.to_s + elsif type == :uploaded_image_list + value = value.map(&:to_s).join("|") + end + + [name, value] + end + .compact ], ) rescue => e @@ -200,6 +233,10 @@ module SiteSettingExtension nil end + def theme_site_settings_json_uncached(theme_id) + MultiJson.dump(theme_site_settings[theme_id]) + end + # Retrieve all settings def all_settings( include_hidden: false, @@ -262,6 +299,11 @@ 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 @@ -291,6 +333,7 @@ module SiteSettingExtension placeholder: placeholder(s), mandatory_values: mandatory_values[s], requires_confirmation: requires_confirmation_settings[s], + themeable: themeable[s], ) opts.merge!(type_hash) end @@ -324,39 +367,63 @@ module SiteSettingExtension "client_settings_json_#{Discourse.git_version}" end - # refresh all the site settings - def refresh! + def self.theme_site_settings_cache_key(theme_id) + # NOTE: we use the git version in the key to ensure + # that we don't end up caching the incorrect version + # in cases where we are cycling unicorns + "theme_site_settings_json_#{theme_id}__#{Discourse.git_version}" + end + + # Refresh all the site settings and theme site settings + def refresh!(refresh_site_settings: true, refresh_theme_site_settings: true) mutex.synchronize do ensure_listen_for_changes - new_hash = - Hash[ - *( - defaults - .db_all - .map do |s| - [s.name.to_sym, type_supervisor.to_rb_value(s.name, s.value, s.data_type)] - end - .to_a - .flatten - ) - ] + if refresh_site_settings + new_hash = + Hash[ + *( + provider + .all + .map do |s| + [s.name.to_sym, type_supervisor.to_rb_value(s.name, s.value, s.data_type)] + end + .to_a + .flatten + ) + ] - defaults_view = defaults.all(new_hash[:default_locale]) + defaults_view = defaults.all(new_hash[:default_locale]) - # add locale default and defaults based on default_locale, cause they are cached - new_hash = defaults_view.merge!(new_hash) + # add locale default and defaults based on default_locale, cause they are cached + new_hash = defaults_view.merge!(new_hash) - # add shadowed - shadowed_settings.each { |ss| new_hash[ss] = GlobalSetting.public_send(ss) } + # add shadowed + shadowed_settings.each { |ss| new_hash[ss] = GlobalSetting.public_send(ss) } - changes, deletions = diff_hash(new_hash, current) + changes, deletions = diff_hash(new_hash, current) - changes.each { |name, val| current[name] = val } - deletions.each { |name, _| current[name] = defaults_view[name] } - uploads.clear + changes.each { |name, val| current[name] = val } + deletions.each { |name, _| current[name] = defaults_view[name] } + uploads.clear + end - clear_cache! + if refresh_theme_site_settings + new_theme_site_settings = ThemeSiteSetting.generate_theme_map + + theme_site_setting_changes, theme_site_setting_deletions = + diff_hash(new_theme_site_settings, theme_site_settings) + theme_site_setting_changes.each do |theme_id, settings| + theme_site_settings[theme_id] ||= {} + theme_site_settings[theme_id].merge!(settings) + end + theme_site_setting_deletions.each { |theme_id, _| theme_site_settings.delete(theme_id) } + end + + clear_cache!( + expire_theme_site_setting_cache: + ThemeSiteSetting.can_access_db? && refresh_theme_site_settings, + ) end end @@ -390,7 +457,29 @@ module SiteSettingExtension ensure_listen_for_changes end + def raise_invalid_setting_access(setting_name) + raise SiteSettingExtension::InvalidSettingAccess.new( + "#{setting_name} cannot be changed like this because it is a themeable setting. Instead, use the ThemeSiteSettingManager service to manage themeable site settings.", + ) + end + + ## + # Removes an override for a setting, reverting it to the default value. + # This method is only called manually usually, more often than not + # setting overrides are removed in database migrations. + # + # Here we also handle notifying the UI of the change in the case + # of theme site settings and clearing relevant caches, and triggering + # server-side events for changed settings. + # + # Themeable site settings cannot be removed this way, they must be + # changed via the ThemeSiteSetting model. + # + # @param name [Symbol] the name of the setting + # @param val [Any] the value to set def remove_override!(name) + raise_invalid_setting_access(name) if themeable[name] + old_val = current[name] provider.destroy(name) current[name] = defaults.get(name, default_locale) @@ -404,7 +493,36 @@ module SiteSettingExtension end end + ## + # Adds an override, which is to say a database entry for the setting + # instead of using the default. + # + # The `set`, `set_and_log`, and `setting_name=` methods all call + # this method. Its opposite is remove_override!. + # + # Here we also handle notifying the UI of the change in the case + # of theme site settings and clearing relevant caches, and triggering + # server-side events for changed settings. + # + # Themeable site settings cannot be changed this way, they must be + # changed via the ThemeSiteSetting model. + # + # @param name [Symbol] the name of the setting + # @param val [Any] the value to set + # + # @example + # SiteSetting.add_override!(:site_description, "My awesome forum") + # + # @raise [SiteSettingExtension::InvalidSettingAccess] if the setting is themeable + # (themeable settings must be changed via ThemeSiteSetting model) + # + # @note When called from the Rails console, this method automatically logs the change + # with the system user. + # + # @see remove_override! for removing an override and reverting to default value def add_override!(name, val) + raise_invalid_setting_access(name) if themeable[name] + old_val = current[name] val, type = type_supervisor.to_db_value(name, val) @@ -423,7 +541,7 @@ module SiteSettingExtension return if current[name] == old_val clear_uploads_cache(name) - notify_clients!(name) if client_settings.include? name + notify_clients!(name) if client_settings.include?(name) clear_cache! if defined?(Rails::Console) @@ -435,12 +553,52 @@ module SiteSettingExtension DiscourseEvent.trigger(:site_setting_changed, name, old_val, current[name]) end + # Updates a theme-specific site setting value in memory and notifies observers. + # + # This method is used to change site settings that are marked as "themeable", + # which means they can have different values per theme. Unlike `add_override!`, + # the database isn't touched here. + # + # @param theme_id [Integer] The ID of the theme to update the setting for + # @param name [String, Symbol] The name of the site setting to change + # @param val [Object] The new "ruby" value for the site setting + # + # @example + # SiteSetting.change_themeable_site_setting(5, "enable_welcome_banner", false) + # + # @note Unlike regular site settings which use add_override!, themeable settings + # should be changed via the ThemeSiteSettingManager service. + # + # @see ThemeSiteSettingManager service for the higher-level implementation that handles + # database persistence and logging. + def change_themeable_site_setting(theme_id, name, val) + name = name.to_sym + + theme_site_settings[theme_id] ||= {} + old_val = theme_site_settings[theme_id][name] + theme_site_settings[theme_id][name] = val + + notify_clients!(name, theme_id: theme_id) if client_settings.include?(name) + + clear_cache!(expire_theme_site_setting_cache: true) + + DiscourseEvent.trigger(:theme_site_setting_changed, name, old_val, val) + end + def notify_changed! MessageBus.publish("/site_settings", process: process_id) end - def notify_clients!(name) - MessageBus.publish("/client_settings", name: name, value: self.public_send(name)) + def notify_clients!(name, scoped_to = nil) + MessageBus.publish( + "/client_settings", + name: name, + # default_locale is a special case, it is not themeable and we define + # a custom getter for it, so we can just use the normal getter + value: + name.to_s == "default_locale" ? self.public_send(name) : self.public_send(name, scoped_to), + scoped_to: scoped_to, + ) end def requires_refresh?(name) @@ -474,6 +632,8 @@ module SiteSettingExtension def set(name, value, options = nil) if has_setting?(name) + raise_invalid_setting_access(name) if themeable[name] + value = filter_value(name, value) if options self.public_send("#{name}=", value, options) @@ -490,6 +650,8 @@ module SiteSettingExtension def set_and_log(name, value, user = Discourse.system_user, detailed_message = nil) if has_setting?(name) + raise_invalid_setting_access(name) if themeable[name] + prev_value = public_send(name) return if prev_value == value set(name, value) @@ -503,9 +665,19 @@ module SiteSettingExtension end end - def get(name) + def get(name, scoped_to = nil) if has_setting?(name) - self.public_send(name) + if themeable[name] + if scoped_to.nil? || !scoped_to.key?(:theme_id) || scoped_to[:theme_id].nil? + raise SiteSettingExtension::InvalidSettingAccess.new( + "#{name} requires a theme_id because it is themeable", + ) + else + self.public_send(name, scoped_to) + end + else + self.public_send(name) + end else raise Discourse::InvalidParameters.new( I18n.t("errors.site_settings.invalid_site_setting", name: name), @@ -535,8 +707,9 @@ module SiteSettingExtension protected - def clear_cache! + def clear_cache!(expire_theme_site_setting_cache: false) Discourse.cache.delete(SiteSettingExtension.client_settings_cache_key) + Theme.expire_site_setting_cache! if expire_theme_site_setting_cache Site.clear_anon_cache! end @@ -578,7 +751,15 @@ module SiteSettingExtension clean_name = name.to_s.sub("?", "").to_sym if type_supervisor.get_type(name) == :uploaded_image_list - define_singleton_method clean_name do + define_singleton_method clean_name do |scoped_to = nil| + if themeable[clean_name] + if scoped_to.nil? || !scoped_to.key?(:theme_id) || scoped_to[:theme_id].nil? + raise SiteSettingExtension::InvalidSettingAccess.new( + "#{clean_name} requires a theme_id because it is themeable", + ) + end + end + uploads_list = uploads[name] return uploads_list if uploads_list @@ -594,7 +775,15 @@ module SiteSettingExtension uploads[name] = uploads_list if uploads_list end elsif type_supervisor.get_type(name) == :upload - define_singleton_method clean_name do + define_singleton_method clean_name do |scoped_to = nil| + if themeable[clean_name] + if scoped_to.nil? || !scoped_to.key?(:theme_id) || scoped_to[:theme_id].nil? + raise SiteSettingExtension::InvalidSettingAccess.new( + "#{clean_name} requires a theme_id because it is themeable", + ) + end + end + upload = uploads[name] return upload if upload @@ -611,13 +800,29 @@ module SiteSettingExtension end end else - define_singleton_method clean_name do + define_singleton_method clean_name do |scoped_to = nil| + if themeable[clean_name] + if scoped_to.nil? || !scoped_to.key?(:theme_id) || scoped_to[:theme_id].nil? + raise SiteSettingExtension::InvalidSettingAccess.new( + "#{clean_name} requires a theme_id because it is themeable", + ) + end + + # If the theme hasn't overridden any theme site settings (or changed defaults) + # then we will just fall back further down bellow to the current site setting value. + settings_overriden_for_theme = theme_site_settings[scoped_to[:theme_id]] + if settings_overriden_for_theme && settings_overriden_for_theme.key?(clean_name) + return settings_overriden_for_theme[clean_name] + end + end + if plugins[name] plugin = Discourse.plugins_by_name[plugins[name]] return false if !plugin.configurable? && plugin.enabled_site_setting == name end refresh! if current[name].nil? + value = current[name] if mandatory_values[name] @@ -642,17 +847,19 @@ module SiteSettingExtension list_type = type_supervisor.get_list_type(name) if %w[simple compact].include?(list_type) || list_type.nil? - define_singleton_method("#{clean_name}_map") do - self.public_send(clean_name).to_s.split("|") + define_singleton_method("#{clean_name}_map") do |scoped_to = nil| + self.public_send(clean_name, scoped_to).to_s.split("|") end end end - define_singleton_method "#{clean_name}?" do - self.public_send clean_name + define_singleton_method "#{clean_name}?" do |scoped_to = nil| + self.public_send(clean_name, scoped_to) end define_singleton_method "#{clean_name}=" do |val| + raise_invalid_setting_access(clean_name) if themeable[clean_name] + add_override!(name, val) end end @@ -703,6 +910,8 @@ module SiteSettingExtension categories[name] = opts[:category] || :uncategorized + themeable[name] = opts[:themeable] ? true : false + if opts[:area] split_areas = opts[:area].split("|") if split_areas.any? { |area| !SiteSetting.valid_areas.include?(area) } diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 1ad5b6bc0e6..e83f12656b9 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -142,7 +142,7 @@ module SiteSettings::DeprecatedSettings end end - define_singleton_method old_setting do |warn: true| + define_singleton_method old_setting do |scoped_to = nil, warn: true| if warn Discourse.deprecate( "`SiteSetting.#{old_setting}` has been deprecated. Please use `SiteSetting.#{new_setting}` instead.", @@ -153,13 +153,13 @@ module SiteSettings::DeprecatedSettings if OVERRIDE_TL_GROUP_SETTINGS.include?(old_setting) self.public_send("_group_to_tl_#{old_setting}") else - self.public_send(override ? new_setting : "_#{old_setting}") + self.public_send(override ? new_setting : "_#{old_setting}", scoped_to) end end SiteSetting.singleton_class.alias_method(:"_#{old_setting}?", :"#{old_setting}?") if !override - define_singleton_method "#{old_setting}?" do |warn: true| + define_singleton_method "#{old_setting}?" do |scoped_to = nil, warn: true| if warn Discourse.deprecate( "`SiteSetting.#{old_setting}?` has been deprecated. Please use `SiteSetting.#{new_setting}?` instead.", @@ -167,7 +167,7 @@ module SiteSettings::DeprecatedSettings ) end - self.public_send("#{override ? new_setting : "_" + old_setting}?") + self.public_send("#{override ? new_setting : "_" + old_setting}?", scoped_to) end SiteSetting.singleton_class.alias_method(:"_#{old_setting}=", :"#{old_setting}=") if !override diff --git a/lib/svg_sprite.rb b/lib/svg_sprite.rb index 4d79490e0b7..3816ddaaaec 100644 --- a/lib/svg_sprite.rb +++ b/lib/svg_sprite.rb @@ -488,9 +488,13 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL # includes svg_icon_subset and any settings containing _icon (incl. plugin settings) site_setting_icons = [] - SiteSetting.settings_hash.select do |key, value| - site_setting_icons |= value.split("|") if key.to_s.include?("_icon") && String === value - end + SiteSetting + .all_settings(include_locale_setting: false) + .select do |setting| + site_setting_icons |= setting[:value].split("|") if setting[:setting].to_s.include?( + "_icon", + ) && String === setting[:value] + end site_setting_icons end diff --git a/lib/theme_site_setting_resolver.rb b/lib/theme_site_setting_resolver.rb new file mode 100644 index 00000000000..200d809a84f --- /dev/null +++ b/lib/theme_site_setting_resolver.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +# Responsible for resolving all possible theme site settings for +# a given theme, including details about the setting type, valid values, +# and description, for use in the theme editor. +# +# The value of the setting will either be the value stored in the +# ThemeSiteSetting table, or the default value of the site setting. +# +# Example output for a single setting, there will be an array of these: +# +# { +# :setting=>:search_experience, +# :default=>"search_icon", +# :description=>"The default position and appearance of search on desktop devices", +# :type=>"enum", +# :valid_values=>[ +# { +# :name=>"search.experience.search_field", :value=>"search_field" +# }, { +# :name=>"search.experience.search_icon", :value=>"search_icon" +# } +# ], +# :translate_names=>true, +# :value=>"search_icon" +# } +class ThemeSiteSettingResolver + attr_reader :theme + + def initialize(theme:) + @theme = theme + end + + def resolved_theme_site_settings + stored_theme_site_settings = + theme.theme_site_settings.to_a.select do |setting| + setting.name.to_sym.in?(SiteSetting.themeable_site_settings) + end + + SiteSetting + .themeable_site_settings + .each_with_object([]) do |setting, settings| + setting_hash = SiteSetting.setting_metadata_hash(setting) + + # If the setting has been saved in the DB, it means the theme has changed + # it in about.json on import, or the admin has changed it manually later on. + stored_setting = + stored_theme_site_settings.find { |db_setting| db_setting.name == setting.to_s } + if !stored_setting.nil? + setting_hash[:value] = stored_setting.setting_rb_value + else + # Otherwise if there is no value in the DB, we use the default value of + # the site setting, since we do not insert a DB value if the about.json + # value is the same as the default site setting value. + setting_hash[:value] = setting_hash[:default] + end + + settings << setting_hash + end + .sort_by { |setting_hash| setting_hash[:setting] } + end +end diff --git a/spec/db/migrate/.gitkeep b/spec/db/migrate/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/spec/db/migrate/20250714010001_backfill_themeable_site_settings_spec.rb b/spec/db/migrate/20250714010001_backfill_themeable_site_settings_spec.rb new file mode 100644 index 00000000000..51b34629875 --- /dev/null +++ b/spec/db/migrate/20250714010001_backfill_themeable_site_settings_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require Rails.root.join("db/migrate/20250714010001_backfill_themeable_site_settings.rb") + +RSpec.describe BackfillThemeableSiteSettings do + fab!(:theme_1) { Fabricate(:theme) } + fab!(:theme_2) { Fabricate(:theme) } + fab!(:theme_3) { Fabricate(:theme, component: true) } + + before do + @original_verbose = ActiveRecord::Migration.verbose + ActiveRecord::Migration.verbose = false + end + + after { ActiveRecord::Migration.verbose = @original_verbose } + + it "works" do + DB.exec( + "INSERT INTO site_settings (name, data_type, value, created_at, updated_at) + VALUES ('enable_welcome_banner', :data_type, :value, NOW(), NOW())", + data_type: SiteSettings::TypeSupervisor.types[:bool], + value: "t", + ) + + BackfillThemeableSiteSettings.new.up + + # This count includes the system themes theme, but not the component. + expect(ThemeSiteSetting.where(name: "enable_welcome_banner").count).to eq(4) + + # Don't insert any record if the site setting was never changed from the default. + expect(ThemeSiteSetting.where(name: "search_experience").count).to eq(0) + + # Make sure the data type + value are the same as the site setting. + expect( + ThemeSiteSetting.find_by(name: "enable_welcome_banner", theme_id: theme_1.id), + ).to have_attributes(data_type: SiteSettings::TypeSupervisor.types[:bool], value: "t") + end +end diff --git a/spec/fabricators/theme_site_setting_fabricator.rb b/spec/fabricators/theme_site_setting_fabricator.rb new file mode 100644 index 00000000000..eabc1804e54 --- /dev/null +++ b/spec/fabricators/theme_site_setting_fabricator.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# NOTE: For most cases, :theme_site_setting_with_service is the better choice to use, +# since it updates SiteSetting and other things properly. +Fabricator(:theme_site_setting) do + theme { Theme.find_default } + + # NOTE: Only site settings with `themeable: true` are valid here. + name { "enable_welcome_banner" } + value { false } + + before_create do |theme_site_setting| + setting_db_value, setting_data_type = + SiteSetting.type_supervisor.to_db_value( + theme_site_setting.name.to_sym, + theme_site_setting.value, + ) + theme_site_setting.value = setting_db_value + theme_site_setting.data_type = setting_data_type + end +end + +Fabricator(:theme_site_setting_with_service, class_name: "Themes::ThemeSiteSettingManager") do + # NOTE: Only site settings with `themeable: true` are valid here. + transient :theme, :name, :value + + initialize_with do |transients| + theme = transients[:theme] || Theme.find_default + result = + resolved_class.call( + params: { + theme_id: theme.id, + name: transients[:name], + value: transients[:value], + }, + guardian: Discourse.system_user.guardian, + ) + + if result.failure? + raise RSpec::Expectations::ExpectationNotMetError.new( + "Service `#{resolved_class}` failed, see below for step details:\n\n" + + result.inspect_steps, + ) + end + + result.theme_site_setting + end +end diff --git a/spec/lib/site_setting_extension_spec.rb b/spec/lib/site_setting_extension_spec.rb index 776b4c22aeb..de35ecf8db2 100644 --- a/spec/lib/site_setting_extension_spec.rb +++ b/spec/lib/site_setting_extension_spec.rb @@ -984,6 +984,65 @@ RSpec.describe SiteSettingExtension do end end + describe "themeable settings" do + fab!(:theme_1) { Fabricate(:theme) } + fab!(:theme_2) { Fabricate(:theme) } + fab!(:tss_1) do + Fabricate( + :theme_site_setting_with_service, + name: "enable_welcome_banner", + value: false, + theme: theme_1, + ) + end + fab!(:tss_2) do + Fabricate( + :theme_site_setting_with_service, + name: "search_experience", + value: "search_field", + theme: theme_2, + ) + end + + it "has the site setting default values when there are no theme site settings for the theme" do + SiteSetting.refresh! + expect(SiteSetting.theme_site_settings[theme_1.id][:search_experience]).to eq("search_icon") + expect(SiteSetting.theme_site_settings[theme_2.id][:enable_welcome_banner]).to eq(true) + end + + it "returns true for settings that are themeable" do + expect(SiteSetting.themeable[:enable_welcome_banner]).to eq(true) + end + + it "returns false for settings that are not themeable" do + expect(SiteSetting.themeable[:title]).to eq(false) + end + + it "caches the theme site setting values on a per theme basis" do + SiteSetting.refresh! + expect(SiteSetting.theme_site_settings[theme_1.id][:enable_welcome_banner]).to eq(false) + expect(SiteSetting.theme_site_settings[theme_2.id][:search_experience]).to eq("search_field") + end + + it "overrides the site setting value with the theme site setting" do + SiteSetting.create!( + name: "enable_welcome_banner", + data_type: SiteSettings::TypeSupervisor.types[:bool], + value: "t", + ) + SiteSetting.create!( + name: "search_experience", + data_type: SiteSettings::TypeSupervisor.types[:enum], + value: SiteSetting.type_supervisor.to_db_value(:search_experience, "search_icon"), + ) + SiteSetting.refresh! + expect(SiteSetting.enable_welcome_banner(theme_id: theme_1.id)).to eq(false) + expect(SiteSetting.enable_welcome_banner(theme_id: theme_2.id)).to eq(true) + expect(SiteSetting.search_experience(theme_id: theme_1.id)).to eq("search_icon") + expect(SiteSetting.search_experience(theme_id: theme_2.id)).to eq("search_field") + end + end + describe "_map extension for list settings" do it "handles splitting group_list settings" do SiteSetting.personal_message_enabled_groups = "1|2" diff --git a/spec/lib/theme_site_setting_resolver_spec.rb b/spec/lib/theme_site_setting_resolver_spec.rb new file mode 100644 index 00000000000..f6582a93518 --- /dev/null +++ b/spec/lib/theme_site_setting_resolver_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +RSpec.describe ThemeSiteSettingResolver do + fab!(:theme) + + subject(:resolver) { described_class.new(theme: theme) } + + describe "#resolved_theme_site_settings" do + let(:themeable_setting) { :enable_welcome_banner } + let(:default_value) { SiteSetting.defaults[themeable_setting] } + + it "returns themeable settings" do + result = resolver.resolved_theme_site_settings + expect(result.map { |s| s[:setting] }).to match_array(SiteSetting.themeable_site_settings) + end + + it "returns settings in alphabetical order" do + result = resolver.resolved_theme_site_settings + expect(result.map { |r| r[:setting] }).to eq(SiteSetting.themeable_site_settings.sort) + end + + it "includes metadata for each setting" do + result = resolver.resolved_theme_site_settings.first + expect(result).to include( + setting: themeable_setting, + default: default_value, + description: I18n.t("site_settings.#{themeable_setting}"), + type: "bool", + ) + end + + context "when theme has not overridden any settings" do + it "uses the default site setting value" do + result = resolver.resolved_theme_site_settings.find { |s| s[:setting] == themeable_setting } + expect(result[:value]).to eq(default_value) + expect(result[:default]).to eq(default_value) + end + end + + context "when theme has overridden settings" do + let(:overridden_value) { false } + + before do + # Create a theme site setting override with a different value than default + Fabricate( + :theme_site_setting, + theme: theme, + name: themeable_setting.to_s, + value: overridden_value, + data_type: SiteSetting.types[:enum], + ) + end + + it "uses the overridden value" do + result = resolver.resolved_theme_site_settings.find { |s| s[:setting] == themeable_setting } + expect(result[:value]).to eq(overridden_value) + expect(result[:default]).to eq(default_value) + end + end + + context "with enum type settings" do + let(:themeable_setting) { :search_experience } + + it "includes valid_values and translate_names" do + result = resolver.resolved_theme_site_settings.find { |s| s[:setting] == themeable_setting } + expect(result[:valid_values]).to be_an(Array) + expect(result).to include(:translate_names) + end + end + end +end diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index 5cc355b4102..ecbcf09e992 100644 --- a/spec/models/remote_theme_spec.rb +++ b/spec/models/remote_theme_spec.rb @@ -467,6 +467,140 @@ RSpec.describe RemoteTheme do expect(screenshot_2.upload.original_filename).to eq("dark.jpeg") end end + + describe "theme site settings" do + it "creates theme site settings defined in about.json" do + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "enable_welcome_banner" => false }) + .to_json, + ) + + theme = RemoteTheme.import_theme(initial_repo_url) + expect(theme.theme_site_settings.count).to eq(1) + expect(theme.theme_site_settings.first.name).to eq("enable_welcome_banner") + expect(theme.theme_site_settings.first.value).to eq("f") + expect(theme.theme_site_settings.first.data_type).to eq(SiteSetting.types[:bool]) + end + + it "does not remove theme site settings that are no longer in about.json" do + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge( + "theme_site_settings" => { + "enable_welcome_banner" => false, + "search_experience" => "search_field", + }, + ) + .to_json, + ) + + theme = RemoteTheme.import_theme(initial_repo_url) + expect(theme.theme_site_settings.count).to eq(2) + + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "enable_welcome_banner" => false }) + .to_json, + ) + + theme.remote_theme.update_from_remote + theme.reload + + expect(theme.theme_site_settings.count).to eq(2) + expect(theme.theme_site_settings.first.name).to eq("enable_welcome_banner") + expect(theme.theme_site_settings.first.value).to eq("f") + expect(theme.theme_site_settings.second.name).to eq("search_experience") + expect(theme.theme_site_settings.second.value).to eq("search_field") + end + + it "ignores non-themeable site settings" do + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "min_admin_password_length" => 20 }) + .to_json, + ) + + theme = nil + + theme = RemoteTheme.import_theme(initial_repo_url) + + expect(theme.theme_site_settings.count).to eq(0) + end + + # TODO (martin) Hard to test this without a better example...we don't have any + # theme site settings that are an enum with > 2 values. + xit "does not override user modified theme site settings" do + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "search_experience" => "search_field" }) + .to_json, + ) + + theme = RemoteTheme.import_theme(initial_repo_url) + expect(theme.theme_site_settings.first.value).to eq("search_field") + + theme.theme_site_settings.first.update!(value: "search_icon") + + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "search_experience" => "search_field" }) + .to_json, + ) + + theme.remote_theme.update_from_remote + theme.reload + + expect(theme.theme_site_settings.first.value).to eq("search_icon") + end + + it "does not update the existing theme site setting" do + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "search_experience" => "search_field" }) + .to_json, + ) + + theme = RemoteTheme.import_theme(initial_repo_url) + expect(theme.theme_site_settings.first.name).to eq("search_experience") + expect(theme.theme_site_settings.first.value).to eq("search_field") + + add_to_git_repo( + initial_repo, + "about.json" => + JSON + .parse(about_json) + .merge("theme_site_settings" => { "search_experience" => "search_icon" }) + .to_json, + ) + + theme.remote_theme.update_from_remote + theme.reload + + expect(theme.theme_site_settings.first.value).to eq("search_field") + end + end end let(:github_repo) do diff --git a/spec/models/theme_site_setting_spec.rb b/spec/models/theme_site_setting_spec.rb new file mode 100644 index 00000000000..7a67bc990b9 --- /dev/null +++ b/spec/models/theme_site_setting_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +RSpec.describe ThemeSiteSetting do + fab!(:theme_1) { Fabricate(:theme) } + fab!(:theme_2) { Fabricate(:theme) } + fab!(:theme_site_setting_1) do + Fabricate( + :theme_site_setting_with_service, + theme: theme_1, + name: "enable_welcome_banner", + value: false, + ) + end + fab!(:theme_site_setting_2) do + Fabricate( + :theme_site_setting_with_service, + theme: theme_1, + name: "search_experience", + value: "search_field", + ) + end + + describe ".generate_theme_map" do + it "returns a map of theme ids mapped to theme site settings, using site setting defaults if the setting records do not exist" do + expect(ThemeSiteSetting.generate_theme_map).to include( + { + theme_1.id => { + enable_welcome_banner: false, + search_experience: "search_field", + }, + theme_2.id => { + enable_welcome_banner: true, + search_experience: "search_icon", + }, + }, + ) + end + + context "when skipping redis" do + before { GlobalSetting.skip_redis = true } + after { GlobalSetting.skip_redis = false } + + it "returns {}" do + expect(ThemeSiteSetting.generate_theme_map).to eq({}) + end + end + + context "when skipping db" do + before { GlobalSetting.skip_db = true } + after { GlobalSetting.skip_db = false } + + it "returns {}" do + expect(ThemeSiteSetting.generate_theme_map).to eq({}) + end + end + + context "when the table doesn't exist yet, in case of migrations" do + before do + ActiveRecord::Base + .connection + .stubs(:table_exists?) + .with(ThemeSiteSetting.table_name) + .returns(false) + end + + it "returns {}" do + expect(ThemeSiteSetting.generate_theme_map).to eq({}) + end + end + end +end diff --git a/spec/multisite/site_settings_spec.rb b/spec/multisite/site_settings_spec.rb index 7aedfe28d89..bf1fe826e85 100644 --- a/spec/multisite/site_settings_spec.rb +++ b/spec/multisite/site_settings_spec.rb @@ -62,4 +62,24 @@ RSpec.describe "Multisite SiteSettings", type: :multisite do test_multisite_connection("second") { SiteSetting.refresh! } end end + + describe "themeable site settings" do + describe "#enable_welcome_banner" do + it "should return the right value" do + test_multisite_connection("default") do + expect(SiteSetting.enable_welcome_banner(theme_id: Theme.find_default.id)).to eq(true) + end + + test_multisite_connection("second") do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) + + expect(SiteSetting.enable_welcome_banner(theme_id: Theme.find_default.id)).to eq(false) + end + + test_multisite_connection("default") do + expect(SiteSetting.enable_welcome_banner(theme_id: Theme.find_default.id)).to eq(true) + end + end + end + end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index b9877ca7b95..0cb856a8427 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -319,6 +319,9 @@ RSpec.configure do |config| Discourse.current_user_provider = TestCurrentUserProvider Discourse::Application.load_tasks + ThemeField.delete_all + ThemeSiteSetting.delete_all + Theme.expire_site_setting_cache! SiteSetting.refresh! # Rebase defaults diff --git a/spec/requests/admin/theme_site_settings_controller_spec.rb b/spec/requests/admin/theme_site_settings_controller_spec.rb new file mode 100644 index 00000000000..d7b6dd06b52 --- /dev/null +++ b/spec/requests/admin/theme_site_settings_controller_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +RSpec.describe Admin::Config::ThemeSiteSettingsController do + fab!(:admin) + fab!(:theme_1) { Fabricate(:theme) } + fab!(:theme_2) { Fabricate(:theme) } + fab!(:theme_3) { Fabricate(:theme) } + fab!(:theme_1_theme_site_setting) do + Fabricate( + :theme_site_setting_with_service, + theme: theme_1, + name: "search_experience", + value: "search_field", + ) + end + fab!(:theme_2_theme_site_setting) do + Fabricate( + :theme_site_setting_with_service, + theme: theme_2, + name: "enable_welcome_banner", + value: false, + ) + end + + before { sign_in(admin) } + + describe "#index" do + it "gets all theme site settings and the themes which have overridden values for these settings" do + get "/admin/config/theme-site-settings.json" + + expect(response.status).to eq(200) + json = response.parsed_body + + expect(json["themeable_site_settings"]).to include( + "search_experience", + "enable_welcome_banner", + ) + + search_setting = + json["themes_with_site_setting_overrides"]["search_experience"].deep_symbolize_keys + expect(search_setting[:setting]).to eq("search_experience") + expect(search_setting[:default]).to eq("search_icon") + expect(search_setting[:description]).to eq(I18n.t("site_settings.search_experience")) + expect(search_setting[:type]).to eq("enum") + expect(search_setting[:themes].find { |t| t[:theme_id] == theme_1.id }).to include( + theme_name: theme_1.name, + theme_id: theme_1.id, + value: "search_field", + ) + + welcome_banner_setting = + json["themes_with_site_setting_overrides"]["enable_welcome_banner"].deep_symbolize_keys + expect(welcome_banner_setting[:setting]).to eq("enable_welcome_banner") + expect(welcome_banner_setting[:default]).to eq(true) + expect(welcome_banner_setting[:description]).to eq( + I18n.t("site_settings.enable_welcome_banner"), + ) + expect(welcome_banner_setting[:type]).to eq("bool") + expect(welcome_banner_setting[:themes].find { |t| t[:theme_id] == theme_2.id }).to include( + theme_name: theme_2.name, + theme_id: theme_2.id, + value: false, + ) + end + + it "does not count theme site settings with same value as site setting default as overridden" do + theme_2_theme_site_setting.update!( + value: SiteSetting.type_supervisor.to_db_value(:enable_welcome_banner, true).first, + ) + + get "/admin/config/theme-site-settings.json" + + expect(response.status).to eq(200) + json = response.parsed_body + + welcome_banner_setting = + json["themes_with_site_setting_overrides"]["enable_welcome_banner"].deep_symbolize_keys + + expect(welcome_banner_setting[:themes].find { |t| t[:theme_id] == theme_2.id }).to be_nil + end + end +end diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 5a9f57e9cfb..80617fd5956 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -1504,6 +1504,7 @@ RSpec.describe ApplicationController do "isStaffWritesOnly", "activatedThemes", "#{TopicList.new("latest", Fabricate(:anonymous), []).preload_key}", + "themeSiteSettingOverrides", ], ) end @@ -1529,6 +1530,7 @@ RSpec.describe ApplicationController do "activatedThemes", "#{TopicList.new("latest", Fabricate(:anonymous), []).preload_key}", "currentUser", + "themeSiteSettingOverrides", "topicTrackingStates", "topicTrackingStateMeta", ], @@ -1556,6 +1558,7 @@ RSpec.describe ApplicationController do "activatedThemes", "#{TopicList.new("latest", Fabricate(:anonymous), []).preload_key}", "currentUser", + "themeSiteSettingOverrides", "topicTrackingStates", "topicTrackingStateMeta", "fontMap", diff --git a/spec/services/themes/create_spec.rb b/spec/services/themes/create_spec.rb index 1d447bf720c..320c6b15c98 100644 --- a/spec/services/themes/create_spec.rb +++ b/spec/services/themes/create_spec.rb @@ -113,6 +113,13 @@ RSpec.describe Themes::Create do result end + it "updates the SiteSetting.theme_site_settings cache for the theme" do + new_theme = result.theme + expect(SiteSetting.theme_site_settings[new_theme.id]).to eq( + ThemeSiteSetting.generate_theme_map[new_theme.id], + ) + end + context "with component param" do let(:params) do theme_params.merge(component: true, user_selectable: false, color_scheme_id: nil) diff --git a/spec/services/themes/theme_site_setting_manager_spec.rb b/spec/services/themes/theme_site_setting_manager_spec.rb new file mode 100644 index 00000000000..e46851ce1ff --- /dev/null +++ b/spec/services/themes/theme_site_setting_manager_spec.rb @@ -0,0 +1,213 @@ +# frozen_string_literal: true + +RSpec.describe Themes::ThemeSiteSettingManager do + describe described_class::Contract, type: :model do + it { is_expected.to validate_presence_of(:theme_id) } + it { is_expected.to validate_presence_of(:name) } + end + + describe ".call" do + subject(:result) { described_class.call(guardian:, params:) } + + fab!(:admin) + fab!(:theme) + + let(:guardian) { admin.guardian } + let(:params) { { theme_id: theme.id, name: "enable_welcome_banner", value: false } } + + before { SiteSetting.refresh! } + + context "when data is invalid" do + let(:params) { {} } + + it { is_expected.to fail_a_contract } + end + + context "when a non-admin user tries to change a setting" do + let(:guardian) { Guardian.new } + + it { is_expected.to fail_a_policy(:current_user_is_admin) } + end + + context "when the site setting is not a themeable one" do + let(:params) { { theme_id: theme.id, name: "title", value: "New Title" } } + + it { is_expected.to fail_a_policy(:ensure_setting_is_themeable) } + end + + context "when theme doesn't exist" do + before { theme.destroy! } + + it "fails to find the theme" do + expect(result).to fail_to_find_a_model(:theme) + end + end + + context "when creating a new theme site setting" do + it { is_expected.to run_successfully } + + it "creates a new theme site setting" do + expect { result }.to change { ThemeSiteSetting.count }.by(1) + + theme_site_setting = ThemeSiteSetting.last + expect(theme_site_setting).to have_attributes( + theme_id: theme.id, + name: "enable_welcome_banner", + value: "f", + data_type: SiteSetting.types[:bool], + ) + end + + it "logs the creation in staff action log" do + StaffActionLogger + .any_instance + .expects(:log_theme_site_setting_change) + .with(:enable_welcome_banner, nil, false, theme) + result + end + + it "refreshes the value in the SiteSetting cache" do + expect { result }.to change { SiteSetting.enable_welcome_banner(theme_id: theme.id) }.from( + true, + ).to(false) + end + + it "should publish changes to clients for client site settings" do + message = MessageBus.track_publish("/client_settings") { result }.first + expect(message.data).to eq( + { name: :enable_welcome_banner, scoped_to: { theme_id: theme.id }, value: false }, + ) + end + + it "sends a DiscourseEvent for the change" do + event = + DiscourseEvent + .track_events { messages = MessageBus.track_publish { result } } + .find { |e| e[:event_name] == :theme_site_setting_changed } + + expect(event).to be_present + expect(event[:params]).to eq([:enable_welcome_banner, true, false]) + end + end + + context "when updating an existing theme site setting" do + fab!(:theme_site_setting) do + # NOTE: This example is a little contrived, because `true` is the same as the site setting default, + # it would usually not ever be inserted into ThemeSiteSetting. + # + # However, we don't have any theme site settings yet with an enum with > 2 choices, + # so we have to fake things a bit here to make sure the update behaviour works. + Fabricate(:theme_site_setting, theme: theme, name: "enable_welcome_banner", value: true) + end + + it { is_expected.to run_successfully } + + it "updates the existing theme site setting" do + expect { result }.not_to change { ThemeSiteSetting.count } + expect(theme_site_setting.reload.value).to eq("f") + end + + it "logs the creation in staff action log" do + StaffActionLogger + .any_instance + .expects(:log_theme_site_setting_change) + .with(:enable_welcome_banner, true, false, theme) + result + end + + it "refreshes the value in the SiteSetting cache" do + expect { result }.to change { SiteSetting.enable_welcome_banner(theme_id: theme.id) }.from( + true, + ).to(false) + end + + it "should publish changes to clients for client site settings" do + message = MessageBus.track_publish("/client_settings") { result }.first + expect(message.data).to eq( + { name: :enable_welcome_banner, scoped_to: { theme_id: theme.id }, value: false }, + ) + end + + it "sends a DiscourseEvent for the change" do + event = + DiscourseEvent + .track_events { messages = MessageBus.track_publish { result } } + .find { |e| e[:event_name] == :theme_site_setting_changed } + + expect(event[:params]).to eq([:enable_welcome_banner, true, false]) + end + end + + context "when removing a theme site setting by ommitting the value" do + let!(:theme_site_setting) do + Fabricate( + :theme_site_setting_with_service, + theme: theme, + name: "enable_welcome_banner", + value: false, + ) + end + + before { SiteSetting.refresh! } + + let(:params) { { theme_id: theme.id, name: "enable_welcome_banner", value: nil } } + + it { is_expected.to run_successfully } + + it "updates the theme site setting to the site setting default value" do + result + expect(theme_site_setting.reload.value).to eq("t") + end + + it "logs the removal in staff action log" do + StaffActionLogger + .any_instance + .expects(:log_theme_site_setting_change) + .with(:enable_welcome_banner, false, true, theme) + result + end + + it "refreshes the value in the SiteSetting cache" do + expect { result }.to change { SiteSetting.enable_welcome_banner(theme_id: theme.id) }.from( + false, + ).to(true) + end + + it "should publish changes to clients for client site settings" do + message = MessageBus.track_publish("/client_settings") { result }.first + expect(message.data).to eq( + { name: :enable_welcome_banner, scoped_to: { theme_id: theme.id }, value: true }, + ) + end + + it "sends a DiscourseEvent for the change" do + event = + DiscourseEvent + .track_events { messages = MessageBus.track_publish { result } } + .find { |e| e[:event_name] == :theme_site_setting_changed } + + expect(event[:params]).to eq([:enable_welcome_banner, false, true]) + end + end + + context "when changing value to the same as the site setting default" do + let!(:theme_site_setting) do + Fabricate( + :theme_site_setting_with_service, + theme: theme, + name: "enable_welcome_banner", + value: false, + ) + end + + let(:params) { { theme_id: theme.id, name: "enable_welcome_banner", value: true } } + + it { is_expected.to run_successfully } + + it "updates theme site setting when value matches default" do + result + expect(theme_site_setting.reload.value).to eq("t") + end + end + end +end diff --git a/spec/support/shared_examples/core_features.rb b/spec/support/shared_examples/core_features.rb index a39d70dcf97..8bb6aa02d22 100644 --- a/spec/support/shared_examples/core_features.rb +++ b/spec/support/shared_examples/core_features.rb @@ -160,7 +160,7 @@ RSpec.shared_examples_for "having working core features" do |skip_examples: []| before do SearchIndexer.enable topics.each { SearchIndexer.index(_1, force: true) } - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) end after { SearchIndexer.disable } diff --git a/spec/system/admin_config_theme_site_settings_spec.rb b/spec/system/admin_config_theme_site_settings_spec.rb new file mode 100644 index 00000000000..1c1689d1aa1 --- /dev/null +++ b/spec/system/admin_config_theme_site_settings_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +describe "Admin Theme Site Settings", type: :system do + fab!(:current_user) { Fabricate(:admin) } + fab!(:theme_1) { Fabricate(:theme, name: "Blue Steel") } + fab!(:theme_2) { Fabricate(:theme, name: "Derelicte") } + fab!(:theme_site_setting_1) do + Fabricate( + :theme_site_setting_with_service, + theme: theme_1, + name: "enable_welcome_banner", + value: false, + ) + end + fab!(:theme_site_setting_2) do + Fabricate( + :theme_site_setting_with_service, + theme: theme_2, + name: "search_experience", + value: "search_field", + ) + end + + let(:theme_site_settings_page) { PageObjects::Pages::AdminThemeSiteSettings.new } + + before { sign_in(current_user) } + + it "shows the themeable site settings and their name, description, and default value" do + visit "/admin/config/theme-site-settings" + expect(theme_site_settings_page).to have_setting_with_default("enable_welcome_banner") + expect(theme_site_settings_page).to have_setting_with_default("search_experience") + end + + it "shows links to each theme that overrides the default and overridden values" do + visit "/admin/config/theme-site-settings" + expect(theme_site_settings_page).to have_theme_overriding( + "enable_welcome_banner", + theme_1, + "false", + ) + expect(theme_site_settings_page).to have_theme_overriding( + "search_experience", + theme_2, + "search_field", + ) + end +end diff --git a/spec/system/admin_customize_themes_spec.rb b/spec/system/admin_customize_themes_spec.rb index 4ad3166764d..411a4bac837 100644 --- a/spec/system/admin_customize_themes_spec.rb +++ b/spec/system/admin_customize_themes_spec.rb @@ -2,7 +2,7 @@ describe "Admin Customize Themes", type: :system do fab!(:color_scheme) - fab!(:theme) { Fabricate(:theme, name: "Cool theme 1") } + fab!(:theme) { Fabricate(:theme, name: "Cool theme 1", user_selectable: true) } fab!(:admin) { Fabricate(:admin, locale: "en") } let(:theme_page) { PageObjects::Pages::AdminCustomizeThemes.new } @@ -323,4 +323,69 @@ describe "Admin Customize Themes", type: :system do expect(page).to have_current_path("/") end end + + describe "editing theme site settings" do + it "shows all themeable site settings and allows editing values" do + theme_page.visit(theme.id) + SiteSetting.themeable_site_settings.each do |setting_name| + expect(theme_page).to have_theme_site_setting(setting_name) + end + theme_page.toggle_theme_site_setting("enable_welcome_banner") + expect(theme_page).to have_overridden_theme_site_setting("enable_welcome_banner") + expect(page).to have_content( + I18n.t("admin_js.admin.customize.theme.theme_site_setting_saved"), + ) + expect( + ThemeSiteSetting.exists?(theme: theme, name: "enable_welcome_banner", value: "f"), + ).to be_truthy + end + + it "allows resetting themeable site setting values back to site setting default" do + Fabricate( + :theme_site_setting_with_service, + theme: theme, + name: "enable_welcome_banner", + value: false, + ) + theme_page.visit(theme.id) + expect(theme_page).to have_overridden_theme_site_setting("enable_welcome_banner") + theme_page.reset_overridden_theme_site_setting("enable_welcome_banner") + expect(page).to have_content( + I18n.t("admin_js.admin.customize.theme.theme_site_setting_saved"), + ) + expect( + ThemeSiteSetting.exists?(theme: theme, name: "enable_welcome_banner", value: "f"), + ).to be_falsey + end + + it "does not show the overridden indicator if the theme site setting value in the DB is the same as the default" do + Fabricate( + :theme_site_setting_with_service, + theme: theme, + name: "enable_welcome_banner", + value: true, + ) + theme_page.visit(theme.id) + expect(theme_page).to have_theme_site_setting("enable_welcome_banner") + expect(theme_page).to have_no_overridden_theme_site_setting("enable_welcome_banner") + end + + it "alters the UI via MessageBus when a theme site setting changes" do + SiteSetting.refresh!(refresh_site_settings: false, refresh_theme_site_settings: true) + banner = PageObjects::Components::WelcomeBanner.new + other_user = Fabricate(:user) + other_user.user_option.update!(theme_ids: [theme.id]) + sign_in(other_user) + visit("/") + expect(banner).to be_visible + + using_session(:admin) do + sign_in(admin) + theme_page.visit(theme.id) + theme_page.toggle_theme_site_setting("enable_welcome_banner") + end + + expect(banner).to be_hidden + end + end end diff --git a/spec/system/header_search_spec.rb b/spec/system/header_search_spec.rb index e911f94d71a..78b6e8d8754 100644 --- a/spec/system/header_search_spec.rb +++ b/spec/system/header_search_spec.rb @@ -4,11 +4,13 @@ RSpec.describe "Header Search - Responsive Behavior", type: :system do fab!(:current_user, :user) let(:search_page) { PageObjects::Pages::Search.new } - before { SiteSetting.search_experience = "search_field" } + before do + Fabricate(:theme_site_setting_with_service, name: "search_experience", value: "search_field") + end context "when welcome banner is enabled" do it "appears based on scroll & screen width with search banner enabled" do - SiteSetting.enable_welcome_banner = true + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: true) sign_in(current_user) visit "/" @@ -31,7 +33,7 @@ RSpec.describe "Header Search - Responsive Behavior", type: :system do end it "appears when search banner is not enabled & shows / hides based on viewport width" do - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) sign_in(current_user) visit "/" @@ -50,7 +52,8 @@ RSpec.describe "Header Search - Responsive Behavior", type: :system do end it "does not appear when search setting is set to icon" do - SiteSetting.search_experience = "search_icon" + Fabricate(:theme_site_setting_with_service, name: "search_experience", value: "search_icon") + sign_in(current_user) visit "/" diff --git a/spec/system/header_spec.rb b/spec/system/header_spec.rb index 6638b932150..65cd36c1608 100644 --- a/spec/system/header_spec.rb +++ b/spec/system/header_spec.rb @@ -227,7 +227,7 @@ RSpec.describe "Glimmer Header", type: :system do it "displays current user when logged in and login required" do SiteSetting.login_required = true - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) sign_in(current_user) visit "/" diff --git a/spec/system/page_objects/pages/admin_customize_themes.rb b/spec/system/page_objects/pages/admin_customize_themes.rb index 9a4c2820d86..73eba227488 100644 --- a/spec/system/page_objects/pages/admin_customize_themes.rb +++ b/spec/system/page_objects/pages/admin_customize_themes.rb @@ -99,6 +99,39 @@ module PageObjects has_css?("#{setting_selector(setting_name)} .desc", exact_text: description) end + def has_theme_site_setting?(setting_name) + find(theme_site_setting_selector(setting_name)).has_css?( + ".setting-label", + text: SiteSetting.humanized_name(setting_name), + ) + find(theme_site_setting_selector(setting_name)).has_css?( + ".setting-value", + text: SiteSetting.description(setting_name), + ) + end + + def has_overridden_theme_site_setting?(setting_name) + has_css?(theme_site_setting_selector(setting_name, overridden: true)) + end + + def has_no_overridden_theme_site_setting?(setting_name) + has_no_css?(theme_site_setting_selector(setting_name, overridden: true)) + end + + def toggle_theme_site_setting(setting_name) + find(theme_site_setting_selector(setting_name)).find( + ".setting-value input[type='checkbox']", + ).click + find(theme_site_setting_selector(setting_name)).find(".setting-controls .ok").click + end + + def reset_overridden_theme_site_setting(setting_name) + find(theme_site_setting_selector(setting_name, overridden: true)).find( + ".setting-controls__undo", + ).click + find(theme_site_setting_selector(setting_name)).find(".setting-controls .ok").click + end + def has_no_themes_list? has_no_css?(".themes-list-header") end @@ -210,6 +243,10 @@ module PageObjects def setting_selector(setting_name, overridden: false) "section.theme.settings .setting#{overridden ? ".overridden" : ""}[data-setting=\"#{setting_name}\"]" end + + def theme_site_setting_selector(setting_name, overridden: false) + "section.theme.theme-site-settings .setting#{overridden ? ".overridden" : ""}.theme-site-setting[data-setting=\"#{setting_name}\"]" + end end end end diff --git a/spec/system/page_objects/pages/admin_theme_site_settings.rb b/spec/system/page_objects/pages/admin_theme_site_settings.rb new file mode 100644 index 00000000000..0ae7e471bd5 --- /dev/null +++ b/spec/system/page_objects/pages/admin_theme_site_settings.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class AdminThemeSiteSettings < PageObjects::Pages::AdminBase + def has_setting_with_default?(setting_name) + setting_row(setting_name).has_css?( + ".admin-theme-site-settings-row__setting .setting-label", + text: SiteSetting.humanized_name(setting_name), + ) + setting_row(setting_name).has_css?( + ".admin-theme-site-settings-row__setting .setting-description", + text: SiteSetting.description(setting_name), + ) + setting_row(setting_name).has_css?( + ".admin-theme-site-settings-row__default", + text: SiteSetting.defaults[setting_name], + ) + end + + def has_theme_overriding?(setting_name, theme, overrride_value) + setting_row(setting_name).has_css?(theme_overriding_css(theme), text: theme.name) + find(theme_overriding_css(theme)).hover + find(".fk-d-tooltip__content.-content.-expanded").has_content?( + I18n.t("admin_js.admin.theme_site_settings.overridden_value", value: overrride_value), + ) + end + + def theme_overriding_css(theme) + ".admin-theme-site-settings-row__overridden .theme-link[data-theme-id='#{theme.id}']" + end + + def setting_row(setting_name) + page.find( + ".d-admin-row__content.admin-theme-site-settings-row[data-setting-name='#{setting_name}']", + ) + end + end + end +end diff --git a/spec/system/search_shortcut_variation_spec.rb b/spec/system/search_shortcut_variation_spec.rb index 1b752919456..033851d7649 100644 --- a/spec/system/search_shortcut_variation_spec.rb +++ b/spec/system/search_shortcut_variation_spec.rb @@ -9,10 +9,14 @@ describe "Search | Shortcuts for variations of search input", type: :system do before { sign_in(current_user) } context "when search_experience is search_field" do - before { SiteSetting.search_experience = "search_field" } + before do + Fabricate(:theme_site_setting_with_service, name: "search_experience", value: "search_field") + end context "when enable_welcome_banner is true" do - before { SiteSetting.enable_welcome_banner = true } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: true) + end it "displays and focuses welcome banner search when / is pressed and hides it when Escape is pressed" do visit("/") @@ -41,7 +45,9 @@ describe "Search | Shortcuts for variations of search input", type: :system do end context "when enable_welcome_banner is false" do - before { SiteSetting.enable_welcome_banner = false } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) + end it "displays and focuses header search when / is pressed and hides it when Escape is pressed" do visit("/") @@ -56,10 +62,14 @@ describe "Search | Shortcuts for variations of search input", type: :system do end context "when search_experience is search_icon" do - before { SiteSetting.search_experience = "search_icon" } + before do + Fabricate(:theme_site_setting_with_service, name: "search_experience", value: "search_icon") + end context "when enable_welcome_banner is true" do - before { SiteSetting.enable_welcome_banner = true } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: true) + end it "displays and focuses welcome banner search when / is pressed and hides it when Escape is pressed" do visit("/") @@ -88,7 +98,9 @@ describe "Search | Shortcuts for variations of search input", type: :system do end context "when enable_welcome_banner is false" do - before { SiteSetting.enable_welcome_banner = false } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) + end it "displays and focuses search icon search when / is pressed and hides it when Escape is pressed" do visit("/") diff --git a/spec/system/search_spec.rb b/spec/system/search_spec.rb index 331836cfb05..2d8bb849ba3 100644 --- a/spec/system/search_spec.rb +++ b/spec/system/search_spec.rb @@ -14,7 +14,7 @@ describe "Search", type: :system do SearchIndexer.enable SearchIndexer.index(topic, force: true) SearchIndexer.index(topic2, force: true) - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) end after { SearchIndexer.disable } @@ -90,7 +90,7 @@ describe "Search", type: :system do SearchIndexer.index(topic, force: true) SiteSetting.rate_limit_search_anon_user_per_minute = 4 RateLimiter.enable - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) end after { SearchIndexer.disable } @@ -116,7 +116,7 @@ describe "Search", type: :system do SearchIndexer.enable SearchIndexer.index(topic, force: true) SearchIndexer.index(topic2, force: true) - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) end after { SearchIndexer.disable } @@ -153,7 +153,9 @@ describe "Search", type: :system do end describe "with search icon in header" do - before { SiteSetting.search_experience = "search_icon" } + before do + Fabricate(:theme_site_setting_with_service, name: "search_experience", value: "search_icon") + end it "displays the correct search mode" do visit("/") @@ -163,7 +165,13 @@ describe "Search", type: :system do end describe "with search field in header" do - before { SiteSetting.search_experience = "search_field" } + before do + Fabricate( + :theme_site_setting_with_service, + name: "search_experience", + value: "search_field", + ) + end it "displays the correct search mode" do visit("/") @@ -219,7 +227,7 @@ describe "Search", type: :system do SearchIndexer.enable SearchIndexer.index(topic, force: true) SearchIndexer.index(topic2, force: true) - SiteSetting.enable_welcome_banner = false + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) sign_in(admin) end diff --git a/spec/system/user_page/user_page_search_spec.rb b/spec/system/user_page/user_page_search_spec.rb index a01dcb6258d..7d3b5bedf8f 100644 --- a/spec/system/user_page/user_page_search_spec.rb +++ b/spec/system/user_page/user_page_search_spec.rb @@ -4,6 +4,10 @@ describe "User page search", type: :system do fab!(:user) let(:search_page) { PageObjects::Pages::Search.new } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) + end + it "filters down to the user" do sign_in(user) diff --git a/spec/system/welcome_banner_spec.rb b/spec/system/welcome_banner_spec.rb index 7d85cd654c4..e1149a013ad 100644 --- a/spec/system/welcome_banner_spec.rb +++ b/spec/system/welcome_banner_spec.rb @@ -6,7 +6,9 @@ describe "Welcome banner", type: :system do let(:search_page) { PageObjects::Pages::Search.new } context "when welcome banner is enabled" do - before { SiteSetting.enable_welcome_banner = true } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: true) + end it "shows for logged in and anonymous users" do visit "/" @@ -60,7 +62,13 @@ describe "Welcome banner", type: :system do end context "when using search_field search_experience" do - before { SiteSetting.search_experience = "search_field" } + before do + Fabricate( + :theme_site_setting_with_service, + name: "search_experience", + value: "search_field", + ) + end it "hides welcome banner and shows header search on scroll, and vice-versa" do Fabricate(:topic) @@ -82,7 +90,9 @@ describe "Welcome banner", type: :system do end context "when using search_icon search_experience" do - before { SiteSetting.search_experience = "search_icon" } + before do + Fabricate(:theme_site_setting_with_service, name: "search_experience", value: "search_icon") + end it "hides welcome banner and shows header search on scroll, and vice-versa" do Fabricate(:topic) @@ -105,7 +115,9 @@ describe "Welcome banner", type: :system do end context "when welcome banner is not enabled" do - before { SiteSetting.enable_welcome_banner = false } + before do + Fabricate(:theme_site_setting_with_service, name: "enable_welcome_banner", value: false) + end it "does not show the welcome banner for logged in and anonymous users" do visit "/"