diff --git a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs index b8d8a08829d..0fc7f6596fa 100644 --- a/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs +++ b/app/assets/javascripts/admin/addon/components/schema-theme-setting/editor.gjs @@ -243,7 +243,7 @@ export default class SchemaThemeSettingEditor extends Component { } fieldDescription(fieldName) { - const descriptions = this.args.setting.objects_schema_property_descriptions; + const descriptions = this.args.setting.metadata?.property_descriptions; if (!descriptions) { return; diff --git a/app/assets/javascripts/admin/addon/models/theme-settings.js b/app/assets/javascripts/admin/addon/models/theme-settings.js index 9b6b5725c39..03ee2a04d86 100644 --- a/app/assets/javascripts/admin/addon/models/theme-settings.js +++ b/app/assets/javascripts/admin/addon/models/theme-settings.js @@ -1,5 +1,6 @@ import EmberObject from "@ember/object"; import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; import Setting from "admin/mixins/setting-object"; export default class ThemeSettings extends EmberObject.extend(Setting) { @@ -16,4 +17,12 @@ export default class ThemeSettings extends EmberObject.extend(Setting) { }, }); } + + loadMetadata(themeId) { + return ajax( + `/admin/themes/${themeId}/objects_setting_metadata/${this.setting}.json` + ) + .then((result) => this.set("metadata", result)) + .catch(popupAjaxError); + } } diff --git a/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show-schema.js b/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show-schema.js index a0e935345bf..2bfe91c525f 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show-schema.js +++ b/app/assets/javascripts/admin/addon/routes/admin-customize-themes-show-schema.js @@ -5,10 +5,12 @@ export default class AdminCustomizeThemesShowSchemaRoute extends Route { const theme = this.modelFor("adminCustomizeThemesShow"); const setting = theme.settings.findBy("setting", params.setting_name); - return { - theme, - setting, - }; + return setting.loadMetadata(theme.id).then(() => { + return { + theme, + setting, + }; + }); } setupController() { diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index 151819edd98..f43f9d6f908 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -173,10 +173,9 @@ class Admin::ThemesController < Admin::AdminController @color_schemes = ColorScheme.all.includes(:theme, color_scheme_colors: :color_scheme).to_a payload = { - themes: ActiveModel::ArraySerializer.new(@themes, each_serializer: ThemeSerializer), + themes: serialize_data(@themes, ThemeSerializer), extras: { - color_schemes: - ActiveModel::ArraySerializer.new(@color_schemes, each_serializer: ColorSchemeSerializer), + color_schemes: serialize_data(@color_schemes, ColorSchemeSerializer), }, } @@ -293,7 +292,7 @@ class Admin::ThemesController < Admin::AdminController @theme = Theme.include_relations.find_by(id: params[:id]) raise Discourse::InvalidParameters.new(:id) unless @theme - render json: ThemeSerializer.new(@theme) + render_serialized(@theme, ThemeSerializer) end def export @@ -352,6 +351,18 @@ class Admin::ThemesController < Admin::AdminController raise Discourse::InvalidAccess if !SiteSetting.experimental_objects_type_for_theme_settings end + def objects_setting_metadata + raise Discourse::InvalidAccess if !SiteSetting.experimental_objects_type_for_theme_settings + + theme = Theme.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) unless theme + + theme_setting = theme.settings[params[:setting_name].to_sym] + raise Discourse::InvalidParameters.new(:setting_name) unless theme_setting + + render_serialized(theme_setting, ThemeObjectsSettingMetadataSerializer, root: false) + end + private def ban_in_allowlist_mode! diff --git a/app/serializers/theme_objects_setting_metadata_serializer.rb b/app/serializers/theme_objects_setting_metadata_serializer.rb new file mode 100644 index 00000000000..0767903b55f --- /dev/null +++ b/app/serializers/theme_objects_setting_metadata_serializer.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class ThemeObjectsSettingMetadataSerializer < ApplicationSerializer + attributes :property_descriptions + + def property_descriptions + locales = {} + key = "theme_metadata.settings.#{object.name}.schema.properties." + + object.theme.internal_translations.each do |internal_translation| + if internal_translation.key.start_with?(key) + locales[internal_translation.key.delete_prefix(key)] = internal_translation.value + end + end + + locales + end +end diff --git a/app/serializers/theme_serializer.rb b/app/serializers/theme_serializer.rb index 796724d1ada..95672fa09be 100644 --- a/app/serializers/theme_serializer.rb +++ b/app/serializers/theme_serializer.rb @@ -56,7 +56,9 @@ class ThemeSerializer < BasicThemeSerializer end def settings - object.settings.map { |_name, setting| ThemeSettingsSerializer.new(setting, root: false) } + object.settings.map do |_name, setting| + ThemeSettingsSerializer.new(setting, scope:, root: false) + end rescue ThemeSettingsParser::InvalidYaml => e @errors << e.message nil diff --git a/app/serializers/theme_settings_serializer.rb b/app/serializers/theme_settings_serializer.rb index 80739e5bd3c..d505bcfbb85 100644 --- a/app/serializers/theme_settings_serializer.rb +++ b/app/serializers/theme_settings_serializer.rb @@ -6,7 +6,6 @@ class ThemeSettingsSerializer < ApplicationSerializer :default, :value, :description, - :objects_schema_property_descriptions, :valid_values, :list_type, :textarea, @@ -38,23 +37,6 @@ class ThemeSettingsSerializer < ApplicationSerializer locale_file_description || object.description end - def objects_schema_property_descriptions - locales = {} - key = "theme_metadata.settings.#{setting}.schema.properties." - - object.theme.internal_translations.each do |internal_translation| - if internal_translation.key.start_with?(key) - locales[internal_translation.key.delete_prefix(key)] = internal_translation.value - end - end - - locales - end - - def include_objects_schema_property_descriptions? - include_objects_schema? - end - def valid_values object.choices end diff --git a/config/routes.rb b/config/routes.rb index db9b376aff9..cbfdf133c96 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -235,7 +235,9 @@ Discourse::Application.routes.draw do get "preview" => "themes#preview" get "translations/:locale" => "themes#get_translations" put "setting" => "themes#update_single_setting" + get "objects_setting_metadata/:setting_name" => "themes#objects_setting_metadata" end + collection do post "import" => "themes#import" post "upload_asset" => "themes#upload_asset" diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index d3eeaa773c2..31ca2a3fcc8 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -1313,4 +1313,83 @@ RSpec.describe Admin::ThemesController do delete "/admin/themes/bulk_destroy.json", params: { theme_ids: theme_ids } end end + + describe "#objects_setting_metadata" do + fab!(:theme) + + let(:theme_setting) do + yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml") + field = theme.set_field(target: :settings, name: "yaml", value: yaml) + theme.save! + theme.settings + end + + before { SiteSetting.experimental_objects_type_for_theme_settings = true } + + it "returns 404 if user is not an admin" do + get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json" + + expect(response.status).to eq(404) + + sign_in(user) + + get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json" + + expect(response.status).to eq(404) + + sign_in(moderator) + + get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json" + + expect(response.status).to eq(404) + end + + context "when user is an admin" do + before { sign_in(admin) } + + it "returns 403 if `experimental_objects_type_for_theme_settings` site setting is not enabled" do + SiteSetting.experimental_objects_type_for_theme_settings = false + + get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_with_categories.json" + + expect(response.status).to eq(403) + end + + it "returns 400 if the `id` param is not the id of a valid theme" do + get "/admin/themes/some_invalid_id/objects_setting_metadata/objects_with_categories.json" + + expect(response.status).to eq(400) + end + + it "returns 400 if the `setting_name` param does not match a valid setting" do + get "/admin/themes/#{theme.id}/objects_setting_metadata/some_invalid_setting_name.json" + + expect(response.status).to eq(400) + end + + it "returns 200 with the right `property_descriptions` attributes" do + theme.set_field( + target: :translations, + name: "en", + value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"), + ) + + theme.save! + + theme_setting + + get "/admin/themes/#{theme.id}/objects_setting_metadata/objects_setting.json" + + expect(response.status).to eq(200) + + expect(response.parsed_body["property_descriptions"]).to eq( + { + "links.name" => "Name of the link", + "links.url" => "URL of the link", + "name" => "Section Name", + }, + ) + end + end + end end diff --git a/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb b/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb new file mode 100644 index 00000000000..3df6fe4eaaf --- /dev/null +++ b/spec/serializers/theme_objects_setting_metadata_serializer_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +RSpec.describe ThemeObjectsSettingMetadataSerializer do + fab!(:theme) + + let(:theme_setting) do + yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml") + theme.set_field(target: :settings, name: "yaml", value: yaml) + theme.save! + theme.settings + end + + before { SiteSetting.experimental_objects_type_for_theme_settings = true } + + describe "#property_descriptions" do + let(:objects_setting_locale) do + theme.set_field( + target: :translations, + name: "en", + value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"), + ) + + theme.save! + end + + it "should return a hash of the settings property descriptions" do + objects_setting_locale + + payload = described_class.new(theme_setting[:objects_setting], root: false).as_json + + expect(payload[:property_descriptions]).to eq( + { + "links.name" => "Name of the link", + "links.url" => "URL of the link", + "name" => "Section Name", + }, + ) + end + end +end diff --git a/spec/serializers/theme_settings_serializer_spec.rb b/spec/serializers/theme_settings_serializer_spec.rb index bb2c903eab2..f210b803015 100644 --- a/spec/serializers/theme_settings_serializer_spec.rb +++ b/spec/serializers/theme_settings_serializer_spec.rb @@ -19,43 +19,4 @@ RSpec.describe ThemeSettingsSerializer do expect(payload[:theme_settings][:objects_schema][:name]).to eq("section") end end - - describe "#objects_schema_property_descriptions" do - let(:objects_setting_locale) do - theme.set_field( - target: :translations, - name: "en", - value: File.read("#{Rails.root}/spec/fixtures/theme_locales/objects_settings/en.yaml"), - ) - - theme.save! - end - - before { SiteSetting.experimental_objects_type_for_theme_settings = true } - - it "should not include the attribute when theme setting is not typed objects" do - yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml") - theme.set_field(target: :settings, name: "yaml", value: yaml) - theme.save! - - payload = ThemeSettingsSerializer.new(theme.settings[:string_setting]).as_json - - expect(payload[:theme_settings][:objects_schema_property_descriptions]).to be_nil - end - - it "should include the attribute when theme setting is of typed objects" do - objects_setting_locale - objects_setting - - payload = ThemeSettingsSerializer.new(objects_setting).as_json - - expect(payload[:theme_settings][:objects_schema_property_descriptions]).to eq( - { - "links.name" => "Name of the link", - "links.url" => "URL of the link", - "name" => "Section Name", - }, - ) - end - end end diff --git a/spec/system/admin_editing_objects_typed_theme_setting_spec.rb b/spec/system/admin_editing_objects_typed_theme_setting_spec.rb index 31b896abca6..f64e0906edb 100644 --- a/spec/system/admin_editing_objects_typed_theme_setting_spec.rb +++ b/spec/system/admin_editing_objects_typed_theme_setting_spec.rb @@ -78,12 +78,19 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do expect(admin_customize_themes_page).to have_overridden_setting("objects_setting") + admin_objects_theme_setting_editor = + admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting") + + expect(admin_objects_theme_setting_editor).to have_setting_field("name", "some new name") + + admin_objects_theme_setting_editor.back + admin_customize_themes_page.reset_overridden_setting("objects_setting") admin_objects_theme_setting_editor = admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting") - expect(admin_objects_theme_setting_editor).to have_setting_field("name", "some new name") + expect(admin_objects_theme_setting_editor).to have_setting_field("name", "section 1") end it "allows an admin to edit a theme setting of objects type via the settings editor" do diff --git a/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb b/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb index 8cb4fdaccfa..4fbca6973b5 100644 --- a/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb +++ b/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb @@ -35,6 +35,11 @@ module PageObjects self end + def back + find(".customize-themes-show-schema__back").click + self + end + private def input_field(field_name)