Fixed #25989 -- Find latest lastmod of all sitemaps #5877
Conversation
| if hasattr(site, 'latest_lastmod'): | ||
| # if latest_lastmod is defined for site, set header so as | ||
| if lastmod is not None: | ||
| # if lastmod is defined for at least one site, set header so as |
There was a problem hiding this comment.
Shouldn't we only set 'Last-Modified' to the most recent lastmod if all sites define one and don't set anything if at least one site doesn't define one?
There was a problem hiding this comment.
I have to agree with you here. The code is rewritten to explicitly require having lastmod defined for all sitemaps.
|
Tests added. |
Determine latest lastmod of all sitemaps supplied to contrib.sitemaps.views.sitemap, instead of unconditionally picking lastmod of a last sitemap. All sitemaps are required to have a lastmod.
|
Fixed python35 |
| @@ -110,6 +110,38 @@ def test_sitemap_last_modified_mixed(self): | |||
| response = self.client.get('/lastmod-mixed/sitemap.xml') | |||
| self.assertFalse(response.has_header('Last-Modified')) | |||
|
|
|||
| def test_sitemaps_lastmod_mixed_ascending_last_modified_missing(self): | |||
| """ | |||
| Tests that Last-Modified header is omitted when lastmod not found in | |||
There was a problem hiding this comment.
Why would the Last-Modified header not be set when it is not found in ALL maps? shouldn't it be set as soon as one is found.. and only NOT set if no last_mod is found.
There was a problem hiding this comment.
Docs don't explicitly define this behaviour. If we stretch lastmod calculation logic of a Sitemap class to this case, all sitemaps should have a lasmod.
There was a problem hiding this comment.
Ah yes.. I see what you mean.
|
merged in 002a4f7, thanks! |
Determine latest lastmod of all sitemaps supplied to
contrib.sitemaps.views.sitemap, instead of unconditionally picking
lastmod of a last sitemap.