-
Notifications
You must be signed in to change notification settings - Fork 809
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
Fix issue on IsNumberString ignores maxDecimalPlaces option #508 #518
Fix issue on IsNumberString ignores maxDecimalPlaces option #508 #518
Conversation
@@ -3281,19 +3294,19 @@ describe("isHash", function() { | |||
it("should not fail if validator.validate said that its valid", function(done) { | |||
checkValidValues(new MyClass(), validValues, done); | |||
}); | |||
|
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.
please remove this "empty" changes
isNumberString(value: unknown, options?: ValidatorJS.IsNumericOptions): boolean { | ||
return typeof value === "string" && this.validatorJs.isNumeric(value, options); | ||
isNumberString(value: unknown, options?: IsNumberOptions): boolean { | ||
return typeof value === "string" && this.validatorJs.isNumeric(value) && (options == null || this.isNumber(parseFloat(value), options)); |
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.
It is necessary && (options == null || this.isNumber(parseFloat(value), options)
part? I think isNumber
accept also undefined as option argument.
@@ -530,8 +530,8 @@ export class Validator { | |||
* Checks if the string is numeric. | |||
* If given value is not a string, then it returns false. | |||
*/ | |||
isNumberString(value: unknown, options?: ValidatorJS.IsNumericOptions): boolean { | |||
return typeof value === "string" && this.validatorJs.isNumeric(value, options); | |||
isNumberString(value: unknown, options?: IsNumberOptions): boolean { |
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.
We use ValidatorJS.*
prefix everywhere. Please do not change it her.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Now
IsNumberString
acceptsIsNumberOptions
options but is NOT passing and validating against the number validator.Fix
Added the support for a number validation with
IsNumberString
. The value will be also validated against providedIsNumberOptions
as a float.