From db993cf8fd6b24b967509bbd4b35014e0b6e3749 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 5 Jul 2024 10:36:41 +1000
Subject: [PATCH] FIX: Do not allow , or . in site setting integer input
 (#27618)

Followup to e113eff6634c9b398b0541ea6a0d1626f6732dcc

We previously sanitized input for integer site settings
on the server side only, which was a bit confusing when
users would enter e.g. 100.5 and end up with 1005, and
not see this reflected in the UI.

Now that we are using native number inputs for these settings,
we can improve the experience a bit by not allowing `.` or `,`
in the input, because it should be whole numbers only, and
add a step size of 1. All other characters are already prevented
in this native number input.
---
 .../components/site-settings/integer.gjs      | 41 +++++++++++++++++++
 .../components/site-settings/integer.hbs      | 11 -----
 .../addon/components/site-settings/integer.js |  3 --
 .../components/site-setting-test.js           | 17 +++++++-
 4 files changed, 56 insertions(+), 16 deletions(-)
 create mode 100644 app/assets/javascripts/admin/addon/components/site-settings/integer.gjs
 delete mode 100644 app/assets/javascripts/admin/addon/components/site-settings/integer.hbs
 delete mode 100644 app/assets/javascripts/admin/addon/components/site-settings/integer.js

diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs b/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs
new file mode 100644
index 00000000000..6869931884e
--- /dev/null
+++ b/app/assets/javascripts/admin/addon/components/site-settings/integer.gjs
@@ -0,0 +1,41 @@
+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
+  updateValue(event) {
+    const num = parseInt(event.target.value, 10);
+
+    if (isNaN(num)) {
+      return;
+    }
+
+    this.args.changeValueCallback(num);
+  }
+
+  @action
+  preventDecimal(event) {
+    if (event.key === "." || event.key === ",") {
+      event.preventDefault();
+    }
+  }
+
+  <template>
+    <input
+      {{on "keydown" this.preventDecimal}}
+      {{on "input" this.updateValue}}
+      type="number"
+      value={{@value}}
+      min={{if @setting.min @setting.min null}}
+      max={{if @setting.max @setting.max null}}
+      class="input-setting-integer"
+      step="1"
+    />
+
+    <SettingValidationMessage @message={{@validationMessage}} />
+    <SiteSettingDescription @description={{@setting.description}} />
+  </template>
+}
diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs b/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs
deleted file mode 100644
index aa81e27991a..00000000000
--- a/app/assets/javascripts/admin/addon/components/site-settings/integer.hbs
+++ /dev/null
@@ -1,11 +0,0 @@
-<input
-  {{on "input" (with-event-value (fn (mut this.value)))}}
-  type="number"
-  value={{this.value}}
-  min={{if this.setting.min this.setting.min null}}
-  max={{if this.setting.max this.setting.max null}}
-  class="input-setting-integer"
-/>
-
-<SettingValidationMessage @message={{this.validationMessage}} />
-<SiteSettings::Description @description={{this.setting.description}} />
\ No newline at end of file
diff --git a/app/assets/javascripts/admin/addon/components/site-settings/integer.js b/app/assets/javascripts/admin/addon/components/site-settings/integer.js
deleted file mode 100644
index 6df18068c19..00000000000
--- a/app/assets/javascripts/admin/addon/components/site-settings/integer.js
+++ /dev/null
@@ -1,3 +0,0 @@
-import Component from "@ember/component";
-
-export default class Integer extends Component {}
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 c3f0cb99cf5..37dc3663f8c 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
@@ -1,6 +1,6 @@
-import { click, fillIn, render } from "@ember/test-helpers";
+import { click, fillIn, render, typeIn } from "@ember/test-helpers";
 import { hbs } from "ember-cli-htmlbars";
-import { module, test } from "qunit";
+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";
@@ -88,4 +88,17 @@ module("Integration | Component | site-setting", function (hooks) {
       "jpg, jpeg, png, gif, heic, heif, webp, avif, svg"
     );
   });
+
+  // Skipping for now because ember-test-helpers doesn't check for defaultPrevented when firing that event chain
+  skip("prevents decimal in integer setting input", async function (assert) {
+    this.set("setting", {
+      setting: "suggested_topics_unread_max_days_old",
+      value: "",
+      type: "integer",
+    });
+
+    await render(hbs`<SiteSetting @setting={{this.setting}} />`);
+    await typeIn(".input-setting-integer", "90,5", { delay: 1000 });
+    assert.dom(".input-setting-integer").hasValue("905");
+  });
 });