Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#18471 closed New feature (wontfix)

Add a localization field option in Model Fields.

Reported by: serge_spaolonzi Owned by: serge_spaolonzi
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: florian+django@… Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Add a new field option for localization in Model Fields in order to simplify the process of localization.
It happens that activating the localization of a field in the Admin site from a model is a complex task now.
Instead it can be done easily using only a parameter in the Model declaration, in order to archive this I had to modify three lines in the file django.db.models.fields.init.py

Example:
Expected result localization of a decimal field in a model according to the THOUSAND_SEPARATOR setting.

Currently it is done this way

import django

class LocalizedModelForm(django.forms.ModelForm):
    def __new__(cls, *args, **kwargs):
        new_class = super(LocalizedModelForm, cls).__new__(cls, *args, **kwargs)
        for field in new_class.base_fields.values():
            if isinstance(field, django.forms.DecimalField):
                field.localize = True
                field.widget.is_localized = True
        return new_class

class FooForm(LocalizedModelForm):
    class Meta:
        model = Foo

django.admin.site.register(Foo, form=FooForm)

After improvement it can be done easily

class Foo(models.Model)
    price = models.DecimalField(verbose_name='Price', max_digits=20, decimal_places=2, localize=True)

I have the patched code in my Github fork, I will update this ticket with the link to my github code.

Change History (10)

comment:1 Changed 3 years ago by serge_spaolonzi

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to serge_spaolonzi
  • Patch needs improvement unset
  • Status changed from new to assigned
Last edited 3 years ago by serge_spaolonzi (previous) (diff)

comment:2 Changed 3 years ago by serge_spaolonzi

  • Summary changed from Add a new field option for localization in Model Fields. to Add a localization field option in Model Fields.

comment:3 Changed 3 years ago by serge_spaolonzi

This is the link to my fork in Github:
https://github.com/cobalys/django/tree/ticket_18471

The modified file is: django/django/db/models/fields/init.py

comment:4 Changed 3 years ago by apollo13

  • Cc florian+django@… added
  • Triage Stage changed from Unreviewed to Design decision needed

I am pretty sure that localize=True is not the way to go (should be False for backwards compat). Aside from that I am a bit hesitant to add form options to the model field definitions. Either way the patch misses docs and tests.

comment:5 follow-up: Changed 3 years ago by Serge Spaolonzi (serge@…

I have updated the code:

  1. Default value to False
  2. Documentation included
  3. Test included: ./tests/runtests.py --settings=test_sqlite localization

Replying to apollo13:

I am pretty sure that localize=True is not the way to go (should be False for backwards compat). Aside from that I am a bit hesitant to add form options to the model field definitions. Either way the patch misses docs and tests.

There are already some options in the model field that are related to the Form/Widget display: help_text and editable.

I think this feature is needed, maybe there is another way to implement it.
One of the issues it deals with is the decimal separator implementation in the Admin site, in the USA and Canada people use the dot while people in Europe and Latino America use the comma.

I have tried other workarounds to help this issue but this one seems to be the cleanest.

comment:6 in reply to: ↑ 5 Changed 3 years ago by apollo13

Replying to Serge Spaolonzi (serge@…:

There are already some options in the model field that are related to the Form/Widget display: help_text and editable.

Yes, those are there due to history (blank is also just Form related).

One of the issues it deals with is the decimal separator implementation in the Admin site, in the USA and Canada people use the dot while people in Europe and Latino America use the comma.

No argument here, we use comma here too…

I have tried other workarounds to help this issue but this one seems to be the cleanest.

What are those other workarounds? Maybe if we all know them we can think of a cleaner solution?

comment:7 Changed 3 years ago by vanschelven

I just opened a duplicate ticket 'cause I missed this one. (#18506). Copy/pasta below for convenience.


As it is now one cannot make localized ModelForms without violating DRY. Developers w/o the desire for localized data in their application can simply take a model and then create a modelform in three lines and an admin interface in even less. However, a developer that wants localization to work in the admin/their modelforms needs to set the attribute localize=True for each single field that they want localized. This breaks DRY as there is both repetition of the "localize=True" and it also introduces new repetition between models & their forms.

I understand that the decision to make localize=False the default was both deliberate and probably the correct one, as Russel explains here: http://groups.google.com/group/django-developers/browse_thread/thread/01096cd968bf276a/9ad982e939f1284f

However, a couple of notes:

  • I think the case Russel mentions is the exception, not the rule. "Usually" a number is just a number, not a year. Postal codes and phone numbers are often not numbers at all.
  • We (Django) have made the opposite decision for template rendering: Indicating USE_L10N in settings makes localized rendering the default throughout the application, and the developer is left to find the exceptions and note them whenever they occur. This is inconsistent with the way we deal with forms (and has the usual drawbacks of inconsistency, such as the confusion of the OP in the email above).

My suggested solution would be to add an attribute to ModelFields.

We already have a number of cases in which attributes of models' fields are propagated to your modelforms. blank => not required, verbose_name => label and help_text => help_text spring to mind.

I'd say name this attribute "localize" and default it to True (since that's the most common case). But this will introduce backwards breakage, so I can also imagine simply keeping it False as a default. That would at least get rid of the violation of DRY mentioned above. Alternatively the default value of this attribute could be settings.USE_L10N.

Once we have localize=True/False on modelfields, we could even envision using that value in template output as well, i.e. years will automatically be recognized as such and we no longer need to explicitly mark them as non-localized each time they're used in a template. But maybe that's ground for another ticket.

comment:8 Changed 3 years ago by vanschelven

Responding to the discussion here, the following remarks:

  • If blank/help_text/verbose_name are considered undesirable elements of Model Fields I'd love to know what the new canonical pattern for DRY between models and corresponding forms is. I _do_ think there is value in not repeating yourself between forms & models.
  • I'm not sure on localize=False being the correct default either; see the remarks above about the behavior of L10N in templates.
  • My workaround is to patch Django's code in a local branch. Not so great I suppose.

comment:9 Changed 3 years ago by jezdez

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

No option about a presentation problem should be added to model fields, citing previously made mistakes that are hard to remove (e.g. help_text) isn't a reason to start doing that.

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