Code

Opened 6 months ago

Closed 4 months ago

Last modified 4 weeks ago

#21386 closed Cleanup/optimization (fixed)

admindocs 'views' section depends on SITE_ID setting

Reported by: ramiro Owned by: bouke
Component: contrib.admindocs Version: master
Severity: Normal Keywords:
Cc: bmispelon, bouke@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by bmispelon)

It seems the admindocs views try to detect if th sited contrib app is installed and avoid to use its features if it's not.

But in a couple of places it uses settings.SITE_ID and the new 1.6 default project template settings.py doesn't include it because the sites framework isn't installed.

Perhaps it could be replaced by 'django.contrib.sites.models.get_current_site(request)` as suggested by https://docs.djangoproject.com/en/1.6/ref/contrib/sites/#hooking-into-the-current-site-from-views?

Attachments (0)

Change History (10)

comment:1 Changed 6 months ago by bmispelon

  • Cc bmispelon added
  • Description modified (diff)
  • Triage Stage changed from Unreviewed to Accepted

Yes.

This would also allow us to get rid of admindocs.views.GenericSite (it seems it was used because sites.models.RequestSite hadn't been implemented yet).

comment:2 Changed 6 months ago by bouke

  • Has patch set
  • Owner changed from nobody to bouke
  • Status changed from new to assigned

comment:3 Changed 6 months ago by bouke

Hooking into get_current_site is not straight-forward as admindocs can inspect multiple settings modules, and each can have their own SITE_ID setting.

comment:4 Changed 6 months ago by bouke

  • Cc bouke@… added

comment:5 Changed 4 months ago by Claude Paroz <claude@…>

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

In a39d672ec7d53637805a61b45a51bc0e7d297a36:

Fixed #21386 -- Removed admindocs dependence on sites framework

  • Removed ADMIN_FOR setting and warn warning
  • Group view functions by namespace instead of site
  • Added a test verifying namespaces are listed

Thanks to Claude Paroz for reviewing and ideas for improvement.

comment:6 Changed 4 months ago by claudep

  • Type changed from Uncategorized to Cleanup/optimization

comment:7 Changed 3 months ago by sthzg@…

Is there a chance to backport this fix to one of the next 1.6.x minor updates? This would leave existing 1.6 installations that don't need the sitesframework completely supported and would allow for an easy update path to solve the problem.

comment:8 Changed 3 months ago by bouke

By including SITE_ID=None in your settings, you should not encounter any issues on 1.6. It would be good to provide a fix for 1.6, but backporting the fix of 1.7 would introduce changes in the featureset. It might be better to include installation instructions for admindocs noting that SITE_ID should be configured.

comment:9 Changed 3 months ago by anonymous

That works perfectly on my setup. Thanks for the hint.

comment:10 Changed 4 weeks ago by Tim Graham <timograham@…>

In cb47969cd7271749393998a21bc8c467088d6235:

Removed warning for settings.ADMIN_FOR which has been removed.

refs #21386.

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.