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)
Change History (10)
by , 16 years ago
Attachment: | NI-localflavor.diff added |
---|
comment:1 by , 16 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
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 , 16 years ago
Attachment: | Ni-localflavor-2.diff added |
---|
Second Patch including documentation and test.
comment:2 by , 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 , 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:
- 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.
- 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. - 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, sinced
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. - 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.
- 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 aForm
class, initialise the form with errors and then test what the output looks like when the form is rendered.
comment:4 by , 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 , 14 years ago
Patch needs improvement: | set |
---|---|
Severity: | → Normal |
Summary: | Nicaragua LocalFlavor ticket → Nicaragua LocalFlavor |
Type: | → New feature |
comment:6 by , 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 , 12 years ago
Keywords: | localflavorsplit added |
---|---|
Resolution: | → invalid |
Status: | new → closed |
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!
Nicaragua local flavor patch.