Code

Opened 5 years ago

Closed 18 months 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 5 years ago.
Nicaragua local flavor patch.
Ni-localflavor-2.diff (12.8 KB) - added by fitoria 5 years ago.
Second Patch including documentation and test.
Ni-localflavor-3.diff (11.6 KB) - added by fitoria 5 years ago.
patch number 3 for Nicaragua Localflavor

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by fitoria

Nicaragua local flavor patch.

comment:1 Changed 5 years ago by ubernostrum

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to 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).

Changed 5 years ago by fitoria

Second Patch including documentation and test.

comment:2 Changed 5 years ago by fitoria

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 Changed 5 years ago by mtredinnick

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.

Changed 5 years ago by fitoria

patch number 3 for Nicaragua Localflavor

comment:4 Changed 5 years ago by fitoria

  • 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 Changed 3 years ago by SmileyChris

  • Patch needs improvement set
  • Severity set to Normal
  • Summary changed from Nicaragua LocalFlavor ticket to Nicaragua LocalFlavor
  • Type set to New feature

comment:6 Changed 3 years ago by jezdez

  • Easy pickings unset
  • UI/UX unset

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

comment:7 Changed 18 months ago by aaugustin

  • Keywords localflavor, localflavorsplit added; localflavor removed
  • Resolution set to invalid
  • Status changed from new to 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!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.