From 68a912952c842226a54ad65aae8e3446c0dc4424 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 16 Nov 2023 12:37:05 +1000 Subject: [PATCH] FIX: min/max not passed to NumberField for site settings (#24402) When we started using NumberField for integer site settings in e113eff6634c9b398b0541ea6a0d1626f6732dcc, we did not end up passing down a min/max value for the integer to the field, which meant that for some fields where negative numbers were allowed we were not accepting that as valid input. This commit passes down the min/max options from the server for integer settings then in turn passes them down to NumberField. c.f. https://meta.discourse.org/t/delete-user-self-max-post-count-not-accepting-1-to-disable/285162 --- .../components/site-settings/integer.hbs | 7 ++- .../discourse/app/components/number-field.js | 20 +++++- .../components/number-field-test.js | 62 +++++++++++++++++++ lib/site_settings/type_supervisor.rb | 11 ++++ .../lib/site_settings/type_supervisor_spec.rb | 14 ++++- 5 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/integration/components/number-field-test.js diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs b/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs index a3a297a4976..ba2b0c92260 100644 --- a/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs +++ b/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs @@ -1,4 +1,9 @@ - +
{{html-safe this.setting.description}}
\ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/number-field.js b/app/assets/javascripts/discourse/app/components/number-field.js index c3ae066df64..38838358ef8 100644 --- a/app/assets/javascripts/discourse/app/components/number-field.js +++ b/app/assets/javascripts/discourse/app/components/number-field.js @@ -26,13 +26,27 @@ const ALLOWED_KEYS = [ export default TextField.extend({ classNameBindings: ["invalid"], - keyDown: function (e) { + keyDown: function (event) { return ( - ALLOWED_KEYS.includes(e.key) || - (e.key === "-" && parseInt(this.get("min"), 10) < 0) + ALLOWED_KEYS.includes(event.key) || + (event.key === "-" && this._minNumber && this._minNumber < 0) ); }, + get _minNumber() { + if (!this.get("min")) { + return; + } + return parseInt(this.get("min"), 10); + }, + + get _maxNumber() { + if (!this.get("max")) { + return; + } + return parseInt(this.get("max"), 10); + }, + @discourseComputed("number") value: { get(number) { diff --git a/app/assets/javascripts/discourse/tests/integration/components/number-field-test.js b/app/assets/javascripts/discourse/tests/integration/components/number-field-test.js new file mode 100644 index 00000000000..9c12e71fc44 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/number-field-test.js @@ -0,0 +1,62 @@ +import { fillIn, render, triggerKeyEvent } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; +import { module, test } from "qunit"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; + +module("Integration | Component | number-field", function (hooks) { + setupRenderingTest(hooks); + + test("number field", async function (assert) { + this.set("value", 123); + + await render(hbs` + + `); + + await fillIn(".number-field-test", "33"); + + assert.equal( + this.get("value"), + 33, + "value is changed when the input is a valid number" + ); + + await fillIn(".number-field-test", ""); + await triggerKeyEvent(".number-field-test", "keydown", 66); // b + + assert.equal( + this.get("value"), + "", + "value is cleared when the input is NaN" + ); + }); + + test("number field | min value", async function (assert) { + this.set("value", ""); + + await render(hbs` + + `); + + await triggerKeyEvent(".number-field-test", "keydown", 189); // - + await triggerKeyEvent(".number-field-test", "keydown", 49); // 1 + + assert.equal( + this.get("value"), + "", + "value is cleared when the input is less than the min" + ); + + await render(hbs` + + `); + + await fillIn(".number-field-test", "-1"); + + assert.equal( + this.get("value"), + "-1", + "negative input allowed when min is negative" + ); + }); +}); diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index db4bfa054a1..c06bd3bfb52 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -180,6 +180,17 @@ class SiteSettings::TypeSupervisor end end + if type == :integer + result[:min] = @validators[name].dig(:opts, :min) if @validators[name].dig( + :opts, + :min, + ).present? + result[:max] = @validators[name].dig(:opts, :max) if @validators[name].dig( + :opts, + :max, + ).present? + end + result[:allow_any] = @allow_any[name] if type == :list result[:choices] = @choices[name] if @choices.has_key? name diff --git a/spec/lib/site_settings/type_supervisor_spec.rb b/spec/lib/site_settings/type_supervisor_spec.rb index b1d416c18d6..c12be5dc9c2 100644 --- a/spec/lib/site_settings/type_supervisor_spec.rb +++ b/spec/lib/site_settings/type_supervisor_spec.rb @@ -418,7 +418,7 @@ RSpec.describe SiteSettings::TypeSupervisor do before do settings.setting(:type_null, nil) - settings.setting(:type_int, 1) + settings.setting(:type_int, 1, min: -10, max: 10) settings.setting(:type_true, true) settings.setting(:type_float, 2.3232) settings.setting(:type_string, "string") @@ -433,24 +433,31 @@ RSpec.describe SiteSettings::TypeSupervisor do it "returns null type" do expect(settings.type_supervisor.type_hash(:type_null)[:type]).to eq "null" end + it "returns int type" do expect(settings.type_supervisor.type_hash(:type_int)[:type]).to eq "integer" end + it "returns bool type" do expect(settings.type_supervisor.type_hash(:type_true)[:type]).to eq "bool" end + it "returns float type" do expect(settings.type_supervisor.type_hash(:type_float)[:type]).to eq "float" end + it "returns string type" do expect(settings.type_supervisor.type_hash(:type_string)[:type]).to eq "string" end + it "returns url_list type" do expect(settings.type_supervisor.type_hash(:type_url_list)[:type]).to eq "url_list" end + it "returns textarea type" do expect(settings.type_supervisor.type_hash(:type_textarea)[:textarea]).to eq true end + it "returns enum type" do expect(settings.type_supervisor.type_hash(:type_enum_choices)[:type]).to eq "enum" end @@ -474,5 +481,10 @@ RSpec.describe SiteSettings::TypeSupervisor do expect(hash[:valid_values]).to eq %w[a b] expect(hash[:translate_names]).to eq false end + + it "returns int min/max values" do + expect(settings.type_supervisor.type_hash(:type_int)[:min]).to eq(-10) + expect(settings.type_supervisor.type_hash(:type_int)[:max]).to eq(10) + end end end