CharField primary keys with underscores are (un)escaped differently in the admin pages

[victor.varvariuc@…]

Migrated from here: https://code.djangoproject.com/ticket/18381#comment:6

Although that bug was fixed 2 years ago, related issues seem still present in the latest stable Django release 1.6.2.
CharField primary keys with underscores are (un)escaped differently in the admin pages, and when reversing admin URLs with django.core.urlresolvers.reverse() or the {% url %} template tag.
The admin pages add "5F" after the underscore in when rendering URLs, but reverse() does not. As a result, links from custom pages into the admin break when the object's primary key contains underscores.

Steps to reproduce:

• Create a model with "id = models.CharField(max_length=100, primary_key=True)" or other similar text primary key field,
• Register the model to the admin site,
• Create a model instance with test_123 as the primary key,
• Visit the admin, and observe admin URLs: they are of the format "/admin/<app>/<model>/test_5F123/"
• Open the shell, and call reverse('admin:app_model_delete', args=test_123)
• The URLs returned are of the format "/admin/<app>/<model>/test_123/" - no 5F
• As a result, if these links are embedded in pages, they don't work - the admin page tries to decode the primary key, expects 5F, and does not find the object.

[trains] vic@vic ~/projects/trains (master *) $ ./manage.py shell
Python 3.3.2+ (default, Oct  9 2013, 14:50:09) 

>>> from django.core.urlresolvers import reverse

>>> reverse('admin:trains_route_change', args=('test_123',))
>>> '/trains/route/test_123/'

[Claude Paroz]

Might be related to #22223

[nott]

This issue is not related to #22223

[nott]

ModelAdmin.get_urls defines the urls like these:

            url(r'^$', wrap(self.changelist_view), name='%s_%s_changelist' % info),
            url(r'^add/$', wrap(self.add_view), name='%s_%s_add' % info),
            url(r'^(.+)/history/$', wrap(self.history_view), name='%s_%s_history' % info),
            url(r'^(.+)/delete/$', wrap(self.delete_view), name='%s_%s_delete' % info),
            url(r'^(.+)/$', wrap(self.change_view), name='%s_%s_change' % info),

As long as we do not know the exact primary key regexp, we have to use (.+)
Consider we have 2 entries with the following PKs: "test_123" and "test_123/history". There is no way to distinguish change view for "test_123/history" and history view for "test_123".
That is why PK quoting was introduced. django.contrib.admin.utils contains 2 functions: quote and unquote. So reversing admin url should usually look like:

reverse('admin:app_model_change', args=(quote(pk),))

Quoting and unquoting doesn't change numerical PKs, that is why this problem haven't been noticed before.
By the way, there is a bug with quoting not being applied in add_view, but it can be easily fixed.

I suppose we are really lacking the documentation (​https://docs.djangoproject.com/en/1.6/ref/contrib/admin/#overriding-vs-replacing-an-admin-template is evertyhing I could find).

There is no way to get rid of quoting if we leave (.+) url pattern for PK. So we can leave everything as is.
We still can make this pattern configurable, e.g.:

class ModelAdmin(BaseModelAdmin):
    [...]
    pk_regexp = ".+"
    [...]
    def get_pk_regexp(self):
        return self.pk_regexp

[nott]

Pull request: ​https://github.com/django/django/pull/2666/

[Russell Keith-Magee <russell@…>]

In ebd70d4d00c252d5122c13906da5bddc8de0bce5:

Fixed #22266 - quote PK before redirecting away from add_view (django.contrib.admin)

[Russell Keith-Magee <russell@…>]

In 75d2da797e100442d3573dfa7ae490972cea32d8:

[1.7.x] Fixed #22266 - quote PK before redirecting away from add_view (django.contrib.admin)

Backport of ebd70d4d00c252d5122c13906da5bddc8de0bce5 from master.