Added NIF, NIE and CIF Spanish documents numbers validation #830
Conversation
* Added some translations to /localization * Added test suite
|
|
||
| return false; | ||
|
|
||
| }, "Por favor, escribe un CIF válido."); |
There was a problem hiding this comment.
The default message should be in English (even though it is a non-English country specific rule ;)
* Changed the default message to English
|
|
||
| if (/^[ABCDEFGHJNPQRSUVW]{1}/.test(CIF)){ | ||
| CIF = n + ''; | ||
| if (parseInt(num[8],10) === parseInt(String.fromCharCode(64 + n),10) || parseInt(num[8],10) === parseInt(CIF.substring(CIF.length-1, CIF.length),10)){ |
There was a problem hiding this comment.
Since this is evaluating to a return on both branches, it can be replaced with
return (num[8] === String.fromCharCode(64 + n) || num[8] === CIF.substring(CIF.length-1, CIF.length));Removing the parseInts makes sense since you're actually comparing 2 strings converted to ints.
There was a problem hiding this comment.
You are right. Done! Thanks!!
* Fixed bug: Changed to string the strict comparison for num[8]. * Fixed: Some comments translated to english.
* Optimized return in cifES.js
* Optimized returns in nifES.js
* Optimized returns in nieES.js
|
I took a crack at refactoring cifES.js here https://gist.github.com/nschonni/bf337e73e57916d88bf5 Could you also switch the code comments to English? |
* cifES.js refactorized by 'nschonni'
|
Thanks for optimization!! 👍 |
| pos = pos.replace('Y','1'); | ||
| pos = pos.replace('Z','2'); | ||
| pos = pos.substring(0, 8) % 23; | ||
| return (NIE[8] === cadenadni.substring(pos, pos + 1)); |
There was a problem hiding this comment.
cadenadni.substring(pos, pos + 1) can be replace with charAt(pos)
* Removed unused variables. * Refactorized.
* As suggested by 'nschonni', cadenadni.substring(pos, pos + 1) replaced with charAt(pos)
|
|
||
| //XYZ | ||
| if (/^[XYZ]{1}/.test(NIE)){ | ||
| pos = NIE.replace('X','0') |
There was a problem hiding this comment.
Not quite sure what this is doing? Some sort of hashing of the number with corrisponds to the a digit in the eight position? Could you add a comment and then inline and remove the pos variable.
* Translated some variables to English. * Applied jQuery style guide.
| "use strict"; | ||
|
|
||
| var pos = '', | ||
| NIE = value.toUpperCase(), |
There was a problem hiding this comment.
May as well remove the NIE with value = value.toUpperCase() and replace the occurrences below.
|
Sorry for all the nitpicky stuff 👅 @jzaefferer sorry for the noise. I guess we really do need to chat about how this is going to work 😄 |
* 'n' variable changed to 'controlDigit'. * Fixed some code styling: spaces after comas, and spaces before curly braces. * Removed NIF, NIE and CIF variables with value = value.toUpperCase(). * Inlined just one use variables (cadenadni, and some others). * Removed unused variables. * Some substring() replaced with charAt(). * Comments translated to English.
|
Thanks Nick! :) Updated! |
| jQuery.validator.addMethod( "nieES", function ( value, element ) { | ||
| "use strict"; | ||
|
|
||
| var position = value.toUpperCase() |
There was a problem hiding this comment.
This should be inlined to line 25 since it only needs to be calculated on the one if branch.
* Removed 'position' variable: inlined to last test()
* Fixed indentation at nieES.js
|
@jzaefferer is there a specific format for the leading comment block, so it can be parsed to the docs site? |
|
Thanks @alfonsomartinde! squashed it down with a few more comments and landed in 317c20f |
|
Thank you @nschonni 👍 |
|
PS: I looks like the email address that you used for the commits isn't registered with GitHub, so it doesn't link to your profile. You can see https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user#attach-the-email-to-your-github-account for "fixing" it if you care. |
|
OMG... Fixed! Thanks again! |
Sorry, i forgot to create a new branch for this PR :(