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 f1b9b9bfcc7..f3cf1a3ace2 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 @@ -3,7 +3,12 @@ import { cached, tracked } from "@glimmer/tracking"; import { fn } from "@ember/helper"; import { on } from "@ember/modifier"; import { action } from "@ember/object"; +import { LinkTo } from "@ember/routing"; +import { service } from "@ember/service"; import DButton from "discourse/components/d-button"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import i18n from "discourse-common/helpers/i18n"; +import { cloneJSON } from "discourse-common/lib/object"; import I18n from "discourse-i18n"; import FieldInput from "./field"; @@ -29,14 +34,19 @@ class Tree { } export default class SchemaThemeSettingEditor extends Component { + @service router; @tracked activeIndex = 0; @tracked backButtonText; + @tracked saveButtonDisabled = false; + + data = cloneJSON(this.args.setting.value); history = []; + schema = this.args.setting.objects_schema; @cached get tree() { - let schema = this.args.schema; - let data = this.args.data; + let schema = this.schema; + let data = this.data; for (const point of this.history) { data = data[point.node.index][point.propertyName]; @@ -52,14 +62,18 @@ export default class SchemaThemeSettingEditor extends Component { object, text: object[schema.identifier], }); + if (index === this.activeIndex) { node.active = true; + const childObjectsProperties = this.findChildObjectsProperties( schema.properties ); + for (const childObjectsProperty of childObjectsProperties) { const subtree = new Tree(); subtree.propertyName = childObjectsProperty.name; + data[index][childObjectsProperty.name].forEach( (childObj, childIndex) => { subtree.nodes.push( @@ -72,11 +86,14 @@ export default class SchemaThemeSettingEditor extends Component { ); } ); + node.trees.push(subtree); } } + tree.nodes.push(node); }); + return tree; } @@ -90,21 +107,25 @@ export default class SchemaThemeSettingEditor extends Component { get fields() { const node = this.activeNode; const list = []; + for (const [name, spec] of Object.entries(node.schema.properties)) { if (spec.type === "objects") { continue; } + list.push({ name, spec, value: node.object[name], }); } + return list; } findChildObjectsProperties(properties) { const list = []; + for (const [name, spec] of Object.entries(properties)) { if (spec.type === "objects") { list.push({ @@ -113,9 +134,28 @@ export default class SchemaThemeSettingEditor extends Component { }); } } + return list; } + @action + saveChanges() { + this.saveButtonDisabled = true; + + this.args.setting + .updateSetting(this.args.themeId, this.data) + .then((result) => { + this.args.setting.set("value", result[this.args.setting.setting]); + + this.router.transitionTo( + "adminCustomizeThemes.show", + this.args.themeId + ); + }) + .catch(popupAjaxError) + .finally(() => (this.saveButtonDisabled = false)); + } + @action onClick(node) { this.activeIndex = node.index; @@ -127,9 +167,11 @@ export default class SchemaThemeSettingEditor extends Component { propertyName: tree.propertyName, node: parentNode, }); + this.backButtonText = I18n.t("admin.customize.theme.schema.back_button", { name: parentNode.text, }); + this.activeIndex = node.index; } @@ -137,6 +179,7 @@ export default class SchemaThemeSettingEditor extends Component { backButtonClick() { const historyPoint = this.history.pop(); this.activeIndex = historyPoint.node.index; + if (this.history.length > 0) { this.backButtonText = I18n.t("admin.customize.theme.schema.back_button", { name: this.history[this.history.length - 1].node.text, @@ -151,6 +194,7 @@ export default class SchemaThemeSettingEditor extends Component { if (field.name === this.activeNode.schema.identifier) { this.activeNode.text = newVal; } + this.activeNode.object[field.name] = newVal; } @@ -164,6 +208,7 @@ export default class SchemaThemeSettingEditor extends Component { class="back-button" /> {{/if}} + + {{#each this.fields as |field|}} {{/each}} + + + + + {{i18n "cancel"}} + } diff --git a/app/assets/javascripts/admin/addon/components/site-setting.hbs b/app/assets/javascripts/admin/addon/components/site-setting.hbs index ee75c96ea12..cacd568c653 100644 --- a/app/assets/javascripts/admin/addon/components/site-setting.hbs +++ b/app/assets/javascripts/admin/addon/components/site-setting.hbs @@ -51,7 +51,7 @@ -{{else if this.setting.overridden}} +{{else if this.overridden}} {{#if this.setting.secret}} {{/if}} diff --git a/app/assets/javascripts/admin/addon/mixins/setting-component.js b/app/assets/javascripts/admin/addon/mixins/setting-component.js index bddff5ffa21..e3945dd3a74 100644 --- a/app/assets/javascripts/admin/addon/mixins/setting-component.js +++ b/app/assets/javascripts/admin/addon/mixins/setting-component.js @@ -9,6 +9,7 @@ import JsonSchemaEditorModal from "discourse/components/modal/json-schema-editor import { ajax } from "discourse/lib/ajax"; import { fmt, propertyNotEqual } from "discourse/lib/computed"; import { splitString } from "discourse/lib/utilities"; +import { deepEqual } from "discourse-common/lib/object"; import discourseComputed, { bind } from "discourse-common/utils/decorators"; import I18n from "discourse-i18n"; import SiteSettingDefaultCategoriesModal from "../components/modal/site-setting-default-categories"; @@ -111,7 +112,7 @@ export default Mixin.create({ settingVal = ""; } - return bufferVal.toString() !== settingVal.toString(); + return !deepEqual(bufferVal, settingVal); }, @discourseComputed("setting", "buffered.value") @@ -278,7 +279,7 @@ export default Mixin.create({ @action resetDefault() { - this.set("buffered.value", this.get("setting.default")); + this.set("buffered.value", this.setting.default); }, @action diff --git a/app/assets/javascripts/admin/addon/mixins/setting-object.js b/app/assets/javascripts/admin/addon/mixins/setting-object.js index cdef6bc128c..7e6384b2eb4 100644 --- a/app/assets/javascripts/admin/addon/mixins/setting-object.js +++ b/app/assets/javascripts/admin/addon/mixins/setting-object.js @@ -2,6 +2,7 @@ import { computed } from "@ember/object"; import { readOnly } from "@ember/object/computed"; import Mixin from "@ember/object/mixin"; import { isPresent } from "@ember/utils"; +import { deepEqual } from "discourse-common/lib/object"; import discourseComputed from "discourse-common/utils/decorators"; import I18n from "discourse-i18n"; @@ -15,7 +16,7 @@ export default Mixin.create({ defaultVal = ""; } - return val.toString() !== defaultVal.toString(); + return !deepEqual(val, defaultVal); }, computedValueProperty: computed( 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 83e82f87e7a..a0e935345bf 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 @@ -6,8 +6,8 @@ export default class AdminCustomizeThemesShowSchemaRoute extends Route { const setting = theme.settings.findBy("setting", params.setting_name); return { - data: setting.value, - schema: setting.objects_schema, + theme, + setting, }; } diff --git a/app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs b/app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs index 509c1b71ff6..7ae2afb44fb 100644 --- a/app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs +++ b/app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs @@ -1,4 +1,4 @@ \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/lib/computed.js b/app/assets/javascripts/discourse/app/lib/computed.js index a7fac989072..7a56bd051ed 100644 --- a/app/assets/javascripts/discourse/app/lib/computed.js +++ b/app/assets/javascripts/discourse/app/lib/computed.js @@ -1,6 +1,7 @@ import { computed } from "@ember/object"; import { htmlSafe as htmlSafeTemplateHelper } from "@ember/template"; import getURL from "discourse-common/lib/get-url"; +import { deepEqual } from "discourse-common/lib/object"; import I18n from "discourse-i18n"; function addonFmt(str, formats) { @@ -39,7 +40,7 @@ function addonFmt(str, formats) { export function propertyEqual(p1, p2) { return computed(p1, p2, function () { - return this.get(p1) === this.get(p2); + return deepEqual(this.get(p1), this.get(p2)); }); } @@ -53,7 +54,7 @@ export function propertyEqual(p1, p2) { **/ export function propertyNotEqual(p1, p2) { return computed(p1, p2, function () { - return this.get(p1) !== this.get(p2); + return !deepEqual(this.get(p1), this.get(p2)); }); } diff --git a/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js b/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js index a445c838c19..cc5b21042fe 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js +++ b/app/assets/javascripts/discourse/tests/fixtures/theme-setting-schema-data.js @@ -1,3 +1,5 @@ +import ThemeSettings from "admin/models/theme-settings"; + export default function schemaAndData(version = 1) { let schema, data; if (version === 1) { @@ -196,5 +198,10 @@ export default function schemaAndData(version = 1) { } else { throw new Error("unknown fixture version"); } - return [schema, data]; + + return ThemeSettings.create({ + objects_schema: schema, + value: data, + setting: "objects_setting" + }); } diff --git a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs index d11243cb73f..75ce2171da8 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs +++ b/app/assets/javascripts/discourse/tests/integration/components/admin-schema-theme-setting/editor-test.gjs @@ -60,9 +60,10 @@ module( setupRenderingTest(hooks); test("activates the first node by default", async function (assert) { - const [schema, data] = schemaAndData(1); + const setting = schemaAndData(1); + await render(); const tree = new TreeFromDOM(); @@ -73,9 +74,10 @@ module( }); test("renders the 2nd level of nested items for the active item only", async function (assert) { - const [schema, data] = schemaAndData(1); + const setting = schemaAndData(1); + await render(); const tree = new TreeFromDOM(); @@ -114,9 +116,10 @@ module( }); test("allows navigating through multiple levels of nesting", async function (assert) { - const [schema, data] = schemaAndData(1); + const setting = schemaAndData(1); + await render(); const tree = new TreeFromDOM(); @@ -191,9 +194,10 @@ module( }); test("the back button is only shown when the navigation is at least one level deep", async function (assert) { - const [schema, data] = schemaAndData(1); + const setting = schemaAndData(1); + await render(); assert.dom(".back-button").doesNotExist(); @@ -225,9 +229,10 @@ module( }); test("the back button navigates to the index of the active element at the previous level", async function (assert) { - const [schema, data] = schemaAndData(1); + const setting = schemaAndData(1); + await render(); const tree = new TreeFromDOM(); @@ -251,9 +256,10 @@ module( }); test("the back button label includes the name of the item at the previous level", async function (assert) { - const [schema, data] = schemaAndData(1); + const setting = schemaAndData(1); + await render(); const tree = new TreeFromDOM(); @@ -287,9 +293,10 @@ module( }); test("input fields for items at different levels", async function (assert) { - const [schema, data] = schemaAndData(2); + const setting = schemaAndData(2); + await render(); const inputFields = new InputFieldsFromDOM(); @@ -332,9 +339,10 @@ module( }); test("input fields of type integer", async function (assert) { - const [schema, data] = schemaAndData(3); + const setting = schemaAndData(3); + await render(); const inputFields = new InputFieldsFromDOM(); @@ -364,9 +372,10 @@ module( }); test("input fields of type boolean", async function (assert) { - const [schema, data] = schemaAndData(3); + const setting = schemaAndData(3); + await render(); const inputFields = new InputFieldsFromDOM(); @@ -393,9 +402,10 @@ module( }); test("input fields of type enum", async function (assert) { - const [schema, data] = schemaAndData(3); + const setting = schemaAndData(3); + await render(); const inputFields = new InputFieldsFromDOM(); @@ -419,9 +429,10 @@ module( }); test("identifier field instantly updates in the navigation tree when the input field is changed", async function (assert) { - const [schema, data] = schemaAndData(2); + const setting = schemaAndData(2); + await render(); const inputFields = new InputFieldsFromDOM(); @@ -448,9 +459,10 @@ module( }); test("edits are remembered when navigating between levels", async function (assert) { - const [schema, data] = schemaAndData(2); + const setting = schemaAndData(2); + await render(); const inputFields = new InputFieldsFromDOM(); diff --git a/spec/system/admin_customize_themes_spec.rb b/spec/system/admin_customize_themes_spec.rb index 339b47c191a..4529fb16c92 100644 --- a/spec/system/admin_customize_themes_spec.rb +++ b/spec/system/admin_customize_themes_spec.rb @@ -83,70 +83,4 @@ describe "Admin Customize Themes", type: :system do expect(ace_content.text).to eq("console.log('test')") end end - - describe "when editing a theme setting of objects type" do - let(:objects_setting) do - theme.set_field( - target: :settings, - name: "yaml", - value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"), - ) - - theme.save! - theme.settings[:objects_setting] - end - - before do - SiteSetting.experimental_objects_type_for_theme_settings = true - objects_setting - end - - it "should allow admin to edit the theme setting of objecst type" do - visit("/admin/customize/themes/#{theme.id}") - - admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting") - - expect(page).to have_current_path( - "/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/admin_editing_objects_typed_theme_setting_spec.rb b/spec/system/admin_editing_objects_typed_theme_setting_spec.rb new file mode 100644 index 00000000000..97977854ee4 --- /dev/null +++ b/spec/system/admin_editing_objects_typed_theme_setting_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +RSpec.describe "Admin editing objects type theme setting", type: :system do + fab!(:admin) + fab!(:theme) + + let(:objects_setting) do + theme.set_field( + target: :settings, + name: "yaml", + value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"), + ) + + theme.save! + theme.settings[:objects_setting] + end + + let(:admin_customize_themes_page) { PageObjects::Pages::AdminCustomizeThemes.new } + + let(:admin_objects_theme_setting_editor_page) do + PageObjects::Pages::AdminObjectsThemeSettingEditor.new + end + + before do + SiteSetting.experimental_objects_type_for_theme_settings = true + objects_setting + sign_in(admin) + end + + describe "when editing a theme setting of objects type" do + it "should allow admin to edit the theme setting of objects type" do + visit("/admin/customize/themes/#{theme.id}") + + expect(admin_customize_themes_page).to have_no_overriden_setting("objects_setting") + + admin_objects_theme_setting_editor = + admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting") + + expect(page).to have_current_path( + "/admin/customize/themes/#{theme.id}/schema/objects_setting", + ) + + admin_objects_theme_setting_editor.fill_in_field("name", "some new name").save + + expect(admin_customize_themes_page).to have_overridden_setting("objects_setting") + + 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") + 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/pages/admin_customize_themes.rb b/spec/system/page_objects/pages/admin_customize_themes.rb index bd255a24092..8a274adcb6a 100644 --- a/spec/system/page_objects/pages/admin_customize_themes.rb +++ b/spec/system/page_objects/pages/admin_customize_themes.rb @@ -15,6 +15,20 @@ module PageObjects has_css?(".select-inactive-mode") end + def has_overridden_setting?(setting_name) + has_css?(overridden_setting_selector(setting_name)) + end + + def has_no_overriden_setting?(setting_name) + has_no_css?(overridden_setting_selector(setting_name)) + end + + def reset_overridden_setting(setting_name) + setting_section = find("section.theme.settings .setting[data-setting=\"#{setting_name}\"]") + setting_section.click_button(I18n.t("admin_js.admin.settings.reset")) + setting_section.find(".setting-controls .ok").click + end + def click_select_inactive_mode find(".select-inactive-mode").click end @@ -41,12 +55,19 @@ module PageObjects def click_edit_objects_theme_setting_button(setting_name) find(".theme-setting[data-setting=\"#{setting_name}\"] .setting-value-edit-button").click + PageObjects::Pages::AdminObjectsThemeSettingEditor.new end def click_theme_settings_editor_button click_button(I18n.t("admin_js.admin.customize.theme.settings_editor")) PageObjects::Components::AdminThemeSettingsEditor.new end + + private + + def overridden_setting_selector(setting_name) + "section.theme.settings .setting.overridden[data-setting=\"#{setting_name}\"]" + end end end end 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 new file mode 100644 index 00000000000..48e7308fdb9 --- /dev/null +++ b/spec/system/page_objects/pages/admin_objects_theme_setting_editor.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class AdminObjectsThemeSettingEditor < PageObjects::Pages::Base + def has_setting_field?(field_name, value) + expect(input_field(field_name).value).to eq(value) + end + + def fill_in_field(field_name, value) + input_field(field_name).fill_in(with: value) + self + end + + def save + click_button(I18n.t("js.save")) + self + end + + private + + def input_field(field_name) + page.find(".schema-field[data-name=\"#{field_name}\"] input") + end + end + end +end