Opened 3 years ago

Closed 3 years ago

Last modified 3 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: master
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 Changed 3 years ago by Baptiste Mispelon

Cc: Baptiste Mispelon added
Description: modified (diff)
Triage Stage: UnreviewedAccepted

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

Has patch: set
Owner: changed from nobody to Bouke Haarsma
Status: newassigned

comment:3 Changed 3 years ago by Bouke Haarsma

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

Cc: bouke@… added

comment:5 Changed 3 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

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 3 years ago by Claude Paroz

Type: UncategorizedCleanup/optimization

comment:7 Changed 3 years 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 years ago by Bouke Haarsma

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

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

comment:10 Changed 3 years 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