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 Kris Ciccarello)

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 Kris Ciccarello, 5 years ago

Description: modified (diff)

comment:2 by Kris Ciccarello, 5 years ago

Version: 3.0master

comment:3 by Carlton Gibson, 5 years ago

Resolution: wontfix
Status: newclosed

Hi Kris.

Thanks for the report.

Given the use of SITE_CACHE

I'm not quite following the need for this here. SITE_CACHE holds the instantiated Site objects, which would fetch detail the first time it was accessed but then re-use the same fetched SiteDetail 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.

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