USE_THOUSAND_SEPARATOR breaks prepopulated_fields functionality

[lev.maximov@…]

# models.py

from django.db import models

class Book(models.Model):
    slug = models.CharField(max_length=1000)
    name = models.CharField(max_length=1000)

# admin.py

from django.contrib import admin
import models

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

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

# settings.py

USE_L10N = True
USE_THOUSAND_SEPARATOR = True

This bug is present with most locales (including en and en-gb). Common values for DECIMAL_SEPARATOR are space, comma and dot. The former two generate a js error in django-generated js code, the latter one results in wrong max_length for slug field (1.000 = 1)

Disclaimer:

This example is somewhat artificial because NUMBER_GROUPING is usually 3 and it doesn't make much sense to use slugs 1000 chars long.
So the severity of the bug is not at all high. But the bug there is.

The patch is against 1.3 but at the moment of writing the issue is present in trunk as well.

[Mathieu Agopian]

I've reproduced the issue, and can confirm everything stated in the ticket with one minor typo (it's maxLength in the javascript and not max_length).

I'm marking this ticket as Accepted but the patch needs improvements: please remove the trailing comma, as it wasn't there in the first place, and it'll break in Internet Explorer.

[lev]

removed the comma

[Mathieu Agopian]

Thanks lev for the update of the patch, looks good now. Sorry for that, but i missed the "needs tests" flag in my previous comment

[Mathieu Agopian]

see #14895 for a similar issue (localization of a pk)

[Mathieu Agopian]

Lev's patch with added regression tests

[Filippo Squillace]

I also report the same bug and the Lev's patch works well. Does it need to be flagged RFC?

[Aymeric Augustin]

Patch doesn't apply cleanly and test_prepopulated_maxlength_localized should use the override_settings decorator.

[anonymous]

I tried to look for override_settings documentation but I didn't find enough. Is it correct the use of it as follows?

    @override_settings(USE_THOUSAND_SEPARATOR = True, USE_L10N = True)
    def test_prepopulated_maxlength_localized(self):
        """
        Regression test for #15938: if USE_THOUSAND_SEPARATOR is set, make sure
        that maxLength (in the javascript) is rendered without separators.
        """

        response = self.client.get('/test_admin/admin/admin_views/prepopulatedpostlargeslug/add/')
        self.assertContains(response, "maxLength: 1000") # instead of 1,000

[Aymeric Augustin]

Yes, that's how it works.

[Filippo Squillace]

Lev's patch with test_prepopulated_maxlength_localized that use the override_settings decorator (I got rid of some new lines).

[Julien Phalip]

In [17033]:

Fixed #15938 -- Prevented the max_length number in the admin prepopulated_fields javascript to be localized when USE_THOUSAND_SEPARATOR is turned on. Thanks to Lev Maximov, Mathieu Agopian and feel.