Opened 17 years ago

Closed 17 years ago

#3946 closed (fixed)

Add Swiss localflavor

Reported by: charly.wilhelm@… Owned by: Adrian Holovaty
Component: contrib.localflavor Version: dev
Severity: Keywords: localflavor, l10n, ch
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Patch to add a module to django.contrib.localflavor for Switzerland: CHIdentityCardNumberField, CHPhoneNumberField, CHStateSelect and CHZipCodeField.

Attachments (2)

ch-localflavor.diff (5.2 KB ) - added by charly.wilhelm@… 17 years ago.
initial ch localflavor
ch-localflavor2.diff (8.7 KB ) - added by charly.wilhelm@… 17 years ago.
Replacement with the suggestions of mtredinnick

Download all attachments as: .zip

Change History (10)

by charly.wilhelm@…, 17 years ago

Attachment: ch-localflavor.diff added

initial ch localflavor

comment:1 by Simon G. <dev@…>, 17 years ago

Has patch: set
Keywords: ch added
Needs tests: set
Triage Stage: UnreviewedReady for checkin

Looks good - but a few tests would be nice, if possible?

comment:2 by Adrian Holovaty, 17 years ago

Component: Contrib appsdjango.contrib.localflavor

comment:3 by Malcolm Tredinnick, 17 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Agree with Simon; some tests would be good just to exercise the code. They often catch bugs when/if we refactor anything. The page you used for the reference implementation of ID numbers has some examples at the end.

A few little improvements for the patch itself (all in forms.py)

  1. line 36 is a print statement. Make it go away!
  2. The validation error messages should be marked as translatable, since they are presented to the user (the person using the web browser). Since our translation functions cannot, at the moment, accept unicode strings, you'll have to remove the initial 'u' from those strings, but it doesn't look like that will hurt anything, since they appear to be ASCII strings currently anyway.
  3. line 67 turns "first" into a string, so why is str(first) needed on line 69? Also, line 69 can just be num = ord(first.upper()) - 65, which seems shorter.
  4. If you make the computation of num simpler on line 69, you can avoid assigning it to a variable and use it directly in line 70. Doesn't look like you're currently using num anywhere at the moment.
  5. It seems that if I pass in something that is not a digit in "number", the code is going to fail with an uncaught exception. You might want to be catching ValueError and failing the validation if that is raised. Again, tests will help exercise this (try an ID number of "banana", for example).
  6. line 91: you can't pass unicode strings to gettext() -- make-messages.py will complain. Not sure how to fix that yet (ticket #3923), so you have to work around it by not using unicode strings there.
  7. Best to say "Swiss-specific" in the docstring. Not everybody outside of Switzerland, particularly outside of Europe, associates CH with Switzerland.

That's all I spotted for the moment (it lengthy, but they're all little things).

comment:4 by philipp.keller@…, 17 years ago

Cool, there are other swiss guys using django!
I got a suggestion for the phone number field: I often write phone numbers "0xx/xxxxxxx" but the slash isn't supported by your parser.
I'd suggest the phonenumber parser to look for a pattern like

0([^0-9]*[0-9]){9}

This would allow also numbers 0xx-000-00-00.

by charly.wilhelm@…, 17 years ago

Attachment: ch-localflavor2.diff added

Replacement with the suggestions of mtredinnick

comment:5 by charly.wilhelm@…, 17 years ago

I changed the suggestions of mtredinnick and added some tests.

To Philipp Keller: Django is the best Framework I've ever seen. easy, fast and funny. To your sugguestion. Therefore I remove some non numeric characters (line 32) and parse the value with phone_digits (line 12), which I think must start with a 0, then a non 0, followed by 8 numbers.

This way the parser accepts also a number as 012/34-567.89 and cleans it to 012 345 67 89.

comment:6 by Simon G. <dev@…>, 17 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:7 by Malcolm Tredinnick, 17 years ago

In future, please make sure there are no subversion conflicts in your tree before generating the patch. You can see in the localflavor.py part of the patch, that there was a subversion conflict (the "<<<<", "====" and ">>>>" markers) that had to be manually fixed before I could commit this.

comment:8 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: newclosed

(In [5132]) Fixed #3946 -- Added Swiss localflavor. Thanks, charly.wilhelm@….

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