From a92cf019db82a95614e4003101bb62636b47a1e8 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Thu, 15 Aug 2024 19:38:47 +0300 Subject: [PATCH] FIX: Make cancel and reset buttons work for `file_size_restriction` settings (#28347) This commit fixes a number of bugs in `file_size_restriction` settings and does a little of refactoring to reduce duplicated code in site setting types (the refactoring is necessary to fix one of the bugs). The bugs in `file_size_restriction` settings that are fixed in this commit: 1. Save/cancel buttons next to a `file_size_restriction` setting are shown upon navigating to the settings page without changes being made to the setting 2. Cancel button that discards changes made to the setting doesn't work 3. Reset button that resets the setting to its default doesn't work 4. Validation error message isn't cleared when resetting/cancelling changes To repro those bugs, navigate to `/admin/site_settings/category/files` and observe the top 2 settings in the page (`max image size kb` and `max attachment size kb`). Internal topic: t/134726. --- .../addon/components/emoji-value-list.hbs | 4 +- .../addon/components/emoji-value-list.js | 7 +- .../addon/components/file-size-input.gjs | 139 ++++++-------- .../addon/components/secret-value-list.hbs | 4 +- .../addon/components/secret-value-list.js | 6 +- .../admin/addon/components/site-setting.hbs | 9 +- .../addon/components/site-settings/bool.hbs | 4 +- .../site-settings/category-list.gjs | 5 - .../components/site-settings/category.hbs | 4 +- .../addon/components/site-settings/color.hbs | 4 +- .../components/site-settings/compact-list.hbs | 5 +- .../components/site-settings/emoji-list.hbs | 8 +- .../addon/components/site-settings/enum.hbs | 5 +- .../site-settings/file-size-restriction.gjs | 29 +-- .../site-settings/file-types-list.gjs | 5 - .../components/site-settings/group-list.hbs | 4 +- .../components/site-settings/host-list.hbs | 5 +- .../components/site-settings/integer.gjs | 5 - .../addon/components/site-settings/list.hbs | 4 +- .../components/site-settings/named-list.hbs | 5 +- .../components/site-settings/secret-list.hbs | 5 +- .../components/site-settings/simple-list.hbs | 4 +- .../addon/components/site-settings/string.hbs | 5 +- .../site-settings/tag-group-list.hbs | 4 +- .../components/site-settings/tag-list.hbs | 4 +- .../addon/components/site-settings/upload.hbs | 4 +- .../site-settings/uploaded-image-list.hbs | 4 +- .../components/site-settings/url-list.hbs | 4 +- .../components/site-settings/value-list.hbs | 4 +- .../admin/addon/mixins/setting-component.js | 15 ++ .../components/file-size-input-test.js | 82 +++++++-- .../components/secret-value-list-test.js | 46 ++++- .../components/site-setting-test.js | 172 ++++++++++++++++++ .../common/components/file-size-input.scss | 1 + config/locales/client.en.yml | 1 + 35 files changed, 400 insertions(+), 216 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/emoji-value-list.hbs b/app/assets/javascripts/admin/addon/components/emoji-value-list.hbs index 56c883807fc..1be68d11178 100644 --- a/app/assets/javascripts/admin/addon/components/emoji-value-list.hbs +++ b/app/assets/javascripts/admin/addon/components/emoji-value-list.hbs @@ -56,6 +56,4 @@ @isEditorFocused={{this.isEditorFocused}} @emojiSelected={{this.emojiSelected}} @onEmojiPickerClose={{this.closeEmojiPicker}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/emoji-value-list.js b/app/assets/javascripts/admin/addon/components/emoji-value-list.js index b788ecefc00..0166c48ed07 100644 --- a/app/assets/javascripts/admin/addon/components/emoji-value-list.js +++ b/app/assets/javascripts/admin/addon/components/emoji-value-list.js @@ -10,7 +10,6 @@ import I18n from "discourse-i18n"; @classNameBindings(":value-list", ":emoji-list") export default class EmojiValueList extends Component { values = null; - validationMessage = null; emojiPickerIsActive = false; isEditorFocused = false; @@ -137,16 +136,14 @@ export default class EmojiValueList extends Component { } _validateInput(input) { - this.set("validationMessage", null); - if (!emojiUrlFor(input)) { - this.set( - "validationMessage", + this.setValidationMessage( I18n.t("admin.site_settings.emoji_list.invalid_input") ); return false; } + this.setValidationMessage(null); return true; } diff --git a/app/assets/javascripts/admin/addon/components/file-size-input.gjs b/app/assets/javascripts/admin/addon/components/file-size-input.gjs index 77df855b90c..b4c144d2b2f 100644 --- a/app/assets/javascripts/admin/addon/components/file-size-input.gjs +++ b/app/assets/javascripts/admin/addon/components/file-size-input.gjs @@ -2,9 +2,6 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; import { on } from "@ember/modifier"; import { action } from "@ember/object"; -import didInsert from "@ember/render-modifiers/modifiers/did-insert"; -import didUpdate from "@ember/render-modifiers/modifiers/did-update"; -import TextField from "discourse/components/text-field"; import { allowOnlyNumericInput } from "discourse/lib/utilities"; import I18n from "discourse-i18n"; import ComboBox from "select-kit/components/combo-box"; @@ -14,30 +11,34 @@ const UNIT_MB = "mb"; const UNIT_GB = "gb"; export default class FileSizeInput extends Component { - @tracked fileSizeUnit; - @tracked sizeValue; - @tracked pendingSizeValue; - @tracked pendingFileSizeUnit; + @tracked unit; - constructor(owner, args) { - super(owner, args); - this.originalSizeKB = this.args.sizeValueKB; - this.sizeValue = this.args.sizeValueKB; + constructor() { + super(...arguments); - this._defaultUnit(); + const sizeInKB = this.args.sizeValueKB; + if (sizeInKB >= 1024 * 1024) { + this.unit = UNIT_GB; + } else if (sizeInKB >= 1024) { + this.unit = UNIT_MB; + } else { + this.unit = UNIT_KB; + } } - _defaultUnit() { - this.fileSizeUnit = UNIT_KB; - if (this.originalSizeKB <= 1024) { - this.onFileSizeUnitChange(UNIT_KB); - } else if ( - this.originalSizeKB > 1024 && - this.originalSizeKB <= 1024 * 1024 - ) { - this.onFileSizeUnitChange(UNIT_MB); - } else if (this.originalSizeKB > 1024 * 1024) { - this.onFileSizeUnitChange(UNIT_GB); + get number() { + const sizeInKB = this.args.sizeValueKB; + if (!sizeInKB) { + return; + } + if (this.unit === UNIT_KB) { + return sizeInKB; + } + if (this.unit === UNIT_MB) { + return sizeInKB / 1024; + } + if (this.unit === UNIT_GB) { + return sizeInKB / 1024 / 1024; } } @@ -55,95 +56,69 @@ export default class FileSizeInput extends Component { } @action - handleFileSizeChange(value) { - if (value !== "") { - this.pendingSizeValue = value; - this._onFileSizeChange(value); - } - } + handleFileSizeChange(event) { + const value = parseFloat(event.target.value); - _onFileSizeChange(newSize) { - let fileSizeKB; - switch (this.fileSizeUnit) { + if (isNaN(value)) { + this.args.onChangeSize(); + return; + } + + let sizeInKB; + switch (this.unit) { case "kb": - fileSizeKB = newSize; + sizeInKB = value; break; case "mb": - fileSizeKB = newSize * 1024; + sizeInKB = value * 1024; break; case "gb": - fileSizeKB = newSize * 1024 * 1024; + sizeInKB = value * 1024 * 1024; break; } - if (fileSizeKB > this.args.max) { - this.args.updateValidationMessage( + + this.args.onChangeSize(sizeInKB); + + if (sizeInKB > this.args.max) { + this.args.setValidationMessage( I18n.t("file_size_input.error.size_too_large", { - provided_file_size: I18n.toHumanSize(fileSizeKB * 1024), + provided_file_size: I18n.toHumanSize(sizeInKB * 1024), max_file_size: I18n.toHumanSize(this.args.max * 1024), }) ); - // Removes the green save checkmark button - this.args.onChangeSize(this.originalSizeKB); + } else if (sizeInKB < this.args.min) { + this.args.setValidationMessage( + I18n.t("file_size_input.error.size_too_small", { + provided_file_size: I18n.toHumanSize(sizeInKB * 1024), + min_file_size: I18n.toHumanSize(this.args.min * 1024), + }) + ); } else { - this.args.onChangeSize(fileSizeKB); - this.args.updateValidationMessage(null); + this.args.setValidationMessage(null); } } @action onFileSizeUnitChange(newUnit) { - if (this.fileSizeUnit === "kb" && newUnit === "kb") { - this.pendingSizeValue = this.sizeValue; - } - if (this.fileSizeUnit === "kb" && newUnit === "mb") { - this.pendingSizeValue = this.sizeValue / 1024; - } - if (this.fileSizeUnit === "kb" && newUnit === "gb") { - this.pendingSizeValue = this.sizeValue / 1024 / 1024; - } - if (this.fileSizeUnit === "mb" && newUnit === "kb") { - this.pendingSizeValue = this.sizeValue * 1024; - } - if (this.fileSizeUnit === "mb" && newUnit === "gb") { - this.pendingSizeValue = this.sizeValue / 1024; - } - if (this.fileSizeUnit === "gb" && newUnit === "mb") { - this.pendingSizeValue = this.sizeValue * 1024; - } - if (this.fileSizeUnit === "gb" && newUnit === "kb") { - this.pendingSizeValue = this.sizeValue * 1024 * 1024; - } - this.pendingFileSizeUnit = newUnit; - } - - @action - applySizeValueChanges() { - this.sizeValue = this.pendingSizeValue; - } - - @action - applyUnitChanges() { - this.fileSizeUnit = this.pendingFileSizeUnit; + this.unit = newUnit; } diff --git a/app/assets/javascripts/admin/addon/components/secret-value-list.hbs b/app/assets/javascripts/admin/addon/components/secret-value-list.hbs index 67b9e460a5e..9203ebc14dd 100644 --- a/app/assets/javascripts/admin/addon/components/secret-value-list.hbs +++ b/app/assets/javascripts/admin/addon/components/secret-value-list.hbs @@ -40,6 +40,4 @@ @icon="plus" class="add-value-btn btn-small" /> - - - \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/secret-value-list.js b/app/assets/javascripts/admin/addon/components/secret-value-list.js index ecd12a83049..1bc7f87c6d0 100644 --- a/app/assets/javascripts/admin/addon/components/secret-value-list.js +++ b/app/assets/javascripts/admin/addon/components/secret-value-list.js @@ -9,7 +9,6 @@ export default class SecretValueList extends Component { inputDelimiter = null; collection = null; values = null; - validationMessage = null; didReceiveAttrs() { super.didReceiveAttrs(...arguments); @@ -57,16 +56,15 @@ export default class SecretValueList extends Component { } _checkInvalidInput(inputs) { - this.set("validationMessage", null); for (let input of inputs) { if (isEmpty(input) || input.includes("|")) { - this.set( - "validationMessage", + this.setValidationMessage( I18n.t("admin.site_settings.secret_list.invalid_input") ); return true; } } + this.setValidationMessage(null); } _addValue(value, secret) { diff --git a/app/assets/javascripts/admin/addon/components/site-setting.hbs b/app/assets/javascripts/admin/addon/components/site-setting.hbs index bb8228fd44d..c8047733e6b 100644 --- a/app/assets/javascripts/admin/addon/components/site-setting.hbs +++ b/app/assets/javascripts/admin/addon/components/site-setting.hbs @@ -39,12 +39,16 @@ this.componentName setting=this.setting value=this.buffered.value - validationMessage=this.validationMessage preview=this.preview isSecret=this.isSecret allowAny=this.allowAny changeValueCallback=this.changeValueCallback + setValidationMessage=this.setValidationMessage }} + + {{#if this.displayDescription}} + + {{/if}} {{/if}} @@ -53,6 +57,7 @@ {{html-safe this.setting.description}} - - - \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/category-list.gjs b/app/assets/javascripts/admin/addon/components/site-settings/category-list.gjs index 01776dbfb74..d57972232a8 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/category-list.gjs +++ b/app/assets/javascripts/admin/addon/components/site-settings/category-list.gjs @@ -3,8 +3,6 @@ import { tracked } from "@glimmer/tracking"; import { action } from "@ember/object"; import didUpdate from "@ember/render-modifiers/modifiers/did-update"; import Category from "discourse/models/category"; -import SettingValidationMessage from "admin/components/setting-validation-message"; -import SiteSettingsDescription from "admin/components/site-settings/description"; import CategorySelector from "select-kit/components/category-selector"; export default class CategoryList extends Component { @@ -50,9 +48,6 @@ export default class CategoryList extends Component { @categories={{this.selectedCategories}} @onChange={{this.onChangeSelectedCategories}} /> - - - } diff --git a/app/assets/javascripts/admin/addon/components/site-settings/category.hbs b/app/assets/javascripts/admin/addon/components/site-settings/category.hbs index bc008e28013..4b281b90d33 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/category.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/category.hbs @@ -2,6 +2,4 @@ @value={{this.value}} @onChange={{fn (mut this.value)}} @options={{hash allowUncategorized=true none=(eq this.setting.default "")}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/color.hbs b/app/assets/javascripts/admin/addon/components/site-settings/color.hbs index 4a706f2f6b3..3bbf69af09e 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/color.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/color.hbs @@ -4,6 +4,4 @@ @onlyHex={{false}} @styleSelection={{false}} @onChangeColor={{this.onChangeColor}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/compact-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/compact-list.hbs index df02a95358c..bd6989af094 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/compact-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/compact-list.hbs @@ -5,7 +5,4 @@ @onChange={{this.onChangeListSetting}} @onChangeChoices={{this.onChangeChoices}} @options={{hash allowAny=this.allowAny}} -/> - - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/emoji-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/emoji-list.hbs index 549b4495533..bbedad6cf9b 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/emoji-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/emoji-list.hbs @@ -1,3 +1,5 @@ - - - \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/enum.hbs b/app/assets/javascripts/admin/addon/components/site-settings/enum.hbs index a4343afe21d..a9ed4c42ddb 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/enum.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/enum.hbs @@ -7,7 +7,4 @@ @options={{hash castInteger=true allowAny=this.setting.allowsNone}} /> -{{this.preview}} - - - \ No newline at end of file +{{this.preview}} \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/file-size-restriction.gjs b/app/assets/javascripts/admin/addon/components/site-settings/file-size-restriction.gjs index 15d356a1330..9af6840fa30 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/file-size-restriction.gjs +++ b/app/assets/javascripts/admin/addon/components/site-settings/file-size-restriction.gjs @@ -1,34 +1,23 @@ import Component from "@glimmer/component"; -import { tracked } from "@glimmer/tracking"; -import { fn } from "@ember/helper"; import { action } from "@ember/object"; import FileSizeInput from "admin/components/file-size-input"; -import SettingValidationMessage from "admin/components/setting-validation-message"; -import SiteSettingsDescription from "admin/components/site-settings/description"; export default class FileSizeRestriction extends Component { - @tracked _validationMessage = this.args.validationMessage; - @action - updateValidationMessage(message) { - this._validationMessage = message; - } - - get validationMessage() { - return this._validationMessage ?? this.args.validationMessage; + changeSize(newValue) { + // Settings are stored as strings, this way the main site setting component + // doesn't get confused and think the value has changed from default if the + // admin sets it to the same number as the default. + this.args.changeValueCallback(newValue?.toString() ?? ""); } } diff --git a/app/assets/javascripts/admin/addon/components/site-settings/file-types-list.gjs b/app/assets/javascripts/admin/addon/components/site-settings/file-types-list.gjs index 7c5baf7c0e2..11fb7393ba9 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/file-types-list.gjs +++ b/app/assets/javascripts/admin/addon/components/site-settings/file-types-list.gjs @@ -8,8 +8,6 @@ import DButton from "discourse/components/d-button"; import i18n from "discourse-common/helpers/i18n"; import { makeArray } from "discourse-common/lib/helpers"; import I18n from "discourse-i18n"; -import SettingValidationMessage from "admin/components/setting-validation-message"; -import SiteSettingsDescription from "admin/components/site-settings/description"; import ListSetting from "select-kit/components/list-setting"; const IMAGE_TYPES = [ @@ -145,8 +143,5 @@ export default class FileTypesList extends Component { }} class="btn file-types-list__button document" /> - - - } diff --git a/app/assets/javascripts/admin/addon/components/site-settings/group-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/group-list.hbs index 9826bcf8e2b..d4b6a0592af 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/group-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/group-list.hbs @@ -6,6 +6,4 @@ @nameProperty={{this.nameProperty}} @valueProperty={{this.valueProperty}} @onChange={{this.onChangeGroupListSetting}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/host-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/host-list.hbs index 72632a82b64..b56d921cf05 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/host-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/host-list.hbs @@ -4,7 +4,4 @@ @choices={{this.settingValue}} @onChange={{this.onChange}} @options={{hash allowAny=this.allowAny}} -/> - - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs b/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs index 89b82a80d4b..211c516b4e4 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs +++ b/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs @@ -1,8 +1,6 @@ import Component from "@glimmer/component"; import { on } from "@ember/modifier"; import { action } from "@ember/object"; -import SettingValidationMessage from "admin/components/setting-validation-message"; -import SiteSettingDescription from "admin/components/site-settings/description"; export default class SiteSettingsInteger extends Component { @action @@ -37,8 +35,5 @@ export default class SiteSettingsInteger extends Component { class="input-setting-integer" step="1" /> - - - } diff --git a/app/assets/javascripts/admin/addon/components/site-settings/list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/list.hbs index ae472fc743a..e515fad9075 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/list.hbs @@ -2,6 +2,4 @@ @values={{this.value}} @inputDelimiter="|" @choices={{this.setting.choices}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/named-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/named-list.hbs index 78c9e82b051..7569b9a1b5d 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/named-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/named-list.hbs @@ -6,7 +6,4 @@ @valueProperty="value" @onChange={{this.onChangeListSetting}} @options={{hash allowAny=this.allowAny}} -/> - - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/secret-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/secret-list.hbs index cd36a749185..a15329ee5d3 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/secret-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/secret-list.hbs @@ -2,6 +2,5 @@ @setting={{this.setting}} @values={{this.value}} @isSecret={{this.isSecret}} -/> - - \ No newline at end of file + @setValidationMessage={{this.setValidationMessage}} +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/simple-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/simple-list.hbs index 7b795e98310..e4262696eab 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/simple-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/simple-list.hbs @@ -4,6 +4,4 @@ @onChange={{this.onChange}} @choices={{this.setting.choices}} @allowAny={{this.setting.allow_any}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/string.hbs b/app/assets/javascripts/admin/addon/components/site-settings/string.hbs index d9ac8d61cc6..283bd94d52e 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/string.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/string.hbs @@ -9,7 +9,4 @@ /> {{else}} -{{/if}} - - - \ No newline at end of file +{{/if}} \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/tag-group-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/tag-group-list.hbs index 179a328ebc8..7809b5a36e2 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/tag-group-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/tag-group-list.hbs @@ -2,6 +2,4 @@ @tagGroups={{this.selectedTagGroups}} @onChange={{this.onTagGroupChange}} @options={{hash filterPlaceholder="category.required_tag_group.placeholder"}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/tag-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/tag-list.hbs index 97933c52ff0..13bd33a0e4e 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/tag-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/tag-list.hbs @@ -4,6 +4,4 @@ @everyTag={{true}} @unlimitedTagCount={{true}} @options={{hash allowAny=false}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/upload.hbs b/app/assets/javascripts/admin/addon/components/site-settings/upload.hbs index 119af8ba5d3..cad0a33a03d 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/upload.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/upload.hbs @@ -4,6 +4,4 @@ @additionalParams={{hash for_site_setting=true}} @type="site_setting" @id={{concat "site-setting-image-uploader-" this.setting.setting}} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/uploaded-image-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/uploaded-image-list.hbs index ddb191e9ee4..06bf42d1c5f 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/uploaded-image-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/uploaded-image-list.hbs @@ -4,6 +4,4 @@ this.showUploadModal (hash value=this.value setting=this.setting) }} -/> - - \ No newline at end of file +/> \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/url-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/url-list.hbs index f33b0274278..54df5b9a43f 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/url-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/url-list.hbs @@ -1,3 +1 @@ - - - \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/components/site-settings/value-list.hbs b/app/assets/javascripts/admin/addon/components/site-settings/value-list.hbs index 3ca238d7d34..47fd877a49a 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/value-list.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/value-list.hbs @@ -1,3 +1 @@ - - - \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/javascripts/admin/addon/mixins/setting-component.js b/app/assets/javascripts/admin/addon/mixins/setting-component.js index 87362d3e638..a55010df72b 100644 --- a/app/assets/javascripts/admin/addon/mixins/setting-component.js +++ b/app/assets/javascripts/admin/addon/mixins/setting-component.js @@ -104,6 +104,10 @@ export default Mixin.create({ this.element.removeEventListener("keydown", this._handleKeydown); }, + displayDescription: computed("componentType", function () { + return this.componentType !== "bool"; + }), + dirty: computed("buffered.value", "setting.value", function () { let bufferVal = this.get("buffered.value"); let settingVal = this.setting?.value; @@ -211,6 +215,10 @@ export default Mixin.create({ } }), + disableSaveButton: computed("validationMessage", function () { + return !!this.validationMessage; + }), + confirmChanges(settingKey) { return new Promise((resolve) => { // Fallback is needed in case the setting does not have a custom confirmation @@ -324,12 +332,18 @@ export default Mixin.create({ this.set("buffered.value", value); }), + setValidationMessage: action(function (message) { + this.set("validationMessage", message); + }), + cancel: action(function () { this.rollbackBuffer(); + this.set("validationMessage", null); }), resetDefault: action(function () { this.set("buffered.value", this.setting.default); + this.set("validationMessage", null); }), toggleSecret: action(function () { @@ -341,6 +355,7 @@ export default Mixin.create({ "buffered.value", this.bufferedValues.concat(this.defaultValues).uniq().join("|") ); + this.set("validationMessage", null); return false; }), diff --git a/app/assets/javascripts/discourse/tests/integration/components/file-size-input-test.js b/app/assets/javascripts/discourse/tests/integration/components/file-size-input-test.js index 1305c3e9d91..2706debdb95 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/file-size-input-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/file-size-input-test.js @@ -8,39 +8,88 @@ module("Integration | Component | file-size-input", function (hooks) { setupRenderingTest(hooks); test("file size unit selector kb", async function (assert) { - this.set("value", 1024); + this.set("value", 1023); this.set("max", 4096); this.set("onChangeSize", () => {}); - this.set("updateValidationMessage", () => {}); + this.set("setValidationMessage", () => {}); await render(hbs` `); - assert.dom(".file-size-input").hasValue("1024", "value is present"); + assert.dom(".file-size-input").hasValue("1023", "value is present"); + assert.strictEqual( + selectKit(".file-size-unit-selector").header().value(), + "kb", + "the default unit is kb" + ); + }); + + test("file size unit is mb when the starting value is 1mb or more", async function (assert) { + this.set("value", 1024); + this.set("onChangeSize", () => {}); + this.set("setValidationMessage", () => {}); + + await render(hbs` + + `); + + assert.dom(".file-size-input").hasValue("1", "value is present"); + assert.strictEqual( + selectKit(".file-size-unit-selector").header().value(), + "mb", + "the default unit is mb" + ); + }); + + test("file size unit is gb when the starting value is 1gb or more", async function (assert) { + this.set("value", 1024 * 1024); + this.set("onChangeSize", () => {}); + this.set("setValidationMessage", () => {}); + + await render(hbs` + + `); + + assert.dom(".file-size-input").hasValue("1", "value is present"); + assert.strictEqual( + selectKit(".file-size-unit-selector").header().value(), + "gb", + "the default unit is gb" + ); }); test("file size unit selector", async function (assert) { this.set("value", 4096); this.set("max", 8192); this.set("onChangeSize", () => {}); - this.set("updateValidationMessage", () => {}); + this.set("setValidationMessage", () => {}); await render(hbs` `); @@ -91,21 +140,22 @@ module("Integration | Component | file-size-input", function (hooks) { test("file size input error message", async function (assert) { this.set("value", 4096); this.set("max", 8192); + this.set("min", 2048); this.set("onChangeSize", () => {}); - let updateValidationMessage = (message) => { + let setValidationMessage = (message) => { this.set("message", message); }; - this.set("updateValidationMessage", updateValidationMessage); + this.set("setValidationMessage", setValidationMessage); await render(hbs` `); @@ -124,5 +174,13 @@ module("Integration | Component | file-size-input", function (hooks) { null, "The message is cleared when the input is less than the max" ); + + await fillIn(".file-size-input", 1); + + assert.strictEqual( + this.message, + "1 MB is smaller than the min allowed 2 MB", + "A message is showed when the input is smaller than the min" + ); }); }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/secret-value-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/secret-value-list-test.js index cc70c9cc3ee..3923a0fa34d 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/secret-value-list-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/secret-value-list-test.js @@ -9,7 +9,14 @@ module("Integration | Component | secret-value-list", function (hooks) { setupRenderingTest(hooks); test("adding a value", async function (assert) { - await render(hbs``); + this.set("setValidationMessage", () => {}); + + await render(hbs` + + `); this.set("values", "firstKey|FirstValue\nsecondKey|secondValue"); @@ -50,7 +57,17 @@ module("Integration | Component | secret-value-list", function (hooks) { }); test("adding an invalid value", async function (assert) { - await render(hbs``); + let setValidationMessage = (message) => { + this.set("message", message); + }; + this.set("setValidationMessage", setValidationMessage); + + await render(hbs` + + `); await fillIn(".new-value-input.key", "someString"); await fillIn(".new-value-input.secret", "keyWithAPipe|Hidden"); @@ -67,16 +84,22 @@ module("Integration | Component | secret-value-list", function (hooks) { "it doesn't add the value to the list of values" ); - assert.ok( - query(".validation-error").innerText.includes( - I18n.t("admin.site_settings.secret_list.invalid_input") - ), + assert.strictEqual( + this.message, + I18n.t("admin.site_settings.secret_list.invalid_input"), "it shows validation error" ); }); test("changing a value", async function (assert) { - await render(hbs``); + this.set("setValidationMessage", () => {}); + + await render(hbs` + + `); this.set("values", "firstKey|FirstValue\nsecondKey|secondValue"); @@ -109,7 +132,14 @@ module("Integration | Component | secret-value-list", function (hooks) { }); test("removing a value", async function (assert) { - await render(hbs``); + this.set("setValidationMessage", () => {}); + + await render(hbs` + + `); this.set("values", "firstKey|FirstValue\nsecondKey|secondValue"); diff --git a/app/assets/javascripts/discourse/tests/integration/components/site-setting-test.js b/app/assets/javascripts/discourse/tests/integration/components/site-setting-test.js index 48e9286c82a..3b757c733aa 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/site-setting-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/site-setting-test.js @@ -4,6 +4,7 @@ import { module, skip, test } from "qunit"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; import pretender, { response } from "discourse/tests/helpers/create-pretender"; import { query } from "discourse/tests/helpers/qunit-helpers"; +import I18n from "discourse-i18n"; module("Integration | Component | site-setting", function (hooks) { setupRenderingTest(hooks); @@ -118,3 +119,174 @@ module("Integration | Component | site-setting", function (hooks) { .hasNoClass("overridden"); }); }); + +module( + "Integration | Component | site-setting | file_size_restriction type", + function (hooks) { + setupRenderingTest(hooks); + + test("shows the reset button when the value has been changed from the default", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "2048", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + assert.dom(".setting-controls__undo").exists("reset button is shown"); + }); + + test("doesn't show the reset button when the value is the same as the default", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "1024", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + assert + .dom(".setting-controls__undo") + .doesNotExist("reset button is not shown"); + }); + + test("shows validation error when the value exceeds the max limit", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "1024", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + await fillIn(".file-size-input", "5000"); + + assert.dom(".validation-error").hasText( + I18n.t("file_size_input.error.size_too_large", { + provided_file_size: "4.9 GB", + max_file_size: "4 MB", + }), + "validation error message is shown" + ); + assert.dom(".setting-controls__ok").hasAttribute("disabled"); + assert.dom(".setting-controls__cancel").doesNotHaveAttribute("disabled"); + }); + + test("shows validation error when the value is below the min limit", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "1000", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + await fillIn(".file-size-input", "100"); + + assert.dom(".validation-error").hasText( + I18n.t("file_size_input.error.size_too_small", { + provided_file_size: "100 KB", + min_file_size: "512 KB", + }), + "validation error message is shown" + ); + assert.dom(".setting-controls__ok").hasAttribute("disabled"); + assert.dom(".setting-controls__cancel").doesNotHaveAttribute("disabled"); + }); + + test("cancelling pending changes resets the value and removes validation error", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "1000", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + + await fillIn(".file-size-input", "100"); + assert.dom(".validation-error").hasNoClass("hidden"); + + await click(".setting-controls__cancel"); + assert + .dom(".file-size-input") + .hasValue("1000", "the value resets to the saved value"); + assert.dom(".validation-error").hasClass("hidden"); + }); + + test("resetting to the default value changes the content of input field", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "1000", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + assert + .dom(".file-size-input") + .hasValue("1000", "the input field contains the custom value"); + + await click(".setting-controls__undo"); + assert + .dom(".file-size-input") + .hasValue("1024", "the input field now contains the default value"); + + assert + .dom(".setting-controls__undo") + .doesNotExist("the reset button is not shown"); + assert.dom(".setting-controls__ok").exists("the save button is shown"); + assert + .dom(".setting-controls__cancel") + .exists("the cancel button is shown"); + }); + + test("clearing the input field keeps the cancel button and the validation error shown", async function (assert) { + this.set("setting", { + setting: "max_image_size_kb", + value: "1000", + default: "1024", + min: 512, + max: 4096, + type: "file_size_restriction", + }); + + await render(hbs``); + + await fillIn(".file-size-input", "100"); + assert.dom(".validation-error").hasNoClass("hidden"); + + await fillIn(".file-size-input", ""); + assert.dom(".validation-error").hasNoClass("hidden"); + assert.dom(".setting-controls__ok").exists("the save button is shown"); + assert.dom(".setting-controls__ok").hasAttribute("disabled"); + assert + .dom(".setting-controls__cancel") + .exists("the cancel button is shown"); + assert.dom(".setting-controls__cancel").doesNotHaveAttribute("disabled"); + + await click(".setting-controls__cancel"); + assert.dom(".file-size-input").hasValue("1000"); + assert.dom(".validation-error").hasClass("hidden"); + assert + .dom(".setting-controls__ok") + .doesNotExist("the save button is not shown"); + assert + .dom(".setting-controls__cancel") + .doesNotExist("the cancel button is shown"); + }); + } +); diff --git a/app/assets/stylesheets/common/components/file-size-input.scss b/app/assets/stylesheets/common/components/file-size-input.scss index 00cb90587f4..2cbcaa362b9 100644 --- a/app/assets/stylesheets/common/components/file-size-input.scss +++ b/app/assets/stylesheets/common/components/file-size-input.scss @@ -3,6 +3,7 @@ .file-size-input { flex: 1; + margin-bottom: 0; } .file-size-unit-selector { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 082c95a2e33..cccdd1d6824 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2525,6 +2525,7 @@ en: file_size_input: error: size_too_large: "%{provided_file_size} is greater than the max allowed %{max_file_size}" + size_too_small: "%{provided_file_size} is smaller than the min allowed %{min_file_size}" emoji_picker: filter_placeholder: Search for emoji