Opened 15 years ago
Closed 14 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 , 15 years ago
comment:2 by , 15 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 , 15 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 , 15 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 , 15 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 , 15 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 , 15 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 , 14 years ago
| UI/UX: | set |
|---|
comment:9 by , 14 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