added support for IL in isIdentityCard() #1041
Conversation
|
@perimiter don't worry about the changes that reflected on the src. That should happen and @profnandaa can give more reference on the same here. |
|
thank you. i figured out the test thing after a few commits. i have a technical question though , i used a test that is valid on ES as invalid for me but that failed . i took tests that are invalid for ES and now everything works. why is that? |
profnandaa
added
🧹 needs-update
|
@profnandaa how can i merge the conflicts using an IDE instead of the editor ? files like |
|
@perimiter -- I see, only fix the merge conflicts on README, |
profnandaa
removed
the
🧹 needs-update
README.md
Outdated
| @@ -100,7 +100,7 @@ Validator | Description | |||
| **isHash(str, algorithm)** | check if the string is a hash of type algorithm.<br/><br/>Algorithm is one of `['md4', 'md5', 'sha1', 'sha256', 'sha384', 'sha512', 'ripemd128', 'ripemd160', 'tiger128', 'tiger160', 'tiger192', 'crc32', 'crc32b']` | |||
| **isHexColor(str)** | check if the string is a hexadecimal color. | |||
| **isHexadecimal(str)** | check if the string is a hexadecimal number. | |||
| **isIdentityCard(str [, locale])** | check if the string is a valid identity card code.<br/><br/>`locale` is one of `['ES', 'zh-TW']` OR `'any'`. If 'any' is used, function will check if any of the locals match.<br/><br/>Defaults to 'any'. | |||
| **isIdentityCard(str [, locale])** | check if the string is a valid identity card code.<br/><br/>`locale` is one of `['ES', 'zh-TW', 'IL']` OR `'any'`. If 'any' is used, function will check if any of the locals match.<br/><br/>Defaults to 'any'. | |||
There was a problem hiding this comment.
@perimiter - we should be good now. However, I missed out this one. Please change the locale to he-IL here and in the other files, then we should be good to go.
There was a problem hiding this comment.
As used in isMobilePhone and also see here for ref
|
|
||
| const id = sanitized; | ||
|
|
||
| if (id.length !== 9 || isNaN(id)) { |
There was a problem hiding this comment.
@profnandaa isn't this check unnecessary if we add the size limit in the regex?
There was a problem hiding this comment.
Sure, that's true. That was oversight from my end. You can do a quick update PR on this?
const DNI = /^\d{9}$/;
cc. @perimiter
There was a problem hiding this comment.
thats true, but you still need the if statement for the isNaN()
There was a problem hiding this comment.
@perimiter your regex is searching only for digits, the function will always exit before the call to isNaN if it's not a number.
As you can see, the path is never taken in the tests
i added IL to readme and i used build for babel . i added tests like in ES . all changes i made are in lib not in src(although when i look in the changed files i can see the changes in src as well,is that bables doing? ).
i hope this time i did it correctly .