Opened 3 years ago

Closed 3 years ago

Last modified 20 months ago

#27018 closed Bug (fixed)

Admin views in admindocs crash with AttributeError

Reported by: Markus Holtermann Owned by: Helen Sherwood-Taylor
Component: contrib.admindocs Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using django.contrib.admindocs the views under the admin namespace are not available. With the exception of /admin/<app_label>/<model>/<var>/ and /admin/r/<content_type_id>/<object_id>/.

Django 1.9.8 returns a HTTP 404 while 1.10 raises an attribute error. It appears that only views that are defined as methods on a class are an issue.

This is likely related to #24931 and #23601.

Change History (13)

comment:1 Changed 3 years ago by Tim Graham

Summary: Admin view in admindocs are not accessibleAdmin views in admindocs crash with AttributeError
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Bisected the change in 1.10 to 31a789f646d0d9af3e8464f2f9a06aa018df5f90.

comment:2 Changed 3 years ago by Helen Sherwood-Taylor

Owner: changed from nobody to Helen Sherwood-Taylor
Status: newassigned

I'll have a look at this.

comment:3 Changed 3 years ago by Helen Sherwood-Taylor

The problem is that the callbacks for these views are methods on classes.

For those two links that do work:
/admin/r/<content_type_id>/<object_id>/ view function is django.contrib.contenttypes.views.shortcut
/admin/<app_label>/<model>/<var>/ view function is django.views.generic.base.RedirectView
and both of those exist and can be resolved in ViewDetailView so they're fine.

However for /admin/ the view function is shown as django.contrib.admin.sites.index which does not exist. It's django.contrib.admin.sites.AdminSite.index (or django.contrib.admin.sites.site.index since the sites module exposes an instance of AdminSite). This means the view function is insufficiently specified on the URL for the view detail.

This is true for all the others that are crashing.

Next step will be to write some failing tests.

comment:4 Changed 3 years ago by Helen Sherwood-Taylor

I'm working on branch https://github.com/helenst/django/tree/ticket_27018

Have written tests for the detail and index views.

comment:5 Changed 3 years ago by Helen Sherwood-Taylor

I've fixed this but it's a Python 3 only fix due to the use of __qualname__ to find out which class the view comes from and therefore generate the correct URL.

There are ways to emulate __qualname__ in py2, but kind of hacky. I see in migrations/serializers.py qualname is also used and a message generated for py2 users. Is a Python 3 only solution acceptable here? In Python 2 it will continue to just not work (and could be tidied up to raise a 404 rather than an error).

I am not sure what is the best thing here, so some input would be good! All is pushed to the branch linked above.

There is also the issue of ensuring the tests pass (i.e. I have added test cases but they only pass for python 3).

comment:6 Changed 3 years ago by Tim Graham

I don't mind if the fix is Python 3 only. Python 2 raising a 404 as happened in older versions of Django would be okay.

comment:7 Changed 3 years ago by Markus Holtermann

Awesome Helen! A Python 3 solution would be fine and more than we have now :)

For the tests I'd either go with

if six.PY2:
    validate_stuff_py2()
else:
    validate_stuff_py3()

or

@unittest.skipIf(six.PY2, "Only supported for Python 3")
def test_foo_py3(self):
    validate_stuff_py3()

@unittest.skipIf(six.PY3, "Only supported for Python 2")
def test_foo_py2(self):
    validate_stuff_py2()

The first might be easier to maintain (for the time where we still have general Python 2 support in Django).

comment:8 Changed 3 years ago by Helen Sherwood-Taylor

PR: https://github.com/django/django/pull/7127

Tests pass on 2.7 and 3.5. I have also added a note in the documentation about this.

comment:9 Changed 3 years ago by Tim Graham

Has patch: set
Patch needs improvement: set

Left some comments for improvement.

comment:10 Changed 3 years ago by Helen Sherwood-Taylor

Patch needs improvement: unset

Hi Tim, thank you for the review! I have made improvements as suggested.

comment:11 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In bc1e2d8e:

Fixed #27018 -- Fixed admindocs crash with a view in a class.

Generated correct admindocs URLs on Python 3. URLs generate 404s on
Python 2, as in older versions of Django.

comment:12 Changed 3 years ago by Tim Graham <timograham@…>

In ae0f55e:

[1.10.x] Fixed #27018 -- Fixed admindocs crash with a view in a class.

Generated correct admindocs URLs on Python 3. URLs generate 404s on
Python 2, as in older versions of Django.

Backport of bc1e2d8e8edde6cc7d2657c68242a13ee65a15b8 from master

comment:13 Changed 20 months ago by Paul Donohue

This seems to have introduced a bug: #29296.

Last edited 20 months ago by Tim Graham (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top