-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add Spanish citizen id and docs #1147
Conversation
lib/faker/id_number.rb
Outdated
@@ -25,6 +25,17 @@ def ssn_valid | |||
INVALID_SSN.any? { |regex| regex =~ ssn } ? ssn_valid : ssn | |||
end | |||
|
|||
def spanish_citizen_number | |||
checks = "TRWAGMYFPDXBNJZSQVHLCKE" | |||
"#{Faker::Number.number(8)}-#{checks[rand(checks.length)]}" |
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.
If you want the identifier to be valid you have to follow and algorithm. https://www.ordenacionjuego.es/en/calculo-digito-control
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.
I wasn't sure how accurate it needed to be.
The spanish_organisation_number code in the company.rb class is just randomly sampling from the characters and I kinda just followed that example.
Before I try to read auto-translated code again :o ... are the rules for the checksum the same as the ones I outline here: #929 (comment) ?
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.
I'm not sure if there are different ways to do it...the way I know is super simple.
<8 digits number> mod 23 = remainder
REMAINDER | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 |
---|---|---|---|---|---|---|---|---|---|---|---|---|
LETTER | T | R | W | A | G | M | Y | F | P | D | X | B |
REMAINDER | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | 20 | 21 | 22 |
---|---|---|---|---|---|---|---|---|---|---|---|
LETTER | N | J | Z | S | Q | V | H | L | C | K | E |
For instance, mine is: 47922235X, so 47922235 mod 23 = 10
10 = X in the table
Alright, added checksum for DNI... would it be the same for NIE but with the 7 numbers? |
It changes a bit @PuZZleDucK. Initial character mapping: for instance, X1234567 == 01234567 or Y1234567 == 11234567 or Z1234567 == 21234567 |
I dear :o ... I got an error in CI I don't understand... I'm not sure what is different to the last change, other than using |
@PuZZleDucK actually the problem is related with the concatenation: However, I don't see where is the problem exactly. |
Ok, now I know and will never again forget why I was warned (but failed to listen to) the advice to always use interpolation and never concatenation... sorry, recovering java programmer :p |
I think you can merge the request @PuZZleDucK 😄 Thank you! |
lib/faker/id_number.rb
Outdated
end | ||
|
||
def spanish_foreign_citizen_number | ||
checks = "TRWAGMYFPDXBNJZSQVHLCKE" |
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.
I can see that you're repeating this line checks = "TRWAGMYFPDXBNJZSQVHLCKE"
a few times. Could we write a constant instead?
…o spanish-id-and-docs
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.
Looks good to me. Thanks for contributing 🥇
Just left a comment suggesting a minor change. I personally enjoyed the idea. Since I'm Brazilian, I'll try to add the Brazilian Portuguese ID Number methods.
btw I just created an issue with my idea 👏 👏 |
I like the constant idea... I'll get to that when I have a moment. |
@PuZZleDucK I made an effort and pushed a few commits. I basically cleaned the code up with |
* update rake * add id_number docs * add tests for ssn * add failing tests for spanish IDs * add spanish citizen ids for issue faker-ruby#872 * cleanup comments * give spanish_citizen_number correct checksum * add validation checksum to Spanish NIE * convert asserts into asert_equals * replacing string concatenation with interpolation * Minor changes * Run rubocop and fix lint
As requested in #872
Also add docs for the IDNumber class in general