Opened 14 years ago

Closed 11 years ago

Last modified 11 years ago

#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)

localized_form_fields.diff (946 bytes ) - added by Henrik Karlsen 14 years ago.

Download all attachments as: .zip

Change History (20)

by Henrik Karlsen, 14 years ago

Attachment: localized_form_fields.diff added

comment:1 by Russell Keith-Magee, 14 years ago

Has patch: unset
Summary: Feature request: USE_L10N as default value for form FieldEasier handling of localize field options in ModelForm
Triage Stage: UnreviewedAccepted

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.

comment:2 by Camilo Nova, 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 Greg Wogan-Browne, 13 years ago

Cc: Greg Wogan-Browne added

comment:4 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:5 by mgventura, 13 years ago

Cc: mgventura added

comment:6 by tusjlanrecords@…, 12 years ago

Easy pickings: unset
UI/UX: unset

comment:7 by Claude Paroz, 11 years ago

#19656 has been closed as a duplicate.

comment:8 by Sasha Romijn, 11 years ago

Keywords: sprint2013 added
Owner: changed from nobody to Sasha Romijn
Status: newassigned

comment:9 by Sasha Romijn, 11 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 Sasha Romijn, 11 years ago

Has patch: set

comment:11 by Joeri Bekker, 11 years ago

Triage Stage: AcceptedReady 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 Simon Charette, 11 years ago

Cc: Simon Charette added

comment:13 by Sasha Romijn, 11 years ago

Triage Stage: Ready for checkinAccepted

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 Sasha Romijn, 11 years ago

Keywords: dceu13 added

comment:15 by Joeri Bekker, 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 Sasha Romijn, 11 years ago

Triage Stage: AcceptedReady for checkin

Docs changed from __all__ to '__all__' (consistent with existing notation).

comment:17 by Erik Romijn <eromijn@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 756b81dbd1a947351670b66c7e91116abe6aa5c2:

Fixed #13546 -- Easier handling of localize field options in ModelForm

comment:18 by Florian Apolloner <florian@…>, 11 years ago

In 16683f29ea81e1e979ee88c79cbef279046dd8b1:

Merge pull request #1084 from erikr/master

Fixed #13546 -- Easier handling of localize field options in ModelForm

comment:19 by Florian Apolloner, 11 years ago

Pushed some backwards compat shim in 1c1695668f678fd8c2db282a79d7d93c11ee0aff

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