Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7810 closed (fixed)

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

Reported by: shanx 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 6 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 6 years ago by Simon Greenhill

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by Alex

  • Version changed from newforms-admin to SVN

comment:3 Changed 6 years ago by brosner

  • milestone changed from 1.0 alpha to 1.0 beta

Not critical for 1.0-alpha.

comment:4 Changed 6 years ago by MattBowen

  • Owner changed from nobody to MattBowen
  • Status changed from new to assigned

Changed 6 years ago by MattBowen

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

comment:5 Changed 6 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 6 years ago by MattBowen

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

comment:7 Changed 6 years ago by MattBowen

  • Has patch set

comment:8 Changed 6 years ago by anonymous

  • Triage Stage changed from Accepted to Ready 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 6 years ago by adamfast

Wasn't logged in. Above notes by me.

comment:10 Changed 6 years ago by brosner

  • milestone changed from 1.0 beta to 1.0

comment:11 Changed 6 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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 6 years ago by jacob

  • Owner changed from MattBowen to jacob
  • Status changed from assigned to new

comment:13 Changed 6 years ago by jacob

  • Status changed from new to assigned

comment:14 Changed 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

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

comment:15 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.