-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Implement isPassportNumber Validator #1250
Implement isPassportNumber Validator #1250
Conversation
@profnandaa Have a look at it when you're free to 👍 |
@dani2819 Abdullah -- FYI ☝️ |
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.
Solid work on this Hamza, thanks! Few comments:
- add a few test cases
- remove the stray code indicated
validator.js
Outdated
@@ -402,7 +402,7 @@ function isFQDN(str, options) { | |||
to the 5th link, and "interface10" belongs to the 10th organization. | |||
* * */ | |||
|
|||
var ipv4Maybe = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/; | |||
var ipv4Maybe = /^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$/; |
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.
can remove this unrelated change.
validator.js
Outdated
@@ -848,6 +848,17 @@ function isBoolean(str) { | |||
return ['true', 'false', '1', '0'].indexOf(str) >= 0; | |||
} | |||
|
|||
var localeReg = /^[A-z]{2,4}([_-]([A-z]{4}|[\d]{3}))?([_-]([A-z]{2}|[\d]{3}))?$/; |
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.
this too.
@@ -2420,6 +2524,7 @@ var validator = { | |||
isJSON: isJSON, | |||
isEmpty: isEmpty, | |||
isLength: isLength, | |||
isLocale: isLocale, |
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.
@profnandaa Should this be removed as well ?
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.
Yes, please -- all the unrelated diff.
@@ -35,12 +35,16 @@ var _isFQDN = _interopRequireDefault(require("./lib/isFQDN")); | |||
|
|||
var _isBoolean = _interopRequireDefault(require("./lib/isBoolean")); | |||
|
|||
var _isLocale = _interopRequireDefault(require("./lib/isLocale")); |
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.
@profnandaa Remove this ?
@@ -227,6 +232,7 @@ var validator = { | |||
isJSON: _isJSON.default, | |||
isEmpty: _isEmpty.default, | |||
isLength: _isLength.default, | |||
isLocale: _isLocale.default, |
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.
@profnandaa Should be removed ?
@@ -1853,6 +1853,522 @@ describe('Validators', () => { | |||
}); | |||
}); | |||
|
|||
it('should validate passport number', () => { |
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.
@profnandaa Check here for the tests, I've already included test-cases, one per country code. It's a Large diff, it had be manually loaded, probably that's why you didn't notice it
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.
My bad missed that.
Great job @hamzahejja 🎉 |
@@ -1853,6 +1853,522 @@ describe('Validators', () => { | |||
}); | |||
}); | |||
|
|||
it('should validate passport number', () => { |
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.
My bad missed that.
@@ -140,6 +140,7 @@ Validator | Description | |||
**isMultibyte(str)** | check if the string contains one or more multibyte chars. | |||
**isNumeric(str [, options])** | check if the string contains only numbers.<br/><br/>`options` is an object which defaults to `{no_symbols: false}`. If `no_symbols` is true, the validator will reject numeric strings that feature a symbol (e.g. `+`, `-`, or `.`). | |||
**isOctal(str)** | check if the string is a valid octal number. | |||
**isPassportNumber(str, countryCode) | check if the string is a valid passport number relative to a specific country code. |
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.
@profnandaa Could you just update README.md
at master branch and change/fix this line to the correct styling, i.e.
**isPassportNumber(str, countryCode)**
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.
Oops, will do that.
isPassportNumber(str, countryCode)
validator, which validates the input string against a passport no. regex that corresponds to the passed-in ISO country code 🎉References:
ISO Country Code(s) Worldwide
EU Passport Number(s) Format/Pattern.
Wikipedia
Tests are added per country, testing valid/invalid string(s) for validity.