Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#21386 closed Cleanup/optimization (fixed)

admindocs 'views' section depends on SITE_ID setting

Reported by: Ramiro Morales Owned by: Bouke Haarsma
Component: contrib.admindocs Version: dev
Severity: Normal Keywords:
Cc: Baptiste Mispelon, 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 Baptiste Mispelon)

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 by Baptiste Mispelon, 11 years ago

Cc: Baptiste Mispelon added
Description: modified (diff)
Triage Stage: Unreviewed → 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 by Bouke Haarsma, 11 years ago

Has patch: set
Owner: changed from nobody to Bouke Haarsma
Status: new → assigned

PR: ​https://github.com/django/django/pull/1871

comment:3 by Bouke Haarsma, 11 years ago

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 by Bouke Haarsma, 11 years ago

Cc: bouke@… added

comment:5 by Claude Paroz <claude@…>, 11 years ago

Resolution: → fixed
Status: assigned → 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 by Claude Paroz, 11 years ago

Type: Uncategorized → Cleanup/optimization

comment:7 by sthzg@…, 11 years ago

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 by Bouke Haarsma, 11 years ago

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 by anonymous, 11 years ago

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

comment:10 by Tim Graham <timograham@…>, 11 years ago

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