Skip to content
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

feat(isTaxID): Canadian Social Insurance Number (SIN) validator #1867

Merged
merged 6 commits into from
Feb 17, 2022

Conversation

boonya
Copy link
Contributor

@boonya boonya commented Nov 15, 2021

Here I have implemented separate validator to verify that an input is a valid SIN (Social Insurance Number in Canada).

I realised that you use a combination of ISO 639-1 + ISO 3166-1 alpha-2 to identify locale in the isTaxID validator, so I have applied appropriate two locales for the Canada - en-CA and fr-CA.

Also I have added a few additional values of postal codes in Ukraine, Poland and Canada to improve autotesting isPostalCode validator.

Checklist

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

@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #1867 (f8892f1) into master (47ee5ad) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1867   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          102       102           
  Lines         2059      2084   +25     
  Branches       464       472    +8     
=========================================
+ Hits          2059      2084   +25     
Impacted Files Coverage Δ
src/lib/isTaxID.js 100.00% <100.00%> (ø)
src/lib/isMACAddress.js 100.00% <0.00%> (ø)
src/lib/isMobilePhone.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47ee5ad...f8892f1. Read the comment docs.

@boonya boonya marked this pull request as ready for review November 15, 2021 18:56
@boonya boonya changed the title Canada social insurance number feat(isSIN): Canada Social Insurance Number validator Nov 15, 2021
@tux-tn
Copy link
Member

tux-tn commented Nov 15, 2021

Thank you for your PR. Isn't SIN a Tax ID for individuals and thereby should be part of isTaxID?

@boonya
Copy link
Contributor Author

boonya commented Nov 15, 2021

Thank you for your PR. Isn't SIN a Tax ID for individuals and thereby should be part of isTaxID?

That is a good question. Actually I don't know. Let me ask my Canadian friends and google to clarify that.

@boonya
Copy link
Contributor Author

boonya commented Nov 17, 2021

Thank you for your PR. Isn't SIN a Tax ID for individuals and thereby should be part of isTaxID?

Okay, I have got an answer below

there is no such Tax ID term. If you have SIN that is enough. Your Tax should be automatically connected to your SIN. In general sense, i think "tax id = sin"

So, I have redone my work. I hope it's good to go now.

@boonya boonya changed the title feat(isSIN): Canada Social Insurance Number validator feat(isTaxID): Canadian Social Insurance Number (SIN) validator Nov 17, 2021
Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

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

Mostly LGTM 🎉
Thank you for your PR! Can you just address my comment below?

README.md Show resolved Hide resolved
@tux-tn tux-tn added 🎉 first-pr 🧹 needs-update For PRs that need to be updated before landing labels Nov 17, 2021
@boonya boonya requested a review from tux-tn November 17, 2021 20:58
Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

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

LGTM ! Thank you for your contribution

@tux-tn tux-tn added ready-to-land For PRs that are reviewed and ready to be landed and removed 🧹 needs-update For PRs that need to be updated before landing labels Nov 17, 2021
Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for your contribution! 🎉

@profnandaa profnandaa merged commit 4ee1655 into validatorjs:master Feb 17, 2022
@boonya boonya deleted the canada-social-insurance-number branch February 17, 2022 09:07
peterblazejewicz added a commit to peterblazejewicz/DefinitelyTyped that referenced this pull request Mar 23, 2022
peterblazejewicz added a commit to peterblazejewicz/DefinitelyTyped that referenced this pull request Mar 23, 2022
typescript-bot pushed a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 first-pr ready-to-land For PRs that are reviewed and ready to be landed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants