[Django] #24851: Reverse one-to-one relations break the admin.

60 views
Skip to first unread message

Django

unread,
May 24, 2015, 6:00:48 PM5/24/15
to django-...@googlegroups.com
#24851: Reverse one-to-one relations break the admin.
-------------------------------+--------------------
Reporter: jerivas | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Including reverse one-to-one relations in `list_display` or `fields` in a
`ModelAdmin` breaks the "change_list" and "change" admin views.

The "change" view raises a `FieldError` saying the reverse one-to-one
field is unknown. The "change_list" raises `AttributeError`: 'OneToOneRel'
object has no attribute 'rel'.

I've setup a small test app here: https://gitlab.com/jerivas/one-to-one-
bug.

To reproduce:
- Clone the repo
- Install deps from requirements.txt
- Start the development server
- Visit the admin. User/pass: proj/proj
- Both the Car and Engine admins should work
- Uncomment lines 7 & 8 from `cars/admin.py`
- Both the "change_list" and "change" views of the Car admin are now
broken. The Engine admin continues to work as expected.

This worked normally in 1.6, and I just found it while updating a project
from 1.6 to 1.8. I don't know if the bug was introduced in 1.7 or 1.8.

--
Ticket URL: <https://code.djangoproject.com/ticket/24851>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
May 25, 2015, 8:57:23 AM5/25/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+------------------------------------

Reporter: jerivas | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by timgraham):

* severity: Normal => Release blocker
* needs_better_patch: => 0
* component: Uncategorized => contrib.admin
* needs_tests: => 0
* needs_docs: => 0
* stage: Unreviewed => Accepted


Comment:

Reproduced the `list_display` crash and bisected the regression in 1.8 at
fb48eb05816b1ac87d58696cdfe48be18c901f16. It's fixed in master by
8f30556329b64005d63b66859a74752a0b261315 but we should fix it in 1.8 too.
Attaching a regression test.

As for the change view error, that seems to be reproducible on
stable/1.6.x and stable/1.4.x so I don't think it's a regression or a bug.

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:1>

Django

unread,
May 25, 2015, 8:57:39 AM5/25/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+------------------------------------
Reporter: jerivas | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by timgraham):

* Attachment "24851-test.diff" added.

Django

unread,
May 25, 2015, 9:48:44 AM5/25/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+------------------------------------
Reporter: jerivas | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by jerivas):

Whoops, you're right, I was only using `list_display` in 1.6. Sorry about
that.

However, wouldn't it be appropriate to support reverse one-to-one
relations in `fields`? I see three possible scenarios:

- Reverse one-to-one relations fully supported in `fields`.
- Supported only when they appear in `readonly_fields`.
- Unsupported, but they raise a more useful exception and is explained in
the docs. The current exception says the one-to-one field is unknown to
the model, which is confusing and gives the impression the user has made a
typo in `fields`. IMO `ImproperlyConfigured` would be better.

If you think any of the above have the potential of flying as a feature
request, I can open a new ticket.

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:2>

Django

unread,
May 25, 2015, 2:31:52 PM5/25/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+------------------------------------
Reporter: jerivas | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------

Comment (by timgraham):

I don't think it could be easily supported in `fields` as that would
require saving the related instance. `readonly_fields` support might be
doable. I think a natural suggestion for the error message would be to
suggest to use an inline.

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:3>

Django

unread,
May 27, 2015, 10:39:49 AM5/27/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+-------------------------------------
Reporter: jerivas | Owner: timgraham
Type: Bug | Status: assigned

Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------
Changes (by timgraham):

* status: new => assigned
* owner: nobody => timgraham


--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:4>

Django

unread,
May 27, 2015, 12:00:49 PM5/27/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+-------------------------------------
Reporter: jerivas | Owner: timgraham
Type: Bug | Status: assigned
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------
Changes (by timgraham):

* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/4717 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:5>

Django

unread,
May 28, 2015, 10:48:30 AM5/28/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+-------------------------------------
Reporter: jerivas | Owner: timgraham
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"2456276b0250c9f21d580ca6c3f3c86345ad6370" 2456276b]:
{{{
#!CommitTicketReference repository=""
revision="2456276b0250c9f21d580ca6c3f3c86345ad6370"
[1.8.x] Fixed #24851 -- Fixed crash with reverse one-to-one relation in
ModelAdmin.list_display
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:6>

Django

unread,
May 28, 2015, 11:02:46 AM5/28/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+-------------------------------------
Reporter: jerivas | Owner: timgraham
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"ad0f0daf8c6aa80775d68068a76c5b6d0fff04ec" ad0f0daf]:
{{{
#!CommitTicketReference repository=""
revision="ad0f0daf8c6aa80775d68068a76c5b6d0fff04ec"


Fixed #24851 -- Fixed crash with reverse one-to-one relation in
ModelAdmin.list_display

Forwardport of 2456276b0250c9f21d580ca6c3f3c86345ad6370 from stable/1.8.x
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:7>

Django

unread,
Sep 15, 2015, 11:06:17 AM9/15/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+-------------------------------------
Reporter: jerivas | Owner: timgraham
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------

Comment (by hobarrera):

I'm having an extremely similar issue, but when the reverse OneToOne is in
the `readonly` list, rather than `fields`. Is it possible it's the same
issue? The above mentioned test app seems to be gone.

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:8>

Django

unread,
Sep 15, 2015, 11:08:42 AM9/15/15
to django-...@googlegroups.com
#24851: Regression in reverse one-to-one field in list_display
---------------------------------+-------------------------------------
Reporter: jerivas | Owner: timgraham
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------------

Comment (by timgraham):

This has been fixed in Django 1.8.3+ so if you can reproduce your problem
there, please open a new ticket with details. Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/24851#comment:9>

Reply all
Reply to author
Forward
0 new messages