From 007cce62e6863192d9336d8340b87c614db495a1 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Wed, 1 Mar 2023 10:12:39 +0100 Subject: [PATCH] DEV: Clean up settings component (#20485) Async, modern syntax, no `on()` component hooks, const extraction, sorted props, template tweaks, and a small filtering bugfix (filtering could throw errors after saving a category-selection setting) --- .../admin/addon/components/site-setting.hbs | 28 +-- .../addon/controllers/admin-site-settings.js | 4 +- .../admin/addon/mixins/setting-component.js | 188 +++++++++--------- 3 files changed, 112 insertions(+), 108 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/site-setting.hbs b/app/assets/javascripts/admin/addon/components/site-setting.hbs index e73d8697742..1e72fadac17 100644 --- a/app/assets/javascripts/admin/addon/components/site-setting.hbs +++ b/app/assets/javascripts/admin/addon/components/site-setting.hbs @@ -1,7 +1,8 @@

+ {{this.settingName}} + {{#if this.staffLogFilter}} - {{this.settingName}} - {{else}} - {{this.settingName}} {{/if}}

+ {{#if this.defaultIsAvailable}} - {{this.setting.setDefaultValuesLabel}} + {{/if}}
+
{{component this.componentName @@ -33,18 +35,20 @@ allowAny=this.allowAny }}
+ {{#if this.dirty}}
- - + +
{{else if this.setting.overridden}} {{#if this.setting.secret}} - + {{/if}} + diff --git a/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js b/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js index 4ee3b0fab92..80355cddc7b 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-site-settings.js @@ -14,7 +14,7 @@ export default Controller.extend({ filterContentNow(category) { // If we have no content, don't bother filtering anything - if (!!isEmpty(this.allSiteSettings)) { + if (isEmpty(this.allSiteSettings)) { return; } @@ -73,7 +73,7 @@ export default Controller.extend({ setting.includes(filter) || setting.replace(/_/g, " ").includes(filter) || item.get("description").toLowerCase().includes(filter) || - (item.get("value") || "").toLowerCase().includes(filter) + (item.get("value") || "").toString().toLowerCase().includes(filter) ); } else { return true; diff --git a/app/assets/javascripts/admin/addon/mixins/setting-component.js b/app/assets/javascripts/admin/addon/mixins/setting-component.js index 6b9bfbda105..0be2ccdd752 100644 --- a/app/assets/javascripts/admin/addon/mixins/setting-component.js +++ b/app/assets/javascripts/admin/addon/mixins/setting-component.js @@ -3,12 +3,10 @@ import { fmt, propertyNotEqual } from "discourse/lib/computed"; import { alias, oneWay } from "@ember/object/computed"; import I18n from "I18n"; import Mixin from "@ember/object/mixin"; -import { Promise } from "rsvp"; import { ajax } from "discourse/lib/ajax"; import { categoryLinkHTML } from "discourse/helpers/category-link"; import discourseComputed, { bind } from "discourse-common/utils/decorators"; import { htmlSafe } from "@ember/template"; -import { on } from "@ember/object/evented"; import showModal from "discourse/lib/show-modal"; import { warn } from "@ember/debug"; import { action } from "@ember/object"; @@ -37,13 +35,61 @@ const CUSTOM_TYPES = [ const AUTO_REFRESH_ON_SAVE = ["logo", "logo_small", "large_icon"]; +const DEFAULT_USER_PREFERENCES = [ + "default_email_digest_frequency", + "default_include_tl0_in_digests", + "default_email_level", + "default_email_messages_level", + "default_email_mailing_list_mode", + "default_email_mailing_list_mode_frequency", + "default_email_previous_replies", + "default_email_in_reply_to", + "default_hide_profile_and_presence", + "default_other_new_topic_duration_minutes", + "default_other_auto_track_topics_after_msecs", + "default_other_notification_level_when_replying", + "default_other_external_links_in_new_tab", + "default_other_enable_quoting", + "default_other_enable_defer", + "default_other_dynamic_favicon", + "default_other_like_notification_frequency", + "default_other_skip_new_user_tips", + "default_topics_automatic_unpin", + "default_categories_watching", + "default_categories_tracking", + "default_categories_muted", + "default_categories_watching_first_post", + "default_categories_normal", + "default_tags_watching", + "default_tags_tracking", + "default_tags_muted", + "default_tags_watching_first_post", + "default_text_size", + "default_title_count_mode", + "default_sidebar_categories", + "default_sidebar_tags", +]; + export default Mixin.create({ - classNameBindings: [":row", ":setting", "overridden", "typeClass"], - content: alias("setting"), - validationMessage: null, - isSecret: oneWay("setting.secret"), - setting: null, attributeBindings: ["setting.setting:data-setting"], + classNameBindings: [":row", ":setting", "overridden", "typeClass"], + validationMessage: null, + setting: null, + + content: alias("setting"), + isSecret: oneWay("setting.secret"), + componentName: fmt("typeClass", "site-settings/%@"), + overridden: propertyNotEqual("setting.default", "buffered.value"), + + didInsertElement() { + this._super(...arguments); + this.element.addEventListener("keydown", this._handleKeydown); + }, + + willDestroyElement() { + this._super(...arguments); + this.element.removeEventListener("keydown", this._handleKeydown); + }, @discourseComputed("buffered.value", "setting.value") dirty(bufferVal, settingVal) { @@ -99,15 +145,11 @@ export default Mixin.create({ return setting.type; }, - componentName: fmt("typeClass", "site-settings/%@"), - @discourseComputed("setting.anyValue") allowAny(anyValue) { return anyValue !== false; }, - overridden: propertyNotEqual("setting.default", "buffered.value"), - @discourseComputed("buffered.value") bufferedValues(value) { return splitString(value, "|"); @@ -127,90 +169,57 @@ export default Mixin.create({ }, @action - update() { - const defaultUserPreferences = [ - "default_email_digest_frequency", - "default_include_tl0_in_digests", - "default_email_level", - "default_email_messages_level", - "default_email_mailing_list_mode", - "default_email_mailing_list_mode_frequency", - "default_email_previous_replies", - "default_email_in_reply_to", - "default_hide_profile_and_presence", - "default_other_new_topic_duration_minutes", - "default_other_auto_track_topics_after_msecs", - "default_other_notification_level_when_replying", - "default_other_external_links_in_new_tab", - "default_other_enable_quoting", - "default_other_enable_defer", - "default_other_dynamic_favicon", - "default_other_like_notification_frequency", - "default_other_skip_new_user_tips", - "default_topics_automatic_unpin", - "default_categories_watching", - "default_categories_tracking", - "default_categories_muted", - "default_categories_watching_first_post", - "default_categories_normal", - "default_tags_watching", - "default_tags_tracking", - "default_tags_muted", - "default_tags_watching_first_post", - "default_text_size", - "default_title_count_mode", - "default_sidebar_categories", - "default_sidebar_tags", - ]; - + async update() { const key = this.buffered.get("setting"); - if (defaultUserPreferences.includes(key)) { - const data = {}; - data[key] = this.buffered.get("value"); + if (!DEFAULT_USER_PREFERENCES.includes(key)) { + await this.save(); + return; + } - ajax(`/admin/site_settings/${key}/user_count.json`, { - type: "PUT", - data, - }).then((result) => { - const count = result.user_count; + const data = { + [key]: this.buffered.get("value"), + }; - if (count > 0) { - const controller = showModal("site-setting-default-categories", { - model: { count, key: key.replaceAll("_", " ") }, - admin: true, - }); + const result = await ajax(`/admin/site_settings/${key}/user_count.json`, { + type: "PUT", + data, + }); - controller.set("onClose", () => { - this.updateExistingUsers = controller.updateExistingUsers; - this.save(); - }); - } else { - this.save(); - } + const count = result.user_count; + + if (count > 0) { + const controller = showModal("site-setting-default-categories", { + model: { count, key: key.replaceAll("_", " ") }, + admin: true, + }); + + controller.set("onClose", () => { + this.updateExistingUsers = controller.updateExistingUsers; + this.save(); }); } else { - this.save(); + await this.save(); } }, @action - save() { - this._save() - .then(() => { - this.set("validationMessage", null); - this.commitBuffer(); - if (AUTO_REFRESH_ON_SAVE.includes(this.setting.setting)) { - this.afterSave(); - } - }) - .catch((e) => { - if (e.jqXHR?.responseJSON?.errors) { - this.set("validationMessage", e.jqXHR.responseJSON.errors[0]); - } else { - this.set("validationMessage", I18n.t("generic_error")); - } - }); + async save() { + try { + await this._save(); + + this.set("validationMessage", null); + this.commitBuffer(); + if (AUTO_REFRESH_ON_SAVE.includes(this.setting.setting)) { + this.afterSave(); + } + } catch (e) { + if (e.jqXHR?.responseJSON?.errors) { + this.set("validationMessage", e.jqXHR.responseJSON.errors[0]); + } else { + this.set("validationMessage", I18n.t("generic_error")); + } + } }, @action @@ -247,18 +256,9 @@ export default Mixin.create({ } }, - _watchEnterKey: on("didInsertElement", function () { - this.element.addEventListener("keydown", this._handleKeydown); - }), - - _removeBindings: on("willDestroyElement", function () { - this.element.removeEventListener("keydown", this._handleKeydown); - }), - - _save() { + async _save() { warn("You should define a `_save` method", { id: "discourse.setting-component.missing-save", }); - return Promise.resolve(); }, });