FIX: min/max not passed to NumberField for site settings (#24402)

When we started using NumberField for integer site settings
in e113eff663, 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
This commit is contained in:
Martin Brennan 2023-11-16 12:37:05 +10:00 committed by GitHub
parent c87f74cef3
commit 68a912952c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 109 additions and 5 deletions

View File

@ -1,4 +1,9 @@
<NumberField @value={{this.value}} @classNames="input-setting-integer" /> <NumberField
@value={{this.value}}
@classNames="input-setting-integer"
@min={{(if this.setting.min this.setting.min null)}}
@max={{(if this.setting.max this.setting.max null)}}
/>
<SettingValidationMessage @message={{this.validationMessage}} /> <SettingValidationMessage @message={{this.validationMessage}} />
<div class="desc">{{html-safe this.setting.description}}</div> <div class="desc">{{html-safe this.setting.description}}</div>

View File

@ -26,13 +26,27 @@ const ALLOWED_KEYS = [
export default TextField.extend({ export default TextField.extend({
classNameBindings: ["invalid"], classNameBindings: ["invalid"],
keyDown: function (e) { keyDown: function (event) {
return ( return (
ALLOWED_KEYS.includes(e.key) || ALLOWED_KEYS.includes(event.key) ||
(e.key === "-" && parseInt(this.get("min"), 10) < 0) (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") @discourseComputed("number")
value: { value: {
get(number) { get(number) {

View File

@ -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`
<NumberField @value={{this.value}} @classNames="number-field-test" />
`);
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`
<NumberField @value={{this.value}} @classNames="number-field-test" @min="1" />
`);
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`
<NumberField @value={{this.value}} @classNames="number-field-test" @min="-10" />
`);
await fillIn(".number-field-test", "-1");
assert.equal(
this.get("value"),
"-1",
"negative input allowed when min is negative"
);
});
});

View File

@ -180,6 +180,17 @@ class SiteSettings::TypeSupervisor
end end
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[:allow_any] = @allow_any[name] if type == :list
result[:choices] = @choices[name] if @choices.has_key? name result[:choices] = @choices[name] if @choices.has_key? name

View File

@ -418,7 +418,7 @@ RSpec.describe SiteSettings::TypeSupervisor do
before do before do
settings.setting(:type_null, nil) 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_true, true)
settings.setting(:type_float, 2.3232) settings.setting(:type_float, 2.3232)
settings.setting(:type_string, "string") settings.setting(:type_string, "string")
@ -433,24 +433,31 @@ RSpec.describe SiteSettings::TypeSupervisor do
it "returns null type" do it "returns null type" do
expect(settings.type_supervisor.type_hash(:type_null)[:type]).to eq "null" expect(settings.type_supervisor.type_hash(:type_null)[:type]).to eq "null"
end end
it "returns int type" do it "returns int type" do
expect(settings.type_supervisor.type_hash(:type_int)[:type]).to eq "integer" expect(settings.type_supervisor.type_hash(:type_int)[:type]).to eq "integer"
end end
it "returns bool type" do it "returns bool type" do
expect(settings.type_supervisor.type_hash(:type_true)[:type]).to eq "bool" expect(settings.type_supervisor.type_hash(:type_true)[:type]).to eq "bool"
end end
it "returns float type" do it "returns float type" do
expect(settings.type_supervisor.type_hash(:type_float)[:type]).to eq "float" expect(settings.type_supervisor.type_hash(:type_float)[:type]).to eq "float"
end end
it "returns string type" do it "returns string type" do
expect(settings.type_supervisor.type_hash(:type_string)[:type]).to eq "string" expect(settings.type_supervisor.type_hash(:type_string)[:type]).to eq "string"
end end
it "returns url_list type" do it "returns url_list type" do
expect(settings.type_supervisor.type_hash(:type_url_list)[:type]).to eq "url_list" expect(settings.type_supervisor.type_hash(:type_url_list)[:type]).to eq "url_list"
end end
it "returns textarea type" do it "returns textarea type" do
expect(settings.type_supervisor.type_hash(:type_textarea)[:textarea]).to eq true expect(settings.type_supervisor.type_hash(:type_textarea)[:textarea]).to eq true
end end
it "returns enum type" do it "returns enum type" do
expect(settings.type_supervisor.type_hash(:type_enum_choices)[:type]).to eq "enum" expect(settings.type_supervisor.type_hash(:type_enum_choices)[:type]).to eq "enum"
end end
@ -474,5 +481,10 @@ RSpec.describe SiteSettings::TypeSupervisor do
expect(hash[:valid_values]).to eq %w[a b] expect(hash[:valid_values]).to eq %w[a b]
expect(hash[:translate_names]).to eq false expect(hash[:translate_names]).to eq false
end 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
end end