#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 , 14 years ago
Attachment: | localized_form_fields.diff added |
---|
comment:1 by , 14 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 , 14 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 , 14 years ago
Cc: | added |
---|
comment:4 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:5 by , 14 years ago
Cc: | added |
---|
comment:6 by , 13 years ago
Easy pickings: | unset |
---|---|
UI/UX: | unset |
comment:8 by , 12 years ago
Keywords: | sprint2013 added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:9 by , 12 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 , 12 years ago
Has patch: | set |
---|
comment:11 by , 12 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 , 12 years ago
Cc: | added |
---|
comment:13 by , 11 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 , 11 years ago
Keywords: | dceu13 added |
---|
comment:15 by , 11 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 , 11 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Docs changed from __all__
to '__all__'
(consistent with existing notation).
comment:17 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:19 by , 11 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:
There would also be a need for an exclude-based specification:
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.