diff --git a/app/assets/javascripts/admin/addon/adapters/theme.js b/app/assets/javascripts/admin/addon/adapters/theme.js index 66f901b2cdb..f6d6fa4bb8a 100644 --- a/app/assets/javascripts/admin/addon/adapters/theme.js +++ b/app/assets/javascripts/admin/addon/adapters/theme.js @@ -1,4 +1,5 @@ import RestAdapter from "discourse/adapters/rest"; +import ThemeSettings from "admin/models/theme-settings"; export default class Theme extends RestAdapter { jsonMode = true; @@ -8,9 +9,11 @@ export default class Theme extends RestAdapter { afterFindAll(results) { let map = {}; + results.forEach((theme) => { map[theme.id] = theme; }); + results.forEach((theme) => { let mapped = theme.get("child_themes") || []; mapped = mapped.map((t) => map[t.id]); @@ -19,7 +22,15 @@ export default class Theme extends RestAdapter { let mappedParents = theme.get("parent_themes") || []; mappedParents = mappedParents.map((t) => map[t.id]); theme.set("parentThemes", mappedParents); + + if (theme.settings) { + theme.set( + "settings", + theme.settings.map((setting) => ThemeSettings.create(setting)) + ); + } }); + return results; } } 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 e28bbc95fbd..5c8e9734356 100644 --- a/app/assets/javascripts/admin/addon/components/theme-setting-editor.js +++ b/app/assets/javascripts/admin/addon/components/theme-setting-editor.js @@ -1,17 +1,10 @@ -import { ajax } from "discourse/lib/ajax"; -import { url } from "discourse/lib/computed"; import SiteSettingComponent from "./site-setting"; export default class extends SiteSettingComponent { - @url("model.id", "/admin/themes/%@/setting") updateUrl; - _save() { - return ajax(this.updateUrl, { - type: "PUT", - data: { - name: this.setting.setting, - value: this.get("buffered.value"), - }, - }); + return this.setting.updateSetting( + this.model.id, + this.get("buffered.value") + ); } } diff --git a/app/assets/javascripts/admin/addon/components/theme-settings-editor.js b/app/assets/javascripts/admin/addon/components/theme-settings-editor.js index b88bb637569..7296fbf03ac 100644 --- a/app/assets/javascripts/admin/addon/components/theme-settings-editor.js +++ b/app/assets/javascripts/admin/addon/components/theme-settings-editor.js @@ -2,7 +2,6 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; import { action } from "@ember/object"; import { service } from "@ember/service"; -import { ajax } from "discourse/lib/ajax"; import I18n from "discourse-i18n"; export default class ThemeSettingsEditor extends Component { @@ -52,6 +51,7 @@ export default class ThemeSettingsEditor extends Component { if (!this.theme) { return null; } + return this.theme.settings.map((setting) => ({ setting: setting.setting, value: setting.value, @@ -86,11 +86,14 @@ export default class ThemeSettingsEditor extends Component { this.saving = true; this.errors = []; this.success = ""; + if (!this.editedContent) { // no changes. return; } + let newSettings = ""; + try { newSettings = JSON.parse(this.editedContent); } catch (e) { @@ -104,6 +107,7 @@ export default class ThemeSettingsEditor extends Component { this.saving = false; return; } + if (!this.validateSettingsKeys(newSettings)) { this.errors = [ ...this.errors, @@ -119,14 +123,17 @@ export default class ThemeSettingsEditor extends Component { const originalNames = this.theme ? this.theme.settings.map((setting) => setting.setting) : []; + const newNames = newSettings.map((setting) => setting.setting); const deletedNames = originalNames.filter( (originalName) => !newNames.find((newName) => newName === originalName) ); + const addedNames = newNames.filter( (newName) => !originalNames.find((originalName) => originalName === newName) ); + if (deletedNames.length) { this.errors = [ ...this.errors, @@ -136,6 +143,7 @@ export default class ThemeSettingsEditor extends Component { }, ]; } + if (addedNames.length) { this.errors = [ ...this.errors, @@ -151,41 +159,49 @@ export default class ThemeSettingsEditor extends Component { return; } - const changedSettings = newSettings.filter((newSetting) => { + const changedSettings = []; + + newSettings.forEach((newSetting) => { const originalSetting = this.theme.settings.find( (_originalSetting) => _originalSetting.setting === newSetting.setting ); - return originalSetting.value !== newSetting.value; + + if (originalSetting.value !== newSetting.value) { + changedSettings.push({ + originalSetting, + value: newSetting.value, + }); + } }); - for (let setting of changedSettings) { + + for (let changedSetting of changedSettings) { try { - await this.saveSetting(this.theme.id, setting); + await this.saveSetting(this.theme.id, changedSetting); } catch (err) { const errorObjects = JSON.parse(err.jqXHR.responseText).errors.map( (error) => ({ - setting: setting.setting, + setting: changedSetting.originalSetting.setting, errorMessage: error, }) ); + this.errors = [...this.errors, ...errorObjects]; } } + if (this.errors.length === 0) { this.editedContent = null; } + this.saving = false; this.dialog.cancel(); this.customizeThemeShowController.send("routeRefreshModel"); } - async saveSetting(themeId, setting) { - const updateUrl = `/admin/themes/${themeId}/setting`; - return await ajax(updateUrl, { - type: "PUT", - data: { - name: setting.setting, - value: setting.value, - }, - }); + async saveSetting(themeId, changedSetting) { + return await changedSetting.originalSetting.updateSetting( + themeId, + changedSetting.value + ); } } diff --git a/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js b/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js index b271dafdd11..c493d0b8859 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js @@ -6,6 +6,7 @@ import { mapBy, match, notEmpty, + readOnly, } from "@ember/object/computed"; import { service } from "@ember/service"; import { popupAjaxError } from "discourse/lib/ajax-error"; @@ -43,6 +44,7 @@ export default class AdminCustomizeThemesShowController extends Controller { @notEmpty("settings") hasSettings; @notEmpty("translations") hasTranslations; @match("model.remote_theme.remote_url", /^http(s)?:\/\//) sourceIsHttp; + @readOnly("model.settings") settings; @discourseComputed("model.component", "model.remote_theme") showCheckboxes() { @@ -146,11 +148,6 @@ export default class AdminCustomizeThemesShowController extends Controller { return `admin.customize.theme.convert_${type}_tooltip`; } - @discourseComputed("model.settings") - settings(settings) { - return settings.map((setting) => ThemeSettings.create(setting)); - } - @discourseComputed("model.translations") translations(translations) { return translations.map((setting) => diff --git a/app/assets/javascripts/admin/addon/models/theme-settings.js b/app/assets/javascripts/admin/addon/models/theme-settings.js index f61da7c2c0e..9b6b5725c39 100644 --- a/app/assets/javascripts/admin/addon/models/theme-settings.js +++ b/app/assets/javascripts/admin/addon/models/theme-settings.js @@ -1,4 +1,19 @@ import EmberObject from "@ember/object"; +import { ajax } from "discourse/lib/ajax"; import Setting from "admin/mixins/setting-object"; -export default class ThemeSettings extends EmberObject.extend(Setting) {} +export default class ThemeSettings extends EmberObject.extend(Setting) { + 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, + }, + }); + } +} diff --git a/lib/theme_settings_manager/objects.rb b/lib/theme_settings_manager/objects.rb index 5c53f28c19e..71b9a9d0643 100644 --- a/lib/theme_settings_manager/objects.rb +++ b/lib/theme_settings_manager/objects.rb @@ -6,6 +6,7 @@ class ThemeSettingsManager::Objects < ThemeSettingsManager end def value=(objects) + objects = JSON.parse(objects) if objects.is_a?(::String) ensure_is_valid_value!(objects) record = has_record? ? update_record!(json_value: objects) : create_record!(json_value: objects) theme.reload diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index 4f24080cff1..6c6a89ed924 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -1040,7 +1040,7 @@ RSpec.describe Admin::ThemesController do end describe "#update_single_setting" do - let(:theme) { Fabricate(:theme) } + fab!(:theme) before do theme.set_field(target: :settings, name: :yaml, value: "bg: red") @@ -1063,6 +1063,38 @@ RSpec.describe Admin::ThemesController do expect(user_history.action).to eq(UserHistory.actions[:change_theme_setting]) end + it "should be able to update a theme setting of `objects` typed" do + SiteSetting.experimental_objects_type_for_theme_settings = true + + field = + theme.set_field( + target: :settings, + name: "yaml", + value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"), + ) + + theme.save! + + put "/admin/themes/#{theme.id}/setting.json", + params: { + name: "objects_setting", + value: [ + { name: "new_section", links: [{ name: "new link", url: "https://some.url.com" }] }, + ].to_json, + } + + expect(response.status).to eq(200) + + expect(theme.settings[:objects_setting].value).to eq( + [ + { + "name" => "new_section", + "links" => [{ "name" => "new link", "url" => "https://some.url.com" }], + }, + ], + ) + end + it "should clear a theme setting" do put "/admin/themes/#{theme.id}/setting.json", params: { name: "bg" } theme.reload diff --git a/spec/system/admin_customize_themes_spec.rb b/spec/system/admin_customize_themes_spec.rb index 1429a9ae873..339b47c191a 100644 --- a/spec/system/admin_customize_themes_spec.rb +++ b/spec/system/admin_customize_themes_spec.rb @@ -110,5 +110,43 @@ describe "Admin Customize Themes", type: :system do "/admin/customize/themes/#{theme.id}/schema/objects_setting", ) end + + it "allows an admin to edit a theme setting of objects type via the settings editor" do + visit "/admin/customize/themes/#{theme.id}" + + theme_settings_editor = admin_customize_themes_page.click_theme_settings_editor_button + + theme_settings_editor.fill_in(<<~SETTING) + [ + { + "setting": "objects_setting", + "value": [ + { + "name": "new section", + "links": [ + { + "name": "new link", + "url": "https://example.com" + } + ] + } + ] + } + ] + SETTING + + theme_settings_editor.save + + try_until_success do + expect(theme.reload.settings[:objects_setting].value).to eq( + [ + { + "links" => [{ "name" => "new link", "url" => "https://example.com" }], + "name" => "new section", + }, + ], + ) + end + end end end diff --git a/spec/system/page_objects/components/ace_editor.rb b/spec/system/page_objects/components/ace_editor.rb index d7aee298524..cbb29db863b 100644 --- a/spec/system/page_objects/components/ace_editor.rb +++ b/spec/system/page_objects/components/ace_editor.rb @@ -9,6 +9,9 @@ module PageObjects end def fill_input(content) + # Clear the input before filling it in because capybara's fill_in method doesn't seem to replace existing content + # unless the content is a blank string. + editor_input.fill_in(with: "") editor_input.fill_in(with: content) self end @@ -18,7 +21,10 @@ module PageObjects end def editor_input - find(".ace-wrapper .ace_text-input", visible: false) + find(".ace-wrapper .ace:not(.hidden)", visible: true).find( + ".ace_text-input", + visible: false, + ) end end end diff --git a/spec/system/page_objects/components/admin_theme_settings_editor.rb b/spec/system/page_objects/components/admin_theme_settings_editor.rb new file mode 100644 index 00000000000..631d44b1391 --- /dev/null +++ b/spec/system/page_objects/components/admin_theme_settings_editor.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module PageObjects + module Components + class AdminThemeSettingsEditor < Base + def fill_in(settings) + editor.fill_input(settings) + self + end + + def save + click_button(I18n.t("admin_js.admin.customize.theme.save")) + self + end + + private + + def editor + @editor ||= within(".settings-editor") { AceEditor.new } + end + end + end +end diff --git a/spec/system/page_objects/pages/admin_customize_themes.rb b/spec/system/page_objects/pages/admin_customize_themes.rb index 1ce116edc6a..bd255a24092 100644 --- a/spec/system/page_objects/pages/admin_customize_themes.rb +++ b/spec/system/page_objects/pages/admin_customize_themes.rb @@ -42,6 +42,11 @@ module PageObjects def click_edit_objects_theme_setting_button(setting_name) find(".theme-setting[data-setting=\"#{setting_name}\"] .setting-value-edit-button").click end + + def click_theme_settings_editor_button + click_button(I18n.t("admin_js.admin.customize.theme.settings_editor")) + PageObjects::Components::AdminThemeSettingsEditor.new + end end end end