Opened 3 years ago

Closed 3 years ago

#20777 closed Bug (fixed)

admin delete page proxy models

Reported by: Collin Anderson Owned by: nobody
Component: contrib.admin Version: 1.6-beta-1
Severity: Normal Keywords:
Cc: Collin Anderson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


This #18491 broke proxy model __unicode__() on the admin delete page:

The concrete model is being instantiated with no attributes except the primary key. This breaks obj.__unicode__() because it can't reference any of its fields. (It also means that we're displaying BaseModel.__unicode__() instead of ProxyModel.__unicode__(), which isn't a problem for me.)

I have a simple project you can use to try it yourself:

Change History (10)

comment:1 Changed 3 years ago by Harm Geerts <hgeerts@…>

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

I'm not sure what the desired fix is but I have created 2 possible solutions.

keep the proxy model representation:
copy the instance dict to the concrete model:

comment:2 Changed 3 years ago by Collin Anderson

I just checked both and either one would solve my problem.

comment:3 Changed 3 years ago by Harm Geerts <hgeerts@…>

Has patch: set

comment:4 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

I think the first solution makes the most sense. The model's __str__/__unicode__ method may refer to fields that aren't on the model which would mean the second solution wouldn't work, right?

I'm getting a test failure with Python 3 with the first solution:

FAIL: test_delete_str_in_model_admin (proxy_models.tests.ProxyModelAdminTests)
Traceback (most recent call last):
  File "/django/tests/proxy_models/", line 406, in test_delete_str_in_model_admin
    self.assertEqual(delete_str, proxy_str)
AssertionError: 'Tracker user: <a href="/admin/proxy_models/trackeruser/100/">TrackerUser:</a>' != 'Tracker user: <a href="/admin/proxy_models/trackeruser/100/">ProxyTrackerUser:Django Pony</a>'

Changing obj.__str__ to obj.__unicode__ in the if six.PY3 branch fixes this. Possibly we should set obj.__unicode__ regardless of Python version and obj.__str__ only for Python 3?

comment:5 Changed 3 years ago by Harm Geerts <hgeerts@…>

I don't think it's possible to define fields on a proxy model but it could refer to a property that is not defined on the base model itself. For example:

class Base(Model):
    def __unicode__(self):
        return self.kind

class Pony(Base):
    kind = 'pony'

    class Meta:
        proxy = True

Not sure if this kind of behavior should be supported but it's possible and would break the second solution.

I'll update the first solution for python 3.

comment:6 Changed 3 years ago by Tim Graham

Patch needs improvement: set

As discussed on IRC, the first solution may not work as obj.__unicode__ and obj.__str___ invoke the class-level methods and cannot be overridden on individual instances. The test works for Python 2 only because of the implementation details of @python_2_unicode_compatible which calls self.__unicode__(). Arguably, this should be klass.__unicode__() to mimic the behavior of Python.

comment:7 Changed 3 years ago by Tim Graham

@akaariai says fixing #16458 will mean we can revert the fix for #18491 which caused this regression. Possibly we should just revert it now, live with that bug in 1.6, and wait for #16458 to be fixed in 1.7.

comment:8 Changed 3 years ago by Anssi Kääriäinen

Pull request seems to fix this issue. However the patch can't be applied to 1.6 at this point as it introduces backwards incompatible changes.

comment:9 Changed 3 years ago by Tim Graham

Patch needs improvement: unset
Severity: Release blockerNormal
Triage Stage: AcceptedReady for checkin

PR looks good. I've reverted the fix for #18491 which caused this regression in 1.6.x [c769c266017c]. Removing the release blocker flag in light of that.

comment:10 Changed 3 years ago by Anssi Kääriäinen

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top