[patch] prepopulated_fields only work with slug fields and break on non-char fields

[Peter Kese]

While analyzing ticket #15922, it became apparent that prepopulate_from field in Django admin unconditionally assumes that all prepopulated fields are of type SlugField and thus the URLify function in javascript is always called.

In addition it is assumed that each target field has a max_length attribute. However, only fields that inherit from a CharField have max_length. For all other field types, the javascript just fails.

Besides samples in #15922, here is another sample case in which any text in the starting_price field will be prepopulated in price field slugified:

# models.py

from django.db import models

class Book(models.Model):
    starting_price = models.CharField(max_length=100)
    price = models.CharField(max_length=100)

# admin.py

from django.contrib import admin
import models

class BookAdmin(admin.ModelAdmin):
    prepopulated_fields = {
        'price': ['starting_price'],
    }

admin.site.register(models.Book, BookAdmin)

[Peter Kese]

A patch for this ticket including tests

[Peter Kese]

I have uploaded a patch, and here is what it does...

First of all it tries to distinguish between SlugFields and other field types in order not to call URLify javascript functions for any fields that are not slugs.
I have thus added an AdminSlugFieldWidget and registered it for use on SlugFields in admin. The sole purpose of AdminSlugFieldWidget is to add an extra vSlugField class on the HTML tag of the slug text-input widget.

The prepopulate.js then distinguishes widgets with the class vSlugField from other pre-populated widgets and applies slugification only to input fields with the class assigned.

In addition there is a patch to prepopulated_fields_js.html that handles all field types that don't have a max_length attribute (only descendants of the CharField would have this attribute). Since max_length is only used for SlugFields (only as a parameter when calling URLify) it is safe to set it to null for any other field types.
Not setting it at all would generate invalid javascript syntax which will break all the pre-population mechanism - which is the cause of bug #15922.

The resolution is that prepopulated_fields will now work for all fields. It will automatically slugify any slug fields while leaving others intact.

[Peter Kese]

Small code cleanups

[Peter Kese]

I just uploaded a small update to the patch, that solves for the default value handling in max_length javascript.

Now, instead of

maxLength: {{ field.field.field.max_length|default_if_none:"50" }}

I chose to rewrite that as

maxLength: {{ field.field.field.max_length|default:'null' }}

Rationale:

In the new code, maxLength is only used for URLifying Slug fields and can be ignored (set to null or anything else) for any other field type.
According to documentation, max_length is required on any CharField (thus including SlugField). In addition,
if max_length is set to None (or omitted) when defining a SlugField, the SlugField's __init__() code will set the value to 50 - see browser/django/trunk/django/db/models/fields/__init__.py.
It is thus safe to assume that max_length will be set to a proper value for each SlugField instance and we thus don't need to do a special case in the javascript generation template.
As said, in all other cases max_length can be safely be ignored as the URLify doesn't even get called.

[Peter Kese]

Updated tests...

[Aymeric Augustin]

Basically, this ticket is about extending the current behavior described in ​https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.prepopulated_fields

The issue of prepopulated_fields only working with SlugField is a duplicate of #11432 and #4509, both of which were closed as wontfix. (You should search existing tickets before opening a new one.)

The issue of the missing max_length was initially reported in #15922, and I see that you closed it in favor of this ticket.

If I apply strictly the triaging rules, I should close this ticket as wontfix (or restrict it to clarifying the documentation) and reopen #15922. However, I see that you've put some effort into the patch. Do you want to bring the topic on the mailing list? This is the only way to reverse a "wontfix" by a core developer.

On a side note, #13386 is related.

[Aymeric Augustin]

Since the "wontfix" on #4509 was weak ("don't mind a wontfix") and a long time ago, and interest for this issue has surfaced again, I'm marking this as DDN in order to choose between:

• 1) By design, prepopulated_fields works only when the target is a SlugField => then it's a documentation issue, and #15922 must be marked as "wontfix" too.

• 2) prepopulated_fields should work with any kind of field => then there should be a discussion of how exactly it behaves with non-slug fields, and this ticket is a good starting point.

If 2) is chosen, since prepopulating non-slug fields was neither explicitly allowed or forbidden, it's difficult to say what is backwards compatible and what isn't. We should try to limit the changes to the current behavior, unless it's obviously buggy.

My advice of writing to django-developers still stands :)

[Aymeric Augustin]

I don't think this was ever brought up to django-developers, so...