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

Validate leap year dates correctly #57

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

dannyhajj
Copy link
Member

Type of change

  • New feature
  • Bug fix
  • Security patch
  • Documentation update

Description

Date validation is resulting in false negatives and positives for some people born on Feb-29 on a leap year. For example, 200002292399 is incorrectly failing validation:

>>> personnummer.Personnummer('200002292399').valid()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/personnummer/personnummer.py", line 29, in __init__
    raise PersonnummerException(
personnummer.personnummer.PersonnummerException: 200002292399 Not a valid Swedish personal identity number!

Related issue
Replaces #46

Motivation

Allow people born on leap years to use services relying on this package 🙈

Checklist

  • I have read the CONTRIBUTING document.
  • I have read and accepted the Code of conduct
  • Tests passes.
  • Style lints passes.
  • Documentation of new public methods exists.
  • New tests added which covers the added code.

PS. I would like to add the 200002292399 SSN to the list of test data here: https://github.com/personnummer/meta/blob/master/testdata/list.json

However, when running the generateTestdata.php script it's generating different SSNs and I'm not sure how is your contribution flow for those test. I could use some help adding it. 😅

@frozzare
Copy link
Member

frozzare commented Nov 10, 2023

Thanks, will check it out later today.

Regarding the generateTestdata.php we add special cases here with a comment to the issue, if you need help with it I can do it later today as well, but it should be easy even if it's php :)

@frozzare
Copy link
Member

I've added a special case for this, so you should be able to test it correctly now personnummer/meta#57

@Johannestegner
Copy link
Member

LGTM!

Thank you for the contribution :)

@Johannestegner Johannestegner merged commit 8344a05 into personnummer:master Nov 14, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants