#10235 closed (fixed)
contrib.sitemaps doesn't support RequestSite
Reported by: | Arthur Koziel | Owned by: | nobody |
---|---|---|---|
Component: | Contrib apps | Version: | 1.0 |
Severity: | Keywords: | sitemap | |
Cc: | miracle2k | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The contrib.sitemaps app will always query the database for a Site object. It doesn't support RequestSite objects (http://docs.djangoproject.com/en/dev/ref/contrib/sites/#requestsite-objects).
The attached patch solves this issue. However, it is pretty hard to test this because the runtests.py has 'django.contrib.sites' in ALWAYS_INSTALLED_APPS, so checking if the app is installed with:
if Site._meta.installed: current_site = Site.objects.get_current() else: current_site = RequestSite(request)
won't work because the tests will always assume that the app is installed and query the database.
Attachments (2)
Change History (10)
by , 16 years ago
Attachment: | sitemaps_requestsite.diff added |
---|
comment:1 by , 16 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 16 years ago
I've revised the patch and added a helper function like you suggested.
Having the request as an attribute on the class would be cleaner than setting site.request in the view but not possible as Sitemaps can be callables as well as objects:
sitemaps = { 'foobar': FoobarSitemap, 'blog': GenericSitemap(info_dict, priority=0.6), }
FlatPageSitemap doesn't support RequestSite because the flatpages app requires the sites framework and RequestSite isn't a subclass of models.Model so flatpage_set will not be available.
The ping_google function also doesn't use RequestSite because it's impossible to get a request from the CLI. The only possibility I can think of is to specify the full sitemap url as an argument to the ping_google command e.g. "./manage.py ping_google http://foobar.com/sitemap.xml" but according to the docs this argument is already reserved for the absolute sitemap url (e.g. "/sitemap.xml") http://docs.djangoproject.com/en/dev/ref/contrib/sitemaps/#pinging-google-via-manage-py
by , 16 years ago
Attachment: | sitemaps_requestsite_2.diff added |
---|
comment:4 by , 15 years ago
Cc: | added |
---|
comment:5 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [13980]) Fixed #14386, #8960, #10235, #10909, #10608, #13845, #14377 - standardize Site/RequestSite usage in various places.
Many thanks to gabrielhurley for putting most of this together. Also to
bmihelac, arthurk, qingfeng, hvendelbo, petr.pulc@…, Hraban for
reports and some initial patches.
The patch also contains some whitespace/PEP8 fixes.
comment:6 by , 14 years ago
(In [13987]) [1.2.X] Fixed #14386, #8960, #10235, #10909, #10608, #13845, #14377 - standardize Site/RequestSite usage in various places.
Many thanks to gabrielhurley for putting most of this together. Also to
bmihelac, arthurk, qingfeng, hvendelbo, petr.pulc@…, Hraban for
reports and some initial patches.
The patch also contains some whitespace/PEP8 fixes.
Backport of [13980] from trunk. Some items could be considered features
(i.e. supporting RequestSite in various contrib apps), others are definite
bugs, but it was much more robust to backport all these tightly related
changes together.
I agree that contrib.sitemaps should support
RequestSite
-- but before we do that, maybe we should make a helper function that does the following logic (which is repeated multiple times in this patch):