#13546 closed New feature (fixed)
Easier handling of localize field options in ModelForm
| Reported by: | Henrik Karlsen | Owned by: | Sasha Romijn |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Normal | Keywords: | sprint2013 dceu13 |
| Cc: | Greg Wogan-Browne, mgventura, Simon Charette | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
When USE_L10N is set to True in settings, you still has to customize every single form Field to use it in forms. While this is fairly easy with the localize argument to django.forms.fields.Field, it is not easy if you use a ModelForm. In that case, you have to do something similar to this:
class CustomForm(ModelForm):
class Meta:
model = SomeModel
def __init__(self, *args, **kwargs):
super(CustomForm, self).__init__(*args, **kwargs)
self.fields['some_field'].localize = True
Where some_field in SomeModel for example could be a DecimalField.
I therefore suggest that the USE_L10N setting is used as default value for form Field, as done in the attached patch.
Attachments (1)
Change History (20)
by , 15 years ago
| Attachment: | localized_form_fields.diff added |
|---|
comment:1 by , 15 years ago
| Has patch: | unset |
|---|---|
| Summary: | Feature request: USE_L10N as default value for form Field → Easier handling of localize field options in ModelForm |
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 15 years ago
I like the solution from russellm seems pretty clean, and there is many cases for a developer would need this.
When it will be possible include this solution at trunk?
comment:3 by , 15 years ago
| Cc: | added |
|---|
comment:4 by , 15 years ago
| Severity: | → Normal |
|---|---|
| Type: | → New feature |
comment:5 by , 15 years ago
| Cc: | added |
|---|
comment:6 by , 14 years ago
| Easy pickings: | unset |
|---|---|
| UI/UX: | unset |
comment:8 by , 13 years ago
| Keywords: | sprint2013 added |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
comment:9 by , 13 years ago
I have created a new pull request https://github.com/django/django/pull/831 in the manner suggested by Russel: a new localized_fields ModelForm Meta option, in which the user can list fields for which localize should be enabled.
I have not added localized_excludes, because it doesn't seem to make sense: the default is that no fields are localized, so excluding a field would have the side effect that then, and only then, all non-excluded fields become localized. I think the counter-intuitiveness of this outweighs any potential benefit of having localized_excludes.
comment:10 by , 13 years ago
| Has patch: | set |
|---|
comment:11 by , 13 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Looking good, covering ModelForm and ModelFormSet, including documention and changes.
I also support your design decision to not add localized_excludes.
comment:12 by , 13 years ago
| Cc: | added |
|---|
comment:13 by , 12 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
I discussed the ticket with Russel. He agreed with not implementing localized_excludes, and also suggested I add __all__ as a special value to localize all fields - in the same manner as fields will do in Django 1.6.
New pull request with these changes: https://github.com/django/django/pull/1084
comment:14 by , 12 years ago
| Keywords: | dceu13 added |
|---|
comment:15 by , 12 years ago
Small addition compared to last review. Still looking good, tests pass and covering all the areas.
One small comment and easy fix: You refer in your documentation to the special value __all__, however, since this is also a reserved keyword it can lead to confusion that people should provide not the variable __all__ but the string "__all__".
Otherwise, good to go.
comment:16 by , 12 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Docs changed from __all__ to '__all__' (consistent with existing notation).
comment:17 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:19 by , 12 years ago
Pushed some backwards compat shim in 1c1695668f678fd8c2db282a79d7d93c11ee0aff
The localize argument was added in [12867] as a solution for #13032. This bug covered a number of common use cases where enabling L10N by default wouldn't be a good idea (in particular, AutoFields, and IntegerFields storing dates, postal codes, and other non-comma-separated data). So, the idea as proposed won't be accepted.
However, I can accept the problem use case of making it easier to propagate the localize setting into fields on a ModelForm. I would suggest that a better approach would be to allow ModelForm Meta options to control the fields that are localized. For example:
class MyForm(forms.ModelForm): class Meta: model = SomeModel localized_fields = ('some_field', 'other_field')There would also be a need for an exclude-based specification:
class MyForm(forms.ModelForm): class Meta: model = SomeModel localized_excludes = ('current_year', 'postcode')There is a case to consider for how localized_fields and localized_excludes would interact -- I suspect the analog with fields/excludes would be suitable.