[Django] #31083: Add select_related support for Site.objects.get_current
Django
-------------------------------------------+------------------------
Reporter: Kris Ciccarello | Owner: nobody
Type: New feature | Status: new
Component: contrib.sites | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31083>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Kris Ciccarello | Owner: nobody
Type: New feature | Status: new
Component: contrib.sites | Version: 3.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Kris Ciccarello:
Old description:
> 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.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31083#comment:1>
Django
Reporter: Kris Ciccarello | Owner: nobody
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kris Ciccarello):
* version: 3.0 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/31083#comment:2>
Django
Reporter: Kris Ciccarello | Owner: nobody
Component: contrib.sites | Version: master
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31083#comment:3>