feat(isIdentityCard): Add PL locale by wiktorwojcik112 · Pull Request #1745

feat(isIdentityCard): Add PL locale by wiktorwojcik112 · Pull Request #1745 · validatorjs/validator.js

Add isPESEL validation.

PESEL is a polish national identification number. Validator checks whether provided string follows criteria of
valid PESEL.

Checklist

PR contains only changes related; no stray files, etc.
README updated (where applicable)
Tests written (where applicable)

fedeci left a comment •

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it have anything to do with isIdentityCard? I am quite in disagreement with adding a specific validator for a specific country code.

Does it have anything to do with isIdentityCard? I am quite in disagreement with adding a specific validator for a specific country code.

I checked and it would fit there. I will add this PESEL validator to isIdentityCard.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR @wiktorwojcik112
Totally agree with @fedeci this validator should be added as a locale to isIdentityCard. Please make the necessary changes and add tests for the uncovered line in your code

Co-authored-by: Federico Ciardi <fed.ciardi@gmail.com>

tux-tn left a comment •

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for updating your PR @wiktorwojcik112 can you please fix the linter issue and add the missing test cases to bring back coverage to 100%.
I suggest you run tests locally before pushing new code (using yarn test or npm run test)
EDIT: Can you please change the PR title to specify you are adding a new locale to isIdentityCard and not a new validator (The PR title will be the commit message when the PR is merged and the actual title is misleading)

wiktorwojcik112 changed the title ~~feat(isPESEL): Add validator for PESEL~~ feat(isIdentityCard): Add PL locale

Oct 2, 2021

Ok. I fixed linter error and changed title, but I still can't figure out how to solve coverage issue.

Ok. I fixed linter error and changed title, but I still can't figure out how to solve coverage issue.

That's tricky but you need to find a test example where the last digit of PESEL number equals 10 minus your modulo.
As i told you earlier the uncovered execution path is lastDigit === 10 - modulo

Ok. I fixed linter error and changed title, but I still can't figure out how to solve coverage issue.

That's tricky but you need to find a test example where the last digit of PESEL number equals 10 minus your modulo. As i told you earlier the uncovered execution path is lastDigit === 10 - modulo

Everything should be good now.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @wiktorwojcik112 can you please address my latest comment concerning istanbul ignore else ?

Co-authored-by: Sarhan Aissi <tux-tn@users.noreply.github.com>

Checklist

fedeci left a comment • Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tux-tn left a comment • Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedeci left a comment •

Loading

tux-tn left a comment •

Loading