DEV: Add save button to editing typed objects theme setting route (#26133)

Why this change?

This is still a work in progress but allows objects type theme setting
to be saved.

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}}
+
       <ul class="tree">
         {{#each this.tree.nodes as |node|}}
           <div class="item-container">
@@ -191,6 +236,7 @@ export default class SchemaThemeSettingEditor extends Component {
           </div>
         {{/each}}
       </ul>
+
       {{#each this.fields as |field|}}
         <FieldInput
           @name={{field.name}}
@@ -200,5 +246,20 @@ export default class SchemaThemeSettingEditor extends Component {
         />
       {{/each}}
     </div>
+
+    <DButton
+      @disabled={{this.saveButtonDisabled}}
+      @action={{this.saveChanges}}
+      @label="save"
+      class="btn-primary"
+    />
+
+    <LinkTo
+      @route="adminCustomizeThemes.show"
+      @model={{@themeId}}
+      class="btn-transparent"
+    >
+      {{i18n "cancel"}}
+    </LinkTo>
   </template>
 }

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

@@ -51,7 +51,7 @@
     <DButton class="ok" @action={{this.update}} @icon="check" />
     <DButton class="cancel" @action={{this.cancel}} @icon="times" />
   </div>
-{{else if this.setting.overridden}}
+{{else if this.overridden}}
   {{#if this.setting.secret}}
     <DButton @action={{this.toggleSecret}} @icon="far-eye-slash" />
   {{/if}}

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

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(

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,
     };
   }
 

app/assets/javascripts/admin/addon/templates/customize-themes-show-schema.hbs

@@ -1,4 +1,4 @@
 <SchemaThemeSetting::Editor
-  @schema={{this.model.schema}}
-  @data={{this.model.data}}
+  @themeId={{@model.theme.id}}
+  @setting={{@model.setting}}
 />

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));
   });
 }
 

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"
+  });
 }

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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       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(<template>
-        <AdminSchemaThemeSettingEditor @schema={{schema}} @data={{data}} />
+        <AdminSchemaThemeSettingEditor @themeId="1" @setting={{setting}} />
       </template>);
 
       const inputFields = new InputFieldsFromDOM();

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

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

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

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