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: 🦺 add support for Dominican Republic passport numbers #2366

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

Conversation

Victor1890
Copy link

@Victor1890 Victor1890 commented Jan 29, 2024

Hey! Would it be possible to support Dominican Republic Passport Valid Number type from validation? Thanks

Checklist

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

Copy link

codecov bot commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (b958bd7) to head (b23d237).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2366      +/-   ##
==========================================
- Coverage   99.95%   99.95%   -0.01%     
==========================================
  Files         107      107              
  Lines        2454     2449       -5     
  Branches      619      619              
==========================================
- Hits         2453     2448       -5     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

profnandaa
profnandaa previously approved these changes Mar 5, 2024
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, except for the minor issue.

README.md Outdated Show resolved Hide resolved
Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

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

Could you provide a source for the regular expression? The examples from the Council of the European Union seems to show passport numbers starting with SC; https://www.consilium.europa.eu/prado/en/DOM-AO-02001/index.html and https://www.consilium.europa.eu/prado/en/DOM-AO-01001/index.html

I also found this forum post mentioning that there is more than just codes starting with RD; https://dr1.com/forums/threads/dominican-passport-numbers.396022/

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.

Meant before DZ but it's all good, I'll fix that in release prep. Let me check why Codecov is failing.

@WikiRik
Copy link
Member

WikiRik commented Mar 5, 2024

@profnandaa Codecov is fixed in #2341

@Victor1890
Copy link
Author

@WikiRik I was use ChatGPT for valid the info and looking for the passport to few people, I have to come the conclusion to passport number have 2 letter (RD) and it have 7 digital number

This is the link: https://chat.openai.com/share/8f1b1bfd-1cdf-4d96-bda0-0ed27f7bf276

@WikiRik
Copy link
Member

WikiRik commented Mar 11, 2024

ChatGPT is not a valid source for this repository.

@Victor1890
Copy link
Author

Victor1890 commented Apr 30, 2024

Hi @WikiRik, thank you for your waiting.

The element that refers to the unique numbering on the passport document is the Document Number (Número de documento) which uniquely identifies a document and is recommended to be identical to the Control Number (Número de control) assigned at the time of manufacture for registration and security purposes.

You can check the information in this link:
https://www.icao.int/publications/Documents/9303_p1_cons_es.pdf

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.

None yet

3 participants