Opened 12 years ago

Closed 11 years ago

#17591 closed Bug (invalid)

USSocialSecurityNumberField doesn't validate the same when entered without hypens

Reported by: aaron.l.madison@… Owned by: nobody
Component: contrib.localflavor Version: 1.3
Severity: Normal Keywords: localflavor, ussocialsecuritynumberfield, localflavorsplit
Cc: aaron.l.madison@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

In the us localflavor USSocialSecurityNumberField, if you enter an SSN without hyphens, the 'Woolworth' validations aren't run properly because they're explicity '078-05-1120' and '219-009-9999' instead of checking against the parts.

I refactored the field and created a patch that fixes it, plus added much better tests (in my opinion... added a test for each scenario).

Lastly, sometimes a user might NOT want to force the output to xxx-xx-xxxx. It might be useful to just leave it without hyphens. (in my case, need to pass ssn to a credit report service and will never need the dashes and I don't want to strip them out every time.) So, I added an optional keyword argument 'no_hyphen' to the ssn field which just forces the output to not have hyphens.

Please include or give feedback on the patch.

Thanks!

Attachments (2)

us_ssnfield_fix.patch (6.9 KB ) - added by aaron.l.madison@… 12 years ago.
us ssn field patch
us_ssnfield_fix_v2.patch (66.2 KB ) - added by anonymous 12 years ago.

Download all attachments as: .zip

Change History (6)

by aaron.l.madison@…, 12 years ago

Attachment: us_ssnfield_fix.patch added

us ssn field patch

comment:1 by Łukasz Rekucki, 12 years ago

Needs documentation: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Few notes:

  • Python 2.5 (the minimal version supported right now) doesn't have "with" by default, you need from __future__ import with_statement. It also doesn't have the .format() method, so that won't work,
  • The tests are repetitive: you both added a case to the dictionary and a method. Just add all invalid cases to the dictionary, to keep it simple,
  • You don't have to write "bool(area == '666')": "area == '666'" is exactly the same. Same for any([]) - that just a needlesly complicated "or" expression,
  • The zeros_re - imho, there was nothing wrong with the previous check for zeroes, no reason to change it for a regexp,
  • Get rid of "_is_all_zeros()", "_raise_invalid", etc. methods. It doesn't save typing nor is more DRY, just makes the code longer.

If you want the "no_hypen" flag included, you also need to update the documentation for this field.

comment:2 by anonymous, 12 years ago

Thanks for your feedback. I've attached another patch.

To me the small methods are much less about DRY and much more about readability. A method that clearly says what it does doesn't need a comment to say what it does. Personal preference I'm sure.

by anonymous, 12 years ago

Attachment: us_ssnfield_fix_v2.patch added

comment:3 by aaron.l.madison@…, 12 years ago

sorry for the spam... accidentally posted the patch and my comment as anonymous.

comment:4 by Aymeric Augustin, 11 years ago

Keywords: localflavorsplit added
Resolution: invalid
Status: newclosed

django.contrib.localflavor is now deprecated — see https://docs.djangoproject.com/en/dev/ref/contrib/localflavor/

A repository was created for each localflavor at https://github.com/django/django-localflavor-? (Replace with the country code.)

If you're still interested in this ticket, could you create a pull request on that repository?

Sorry for not resolving this issue earlier, and thanks for your input!

Note: See TracTickets for help on using tickets.
Back to Top