Code

Opened 4 years ago

Closed 3 years ago

#14101 closed (worksforme)

Localized DecimalField doesn't accept localized input

Reported by: dfoerster Owned by: nobody
Component: Internationalization Version: 1.2
Severity: Keywords: localize
Cc: michelts@…, charette.s@…, aledr Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I'm having trouble with a localized form field not accepting localized input. The form is defined like this:

class XyForm(forms.ModelForm):
	def __init__(self, *args, **kwargs):
		super(XyForm,self).__init__(*args,**kwargs)
		self.fields['betrag'].localize = True

USE_L10N is set to true and LANGUAGE_CODE to de-de. However, entering a number with a comma as a decimal separator triggers a validation error. (Enter a number.) I tried to dig into this and found, that from a python shell started with manage.py the get_language() function from utils.translation returns 'en-us' instead of the configured 'de-de'.

Attachments (3)

1 - Display.png (20.1 KB) - added by aledr 3 years ago.
2 - Change.png (23.2 KB) - added by aledr 3 years ago.
3 - Input With Comma.png (26.5 KB) - added by aledr 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 4 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Can you confirm that you are using the 1.2 release?. Si so, try with a checkout form the releases/1.2.X SVN branch. There have been a fix for localized output/input formats on form fields after 1.2.1 that will be included in the 1.2.2 one.

comment:2 follow-up: Changed 4 years ago by dfoerster

This issue persists using the current version from trunk (r13746).

comment:3 in reply to: ↑ 2 Changed 4 years ago by ramiro

Replying to dfoerster:

This issue persists using the current version from trunk (r13746).

What happens if you use 'de' instead if 'de-de' for LANGUAGE_CODE?

comment:4 Changed 4 years ago by michelts

I'm having similar problems. I'm using the pt_BR language code.

First I tested with django.1.2.3 and trunk version. I make a model form with datetime fields and decimal fields. The tests run as follow:

Django 1.2.3

  • datetime fields input: ok
  • datetime fields output: ok
  • decimal fields input: ok
  • decimal fields output: not ok

When using a form for an instance, the original values are not filled properly according the locale set. It can be fixed by adding a "prepare_value" method on the DecimalField class to convert the original output.

Anyway, even making this change, the correct value will only be picked if the Field instance has the localize attribute set to true. I think it should localize it always if the USE_L10N is set to true. This way, the Field should call the number_format and sanitize_separators methods always, these methods can decide is they will convert (USE_L10N is on) or not convert (USE_L10N is off) the value.

Django trunk

  • datetime fields input: ok
  • datetime fields output: not ok
  • decimal fields input: ok
  • decimal fields output: not ok

In the trunk, even the datetime output is not correct. The problem is another one.

There was a fix on the way the formats modules was get, it is now cached. Some code was broken during the cache implementation and this way, a generic language is used in prior of a specific one when the oppose is expected (e. g. pt is used in prior of pt_BR).

This way, I think there is 3 bugs on this topic:

  1. the DecimalField class should have a prepare_value method to localize the number if needed;
  2. the DecimalField to_python method should call sanitize_separators always, this method will act only if the USE_L10N is on;
  3. the order of the language picked by the get_format / get_format_modules should be fixed to priorize specific languages;

I can do the work if you agree with my proposes.

Best regards!

comment:5 Changed 4 years ago by michelts

  • Cc michelts@… added

comment:6 Changed 3 years ago by charettes

Any update on this ticket and mitchetls's approach? The DecimalField bug part (haven't tested the datimefield issue) is still present both in trunk and 1.2.3.

comment:7 Changed 3 years ago by charettes

  • Cc charette.s@… added

comment:8 Changed 3 years ago by russellm

  • Resolution set to worksforme
  • Status changed from new to closed

This ticket is suffering from an excess of ambiguity. It's a little hard to agree with proposals when the analysis consists of the text "not ok". What does "not OK" mean? What are you considering "OK"? What models, settings, middleware etc are you using? How are you generating "output"?

The input_formats test module in the forms regression tests contains a number of tests for datefields etc, and to my inspection, they all appear correct.

I'm also not convinced that the behavior of DecimalField is incorrect as currently implemented. To my testing, a correctly localized DecimalField *will* accept comma-based decimal separation.

What is really needed is a programatic test case; ideally, integrated into Django's own test suite. Closing worksforme until someone provides a clear test case.

comment:9 Changed 3 years ago by aledr

  • Cc aledr added
  • Resolution worksforme deleted
  • Status changed from closed to reopened

My tests using Django's admin displays DecimalField using comma as separator but does not accept a input with comma.

comment:10 Changed 3 years ago by russellm

  • Resolution set to worksforme
  • Status changed from reopened to closed

... and again, what is needed is a *clear test case*. Ideally, this would be a programatic test case, but I'll settle for a clear set of instructions. I have a set of test cases that test this feature, and they are currently passing. If you are convinced there is a problem, you need to show me *exactly* what is going wrong, and where.

Changed 3 years ago by aledr

Changed 3 years ago by aledr

Changed 3 years ago by aledr

comment:11 Changed 3 years ago by aledr

  • Resolution worksforme deleted
  • Status changed from closed to reopened

settings.py has LANGUAGE_CODE = 'pt-br'
You can check at 'django/conf/locale/pt_BR/formats.py' that DECIMAL_SEPARATOR is set to ','.

Please note fields 'Pressão' e 'Vazão', both are DecimalFields, check how it displays the list, the change form and the error when I try to input a comma separated number in the change form.

comment:12 Changed 3 years ago by russellm

  • Resolution set to worksforme
  • Status changed from reopened to closed

This *still* isn't a reproducible test case. A reproducible test case requires code samples, or a full set of instructions. These instructions or code samples tells someone else *exactly* how to reproduce a problem. i.e., here are my models, here are my admin registrations, etc. Three screenshots is *not* a reproducible test case.

However, based on what I can glean from your screenshots, this appears to be working as expected. Django's admin doesn't localize widgets by default. If you want localized widgets in the admin, you need to override forms. This is something that could be made easier, but doesn't change the fact that it isn't a bug as described.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.