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

Checking Valid AadharCard Number and Some Country Code and Name Validation #1710

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pnirmale
Copy link

Validator added:
1)isAadharCardNumber=basically Aadhar card number is unique identitiy number of Indian citizen and it require in various form filing along with documents .It is 12 or 16 digit number so i check for both using regex

2)isCountryByCode= Each country has it own Country Code Like US ,IN ,AF so we check give Country Code and find it valid or not along with is return country name if is is invalid it return invalid country code

3)isValidCountryName=if we are only using country name the we need to valid country name also so I list down all the country an validating country from all country list currently available

No Readme Updated

All the test case are return in Client-side.js and passed
Screenshot 2021-08-20 at 10 49 51 AM
Screenshot 2021-08-20 at 10 50 43 AM

@codecov
Copy link

codecov bot commented Aug 20, 2021

Codecov Report

Merging #1710 (af0c5cb) into master (8c4b3b3) will decrease coverage by 0.19%.
The diff coverage is 63.63%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master    #1710      +/-   ##
===========================================
- Coverage   100.00%   99.80%   -0.20%     
===========================================
  Files          101      101              
  Lines         2005     2016      +11     
  Branches       452      455       +3     
===========================================
+ Hits          2005     2012       +7     
- Misses           0        2       +2     
- Partials         0        2       +2     
Impacted Files Coverage Δ
src/index.js 100.00% <ø> (ø)
src/lib/isIdentityCard.js 97.51% <63.63%> (-2.49%) ⬇️

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 8c4b3b3...af0c5cb. Read the comment docs.

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.

Thank you for your contribution @pnirmale

Some comments regarding your PR:

  • Since Aadhar is an IdentityNumber, i suggest you add the Aadhar validation to isIdentityCard
  • Your isCountryByCode is not a validator, the purpose of this library is to provide a list of string validators and sanitizers
  • client-side tests are meant to test if the minified version of validator is working correctly, you should probably check test examples from test/validators and add valid/invalid cases
  • Some lines are not covered, please check coverage report
  • Add an entry in README.md for the added validators

Also a suggestion, you should probably open a PR about a single new validator at a time to prevent long review and approval time. Personally i don't agree with adding a isValidCountryName validator

@@ -6,6 +6,18 @@ feat(validatorName): brief title of what has been done
<!--- briefly describe what you have done in this PR --->

## Checklist
Validator added:
Copy link
Member

Choose a reason for hiding this comment

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

Can you discard your changes to this file?
I suppose you made a copy/paste error

Copy link

Choose a reason for hiding this comment

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

Thanks for your help. I'm not sure what it is exactly you'd like me to delete, could you please point i

Copy link

Choose a reason for hiding this comment

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

Point it out, obviously I have no clue what I'm doing or how I wound up here. Sorry about that

Copy link
Author

Choose a reason for hiding this comment

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

Yes Surely I do required changes

Copy link
Author

Choose a reason for hiding this comment

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

What is your opinion about isCountryByCode i could change to isvalidCountryCode ?

My opinion about isValidCountryName= it will beneficial when we are not providing those search country option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants