Opened 16 years ago

Closed 12 years ago

#10203 closed New feature (invalid)

Nicaragua LocalFlavor

Reported by: fitoria Owned by: nobody
Component: contrib.localflavor Version: 1.0
Severity: Normal Keywords: Nicaragua, localflavor, localflavorsplit
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I made the local flavor for nicaragua including Departament Select, Cedula number and phone number input. Check the attachment for the svn diff. Any correction just let me know.

Attachments (3)

NI-localflavor.diff (3.5 KB ) - added by fitoria 16 years ago.
Nicaragua local flavor patch.
Ni-localflavor-2.diff (12.8 KB ) - added by fitoria 16 years ago.
Second Patch including documentation and test.
Ni-localflavor-3.diff (11.6 KB ) - added by fitoria 16 years ago.
patch number 3 for Nicaragua Localflavor

Download all attachments as: .zip

Change History (10)

by fitoria, 16 years ago

Attachment: NI-localflavor.diff added

Nicaragua local flavor patch.

comment:1 by James Bennett, 16 years ago

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

This needs several things:

  • Documentation explaining what it provides and how to use it (integrated into the existing localflavor docs; see them for examples).
  • Tests verifying that the code works properly.
  • Probably some model fields corresponding to the form fields would be nice to have (several other localflavor modules provide this).

by fitoria, 16 years ago

Attachment: Ni-localflavor-2.diff added

Second Patch including documentation and test.

comment:2 by fitoria, 16 years ago

I added the documentation and test into this new patch check it out. Nicaragua will add an extra digit to the phone numbers on April 1st 2009 I will submit a new patch on that date is this the correct procedure?.

comment:3 by Malcolm Tredinnick, 16 years ago

A few things that need tidying up here:

Firstly, it's not really going to be harmful to allow "123 4567" (i.e. not hyphen) as a phone number. Even "1-23-456-7" is understandable. The idea is to accept anythign reasonable and then normalise it (in the clean()) method to something consistent. Have a look at how other local flavours handle their phone number fields. (aside: we really need to come up with a better way to do this, since having everybody provide a class for this is getting ridiculous). Probably removing all hyphens and spaces and then checking the result is all digits of the required length is the easiest approach.

The tests need some more work. You've got the right idea, but right now, they aren't particularly useful:

  1. Look at how other tests handle exceptions. The full traceback isn't useful, since it includes path names and stuff like that which will be different on everybody else's systems. Notice how other tests only include the first and the last line of the traceback.
  2. It's very exceptional (and this isn't an exceptional case) that a test would need to call an underscore method (e.g. _get_choices()). The test should look like how a user is going to use these functions. So we typically test things like the output of form rendering and the like.
  3. Something like <django.utils.functional.__proxy__ object at 0x7f4537003410> in test output has the same problem as tracebacks. It won't be the same every time and certainly not on different machines. Also, the fact that it's a proxy object is entirely unimportant. What is important is what the output looks like when that object is rendered as a unicode string. In that particular case, since d is a form field, put it into a form class and render the form (e.g. form.as_p()). Again, have a look at what other tests in the same area do.
  4. Line 113 of the tests part of the patch includes some non-English output. Generally, unless we are explicitly testing the translation infrastructure (which we aren't doing here), we only use English output so that it's easy to understand and isn't relying on any particular PO file containing a certain translation.
  5. Similar to point 3, and looking at line 113 again, instead of calling d.clean() directly, rewrite the test so that it looks like how the field will be used in real code. Make it part of a Form class, initialise the form with errors and then test what the output looks like when the form is rendered.

by fitoria, 16 years ago

Attachment: Ni-localflavor-3.diff added

patch number 3 for Nicaragua Localflavor

comment:4 by fitoria, 16 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

@mtredinnick: I have followed your suggestions and now the phone number field can accept format with or without hyphen, and blankspaces also.

DepartamentField have been updated to use official numbers matching the departament name, this data is used in the government institutions.

comment:5 by Chris Beaven, 14 years ago

Patch needs improvement: set
Severity: Normal
Summary: Nicaragua LocalFlavor ticketNicaragua LocalFlavor
Type: New feature

comment:6 by Jannis Leidel, 13 years ago

Easy pickings: unset
UI/UX: unset

Please update the patch to follow Django's new unittest-only policy for localflavors.

comment:7 by Aymeric Augustin, 12 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/).

If you'd like, I encourage you to create a django-localflavor- (where is your country code), following the template of all the other django-localflavor- packages. See http://github.com/django for the list.

Once that's done, we can link to it from the documentation.

Thanks for your understanding!

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