Opened 5 years ago
Closed 5 years ago
#31083 closed New feature (wontfix)
Add select_related support for Site.objects.get_current
Reported by: | Kris Ciccarello | Owned by: | nobody |
---|---|---|---|
Component: | contrib.sites | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Since we cannot extend Site
, a common pattern is to use a OneToOneField
to extend a Site
(i.e. with a SiteDetail
class). When performing request.site = get_current_site(request)
in middleware, it would be nice to be able to efficiently load any extended Site
objects onto request.site
at that time.
Given this class:
from django.contrib.sites.models import Site class SiteDetail(models.Model): site = models.OneToOneField( Site, on_delete=models.CASCADE, primary_key=True, related_name="detail" )
The following does not work:
Site.objects.select_related("detail").get_current(request) > AttributeError: 'QuerySet' object has no attribute 'get_current'
We could add support for something like this:
Site.objects.get_current(request, select_related=["detail"])
Which would allow the user to pass through any custom Site
extensions without having to re-write the code in the Site
manager. Given the use of SITE_CACHE
inside of SiteManager
, it seems like making this addition to its API would be a good step.
Does this make sense, or would another approach be better? Happy to make a PR if this would be considered.
Change History (3)
comment:1 by , 5 years ago
Description: | modified (diff) |
---|
comment:2 by , 5 years ago
Version: | 3.0 → master |
---|
comment:3 by , 5 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Hi Kris.
Thanks for the report.
I'm not quite following the need for this here.
SITE_CACHE
holds the instantiatedSite
objects, which would fetchdetail
the first time it was accessed but then re-use the same fetchedSiteDetail
from memory, rather than re-hitting the database each time after that. (Yes?) So, I can't see there's much optimisation to be had...Beyond that, most uses of the sites framework are behind the scenes so it's not clear that users would have much opportunity to pass the select_related parameter. (Most times we're not calling
get_current_site()
ourselves.)As such, I don't think this is a needed addition. Please follow-up though if I've missed the point. Thanks.