-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: Validate inserted values in numeric global settings #10279
UI: Validate inserted values in numeric global settings #10279
Conversation
@bernardodemarco a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10279 +/- ##
=========================================
Coverage 15.14% 15.14%
- Complexity 11283 11284 +1
=========================================
Files 5408 5408
Lines 473823 473832 +9
Branches 57824 57829 +5
=========================================
+ Hits 71764 71775 +11
+ Misses 394037 394033 -4
- Partials 8022 8024 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
UI build: ✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm and tested in QA, one minor nit: the period is not localised (Dutch uses comma ','. and this is not allowed in decimal fields like overprovisioning.
@DaanHoogland, thanks for testing. Yes, Portuguese also uses comma. However, the backend has a validation that does not accept it, for example: (acs-all-in-one) 🙉 > update configuration name="cpu.overprovisioning.factor" value="2,55"
🙈 Error: (HTTP 431, error code 4350) Value [2,55] is not a valid [class java.lang.Float]. |
yes, no issue with this PR for sure, just a remark/not to self to maybe deal with in the future (or not) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm. Tested and works as expected.
Description
Currently, when changing the
Decimal
,Number
andRange
global settings values through the UI, CloudStack does not validate if the inserted values are numeric. Additionally, when letters are entered in the inputs, a message indicating that the setting value was successfully updated is returned to the operators, which does not effectively happen.Therefore, this PR proposes to verify the values that are entered in those input fields. If non-numeric values are inserted, they will be ignored. Furthermore, it is still possible to use special keys, such as
Ctrl
andAlt
, in order to make use of keyboard shortcuts.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
Decimal
andRange
global settings it is only possible to enter digits and the period (.
) character.Number
global settings it is only possible to enter digits.Decimal
,Number
andRange
global settings it is still possible to make use of keyboard shortcuts with special keys.