#17005 closed New feature (fixed)
Add CurrentSiteMiddleware to set the current site on the request
Reported by: | Owned by: | chrismedrela | |
---|---|---|---|
Component: | contrib.sites | Version: | |
Severity: | Normal | Keywords: | |
Cc: | krzysiumed@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I end up doing something like the following all over my code:
from django.contrib.sites.models import Site def my_view(request, template_name="foo.html"): site = Site.objects.get_current() ... return render(request, template_name, locals())
Seems like it would save time (and possibly reduce database requests, depending on where else in the application the Site object is being used) if django.contrib.sites.middleware.SiteMiddleware
was added, which either set request.site
to Site.objects.get_current()
or added site
or current_site
to the RequestContext
I just did a search for "Site.objects.get_current" in one of my web apps and came up with 26 hits. There's really no reason to have to write out that code so many times. It should just be present as a variable somewhere.
Attachments (1)
Change History (16)
comment:1 Changed 12 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 Changed 12 years ago by
I'd be happy to be involved with the coding of it.
It's similar to the static and media context processors."
So do you think it should be made available via a context processor, or as middleware?
comment:2 Changed 12 years ago by
I'd be happy to be involved with the coding of it.
It's similar to the static and media context processors."
So do you think it should be made available via a context processor, or as middleware?
comment:3 Changed 12 years ago by
A middleware is more appropriate — I expect the current site object to be used in the view rather than in the template.
The comparison with the static and media context processors was just about the triviality of the code.
comment:4 Changed 12 years ago by
Owner: | changed from nobody to anonymous |
---|---|
Status: | new → assigned |
comment:5 Changed 12 years ago by
Owner: | changed from anonymous to Christopher Medrela |
---|---|
Status: | assigned → new |
comment:6 Changed 12 years ago by
Status: | new → assigned |
---|
Changed 12 years ago by
Attachment: | 17005.diff added |
---|
comment:7 Changed 12 years ago by
I'd written a patch. It contains docs and tests (I believe they are not necessary, because the middleware is very simple). Sorry for my poor English in docs.
comment:8 Changed 12 years ago by
Has patch: | set |
---|
comment:9 Changed 12 years ago by
Cc: | krzysiumed@… added |
---|
comment:10 Changed 11 years ago by
#13559 is about adding a context processor for the same purpose in templates.
comment:11 Changed 10 years ago by
Owner: | changed from Christopher Medrela to chrismedrela |
---|
I've updated the patch: https://github.com/django/django/pull/1939. BTW, krzysiumed is my old nick.
comment:12 Changed 10 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
The code looks good to me. Can someone review the docs?
comment:13 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:14 Changed 10 years ago by
Summary: | Adding django.contrib.site.middleware ? → Add CurrentSiteMiddleware to set the current site on the request |
---|
I think the cleanest solution is to set
request.site = Site.objects.get_current()
. I wouldn't useget_current_site()
because this middleware doesn't make sense when thesites
app isn't enabled. The current site is cached, so the performance impact is negligible.This middleware will be trivial, but it could be useful, and I don't see any reason not to include it in Django. It's similar to the
static
andmedia
context processors.