Opened 3 years ago

Closed 3 months ago

Last modified 8 weeks ago

#22654 closed Bug (fixed)

DecimalField and DECIMAL_SEPARATOR (in admin)

Reported by: matija@… Owned by: Raphael Michel
Component: Forms Version: 1.11
Severity: Normal Keywords:
Cc: tonnzor Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

DecimalField with following settings:

LANGUAGE_CODE = 'hr-hr'
TIME_ZONE = 'CET'
USE_I18N = True
USE_L10N = False
USE_TZ = False
DATE_FORMAT = 'd.m.Y.'
DATE_INPUT_FORMATS = ('%d.%m.%Y.', '%d.%m.%Y', '%Y-%m-%d')
DECIMAL_SEPARATOR = ','

Correctly shows Decimal field in admin, but raises validation error.

As most of our customers do not set their preferences to Croatian, and we need to show Croatian locale for all users. So with settings above, things sdhould work.

Tried in 1.6 and 1.7.

In order to reproduce, all you need is a single model with a DecimalField, registered in Admin. And the above settings. Input any non-integer value and press save_and_continue_editing. If you encounter no errors, press it again. Now it will show an error.

I added a new ticked because related ones were closed years ago.

Change History (20)

comment:1 Changed 3 years ago by sabinemaennel

Owner: changed from nobody to sabinemaennel
Status: newassigned

comment:2 Changed 3 years ago by sabinemaennel

I think this is clearly an error. I tested it with a decimal field. The settings DECIMAL_SEPARATOR = ',' is taken into account when the field is shown in the admin, but when validating the field only input with a . as delimiter is accepted. The field validation needs to be adapted to the delimiter given in the settings as well. I tested this with the new 1.7 Django.

comment:3 Changed 3 years ago by sabinemaennel

Component: UncategorizedForms
Owner: sabinemaennel deleted
Status: assignednew
Triage Stage: UnreviewedSomeday/Maybe
Type: UncategorizedBug

This is actually an HTML5 Problem. The admin form for the input of a number uses the following HTML5 statement:
<input id="id_decimal_number" name="..." step="0.01" type="number" />.
step= in HTML5 can only be defined that way with a .as delimiter. Then the browser will show it on the screen according to local browser settings. Mine showed it as a dot, but I read on stackoverflow, that some other users see it with a comma.http://stackoverflow.com/questions/6178332/force-decimal-point-instead-of-comma-in-html5-number-input-client-side.

I think HTML5 will develop further and this behaviour can be corrected via a Javascript-Library-Use in the meantime. I am not sure what to do with this bug. I would probably place it outside the influence of Django even though I can see that it is annoying. Django decided to use HTML5 for its forms and this is due to the actual state of HTML5. I feel Django is not responsible to correct HTML5 behaviour, but I would really like to know the opinion of some more experienced Django contributers on this. So I am handing the ticket back.

comment:4 Changed 3 years ago by matija@…

ACTUALLY this is DJANGO problem. Remove type="number". The ValidationError on Django side is the issue, not browser side.

comment:5 Changed 3 years ago by Claude Paroz

Could you please give us a little more information: what is displayed exactly in your widget (dot or comma), what value is actually posted to the request, what validation error message do you get ?

comment:6 Changed 3 years ago by anonymous

Sure. With settings above, everything works as excpected, comma is shown, but when I try to save, ValidationError is shown.
Error is, of course, in croatian, but it comes back to this:

invalid': _('Enter a number.'), https://github.com/django/django/blob/master/django/forms/fields.py

If L10N is set tu True, everything actually works. The problem is in this line. When self.localize is set to true, this works fine, but this should be run when localization is off, but DECIMAL_SEPARATOR is anything other than '.'. Would anything break if this "if" is simple removed?

if self.localize:

value = formats.sanitize_separators(value)

comment:7 Changed 3 years ago by Claude Paroz

So do you really get a comma in the widget value, even when L10N is off? Is that dot-to-comma conversion done by the browser itself (AFAIK Django is sending a dot in the HTML source)?

comment:8 Changed 3 years ago by matija@…

Ok, I got it.

This is definitely a design problem. Date/Datetime settings work regardless of L10N, and Decimal-related only when L10N is set. (sanitize_separators checks L10N first).

These two things are closely related and should follow the same logic.

IMHO the key question is should Dates AND Decimals (and other L10N-related stuff) follow DATE_FORMATS, DECIMAL_SEPARATOR and other L10N related setting when L10N is False. The answer should be Yes or No for all of them.

Yes, I do get a comma in the output. However, on re-saving, errors are shown. If I replace it with a dot, it works, and comma is shown in response.

Last edited 6 months ago by Anton Samarchyan (previous) (diff)

comment:9 Changed 3 years ago by anonymous

Sorry, no comma with the default widget. However, with TextInput, it is there.

This design problem still exists, at least docs should be updated indicating what works when. This definitely does not work as one may expect.

However, there SHOULD be a way to work with a fixed locale other than en-us ;)

comment:10 Changed 3 years ago by Claude Paroz

Triage Stage: Someday/MaybeAccepted

I think we should be as much as flexible as possible on the input side of things (potentially removing the is_localize/USE_L10N tests).
However, it's a bit tricky when you get ambiguous inputs like 24,637...

comment:11 Changed 3 years ago by matija

Well, it is clear that nothing should be done quickly. The whole thing seems a bit messy. I did test many different configs and mostly got unexpected results, either in Decimal or Date.

Id say this is 1.8 issue.

My deadline is approaching and I already spent too much time on this, right now I have a middleware forcing locale I want and L10N turned on and formats ... works fine.

Ill do an update when I find time to re-test from scratch (weekend perhaps).

comment:12 Changed 3 months ago by tonnzor

I have the same issue.

It is very easy to reproduce:

# in settings.py

LANGUAGES = (('de', 'German'),)
LANGUAGE_CODE = 'de'

# in models.py

CHOICES = (
    (Decimal('0.25'), '0.25d'),
    (Decimal('0.50'), '0.5d'),
    (Decimal('0.75'), '0.75d'),
    (Decimal('1.00'), '1d'),
)

class Number(models.Model):
    num = models.DecimalField(max_digits=3, decimal_places=2, choices=CHOICES)

# in admin.py

@admin.register(Number)
class NumberAdmin(admin.ModelAdmin):
    pass

If you go to admin and try to save it would give you a validation error.

Reason is that it generates following HTML -- see comma in value:

<select name="num" id="id_num">
  <option value="0,25">0.25d</option>
  <option value="0,50">0.5d</option>
  <option value="0,75">0.75d</option>
  <option value="1,00" selected="">1d</option>
</select>

But when you provide choices -- it would be TypedChoiceField. Somehow it cannot handle comma in value and convert it correctly to decimal.

Interesting here if you drop choices. Then DecimalField would be used and everything would work -- you would use comma in the field.

BUT if you check HTML, it would use dots internally:

<input name="num" value="0.25" step="0.01" required="" id="id_num" type="number">

Also, POST request would send dot-separated value, not comma (though comma is shown in UI).

It seems if we force TypedChoiceField generate dot-separated HTML for Decimals despite of localization and i18n -- it would fix the issue.

Last edited 3 months ago by tonnzor (previous) (diff)

comment:13 Changed 3 months ago by tonnzor

Version: 1.61.11

It is still present in Django 1.11rc1

comment:14 Changed 3 months ago by tonnzor

Cc: tonnzor added

comment:15 in reply to:  13 Changed 3 months ago by inoks

Replying to tonnzor:

It is still present in Django 1.11rc1

Also present in Django 1.11 release

comment:16 Changed 3 months ago by Raphael Michel

Owner: set to Raphael Michel
Status: newassigned

comment:17 Changed 3 months ago by Raphael Michel

Has patch: set

comment:18 Changed 3 months ago by Florian Apolloner <apollo13@…>

Resolution: fixed
Status: assignedclosed

In bde81414:

Fixed #22654 -- Broken decimal validation

comment:19 Changed 3 months ago by Claude Paroz

I think this change warrants a note in backwards incompatibilities:

For example, with following settings:

LANGUAGE_CODE='de'
USE_L10N=False
USE_THOUSAND_SEPARATOR=True

Before: "1.345" => 1.345
After: "1.345" => 1345

comment:20 Changed 8 weeks ago by Tim Graham <timograham@…>

In 504e778:

Refs #22654 -- Added additional tests and amended release note.

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