Opened 22 months ago

Closed 21 months ago

Last modified 17 months 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?

Change History (10)

comment:1 Changed 22 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 22 months ago by bouke

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

comment:3 Changed 22 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 22 months ago by bouke

  • Cc bouke@… added

comment:5 Changed 21 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 21 months ago by claudep

  • Type changed from Uncategorized to Cleanup/optimization

comment:7 Changed 20 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 20 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 20 months ago by anonymous

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

comment:10 Changed 17 months ago by Tim Graham <timograham@…>

In cb47969cd7271749393998a21bc8c467088d6235:

Removed warning for settings.ADMIN_FOR which has been removed.

refs #21386.

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