Opened 13 years ago
Closed 13 years ago
#15922 closed Bug (duplicate)
prepopulated_fields for DecimalField generates js error
Reported by: | lev | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | 1.3 |
Severity: | Normal | Keywords: | prepopulated_fields Decimal default_if_none |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | yes |
Description
1)
maxLength: {{ field.field.field.max_length|default_if_none:"50" }} )
is rendered as
maxLength: )
2) DecimalField doesn't have max_length attribute (raises AtttibuteError).
3)
{% ifequal field.field.field.max_length None %}yes{% else %}no{% endif %}
returns yes;
4)
{{ field.field.field.max_length|default:"50" }}
generates 50 being an imperfect workaround since it should be default_if_none
Change History (9)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Missing attributes are replaced by settings.TEMPLATE_STRING_IF_INVALID
, which defaults to ''
(the empty string) and not None
.
See http://docs.djangoproject.com/en/dev/ref/templates/api/#how-invalid-variables-are-handled
Django works as advertised here.
The doc states that it is invalid to compare something to a Python object such as None
with the ifequal
tag.
See http://docs.djangoproject.com/en/dev/ref/templates/builtins/#ifequal
What actually happens in your example is that both field.field.field.max_length
and None
evaluate to ''
in a template context, so the condition matches.
comment:3 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
OK, once again from the beginning:
class Book(models.Model): starting_price = DecimalField() price = DecimalField() class BookAdmin(models.ModelAdmin): prepopulated_fields = {'price': ['starting_price']}
This does not work as intended
comment:4 by , 13 years ago
Documentation states:
prepopulated_fields doesn't accept `DateTimeField`, `ForeignKey`, nor `ManyToManyField` fields.
Also, ImproperlyConfigured
is raised in such cases.
So either:
1) perpopulated_js
should be fine-tuned somehow to work with DecimalFields
as well (ie not trim decimal point/decimal comma) or
2) DecimalField
should be included into the list of unsupported fields to docs and raise ImproperlyConfigured
as well.
comment:5 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
lev, we can't see your code, so we need some context to understand the problem you're reporting. See the instructions here for how to write a good bug report:
http://docs.djangoproject.com/en/dev/internals/contributing/#reporting-bugs
I could eventually reproduce the problem, here is a minimal test case that works:
# models.py from django.db import models class Book(models.Model): starting_price = models.DecimalField(decimal_places=2, max_digits=10) price = models.DecimalField(decimal_places=2, max_digits=10) # admin.py from django.contrib import admin import models class BookAdmin(admin.ModelAdmin): prepopulated_fields = { 'price': ['starting_price'], } admin.site.register(models.Book, BookAdmin)
With this code, trying to add a new book with the admin results in the javascript error described in the initial bug report. This error is in dynamically generated javascript in the HTML.
comment:6 by , 13 years ago
I found out that the bug does not occur if DecimalField
is used only as a source field and not as a destination.
So while changing the behaviour of prepopulated_fields
so that it accepts DecimalField
as destination field is somewhat dubious, here're some fixes that can be applied without such design decision:
1) In documentation:
- prepopulated_fields doesn't accept DateTimeField, ForeignKey, nor ManyToManyField fields. + prepopulated_fields doesn't accept DateTimeField, ForeignKey, nor ManyToManyField fields as source fields.
2) Prohibit DecimalField
to be used as a destination field in the code.
3) Currently type control is applied to destination field only. Prohibiting DateTimeField
as a source field could also make sense as it doesn't work either.
comment:7 by , 13 years ago
To be more explicit, the following works well:
# models.py from django.db import models class Book(models.Model): slug = models.CharField(max_length=100) starting_price = models.DecimalField(decimal_places=2, max_digits=10) # admin.py from django.contrib import admin import models class BookAdmin(admin.ModelAdmin): prepopulated_fields = { 'slug': ['starting_price'], } admin.site.register(models.Book, BookAdmin)
comment:8 by , 13 years ago
UI/UX: | set |
---|
comment:9 by , 13 years ago
Resolution: | → duplicate |
---|---|
Status: | reopened → closed |
The code I'm talking about is here: django/contrib/admin/templates/admin/prepopulated_fields_js.html:10