Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#7810 closed (fixed)

Url to contrib.admindocs is still hardcoded in base template of contrib.admin

Reported by: Remco Wendt Owned by: Jacob
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

In revision 7947 of newforms-admin, the base template of contrib.admin app still hardlinks to the admindocs with:

<a href="{{ root_path }}doc/">{% trans 'Documentation' %}</a>

This behaviour is incorrect, since users now decide for themselves where the admindocs recide. The hard coding of the url effectively forces a user to add:

    (r'^admin/doc/', include('django.contrib.admindocs.urls')),

Above their admin app mapping in urls.py. In the current situation changing the url to for example be:

    (r'^admin/documentation/', include('django.contrib.admindocs.urls')),

In #6962 it was already mentioned to use a named url, since there is coupling between the admin and admindocs app anyway.

Attachments (1)

named-urls-admindocs-7810.diff (2.8 KB) - added by MattBowen 8 years ago.
adds names for the admindocs urls and uses the named URL in admin's base.html

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by Simon Greenhill

Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by Alex Gaynor

Version: newforms-adminSVN

comment:3 Changed 8 years ago by Brian Rosner

milestone: 1.0 alpha1.0 beta

Not critical for 1.0-alpha.

comment:4 Changed 8 years ago by MattBowen

Owner: changed from nobody to MattBowen
Status: newassigned

Changed 8 years ago by MattBowen

adds names for the admindocs urls and uses the named URL in admin's base.html

comment:5 Changed 8 years ago by MattBowen

I used the namespace django-admindocs-* for all the named pattens; it's a little long, but it will likely be unique for applications (unless someone is looking for trouble), and it probably won't need to be typed often anyway. This change has the interesting side effect of making the Documentation url simple point to the admin's root if the admindocs URLs are not loaded. This gets rid of the 404, but is arguably a more confusing behavior.

comment:6 Changed 8 years ago by MattBowen

It looks like I missed some spots in bookmarkets, so I'm revising my patch.

comment:7 Changed 8 years ago by MattBowen

Has patch: set

comment:8 Changed 8 years ago by anonymous

Triage Stage: AcceptedReady for checkin

Ran this against rev. 8229, changed the admin doc urls, and this does what it's supposed to. Marking ready for checkin.

comment:9 Changed 8 years ago by adamfast

Wasn't logged in. Above notes by me.

comment:10 Changed 8 years ago by Brian Rosner

milestone: 1.0 beta1.0

comment:11 Changed 8 years ago by Malcolm Tredinnick

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I have some Secret Inside Knowledge(tm) here. Matt's patch was done at the DC sprint on August 1 and after he attached it, we realised there was one problematic case: bookmarklets. They assume there is a single admin site (no longer necessarily true) and seem to have to make some assumptions about the URL of that site.

The patch here is a good start, but it doesn't quite solve all the problems (see comment:6). We need to make a decision about bookmarklets -- carve them off to somewhere else,? Require configuration? Inventive third option we didn't think of yet? But current patch only shuffles the bugs until that is worked out.

(Remember, campers, tickets aren't the place to do design work, so resist the urge to post random suggestions on that item in the comments. We have a mailing list for that.)

comment:12 Changed 8 years ago by Jacob

Owner: changed from MattBowen to Jacob
Status: assignednew

comment:13 Changed 8 years ago by Jacob

Status: newassigned

comment:14 Changed 8 years ago by Jacob

Resolution: fixed
Status: assignedclosed

(In [8717]) Fixed #7810: added named URLs for admin docs, and use them in the admin base template. Thanks, MattBowen.

comment:15 Changed 5 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top