Opened 5 years ago

Closed 2 years ago

Last modified 2 years ago

#13546 closed New feature (fixed)

Easier handling of localize field options in ModelForm

Reported by: hsk Owned by: erikr
Component: Forms Version: master
Severity: Normal Keywords: sprint2013 dceu13
Cc: wogan, mgventura, charettes 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 hsk 5 years ago.

Download all attachments as: .zip

Change History (20)

Changed 5 years ago by hsk

comment:1 Changed 5 years ago by russellm

  • Has patch unset
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Feature request: USE_L10N as default value for form Field to Easier handling of localize field options in ModelForm
  • Triage Stage changed from Unreviewed to Accepted

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

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

  • Cc wogan added

comment:4 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 4 years ago by mgventura

  • Cc mgventura added

comment:6 Changed 4 years ago by tusjlanrecords@…

  • Easy pickings unset
  • UI/UX unset

comment:7 Changed 2 years ago by claudep

#19656 has been closed as a duplicate.

comment:8 Changed 2 years ago by erikr

  • Keywords sprint2013 added
  • Owner changed from nobody to erikr
  • Status changed from new to assigned

comment:9 Changed 2 years ago by erikr

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 Changed 2 years ago by erikr

  • Has patch set

comment:11 Changed 2 years ago by joeri

  • Triage Stage changed from Accepted to 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 Changed 2 years ago by charettes

  • Cc charettes added

comment:13 Changed 2 years ago by erikr

  • Triage Stage changed from Ready for checkin to 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 Changed 2 years ago by erikr

  • Keywords dceu13 added

comment:15 Changed 2 years ago by joeri

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 Changed 2 years ago by erikr

  • Triage Stage changed from Accepted to Ready for checkin

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

comment:17 Changed 2 years ago by Erik Romijn <eromijn@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 756b81dbd1a947351670b66c7e91116abe6aa5c2:

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

comment:18 Changed 2 years ago by Florian Apolloner <florian@…>

In 16683f29ea81e1e979ee88c79cbef279046dd8b1:

Merge pull request #1084 from erikr/master

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

comment:19 Changed 2 years ago by apollo13

Pushed some backwards compat shim in 1c1695668f678fd8c2db282a79d7d93c11ee0aff

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