From 8a236463459ca5e0a36fcffca87526b7421f8e90 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Thu, 23 May 2019 18:15:08 +0600 Subject: [PATCH 1/7] Sitemap sort order priorities updated --- readthedocs/core/views/serve.py | 4 +-- readthedocs/projects/version_handling.py | 10 +++--- .../rtd_tests/tests/test_doc_serving.py | 33 ++++++++++++++++++- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 390063f256a..9f9775fca77 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -369,14 +369,14 @@ def changefreqs_generator(): """ Generator returning ``changefreq`` needed by sitemap.xml. - It returns ``daily`` on first iteration, then ``weekly`` and then it + It returns ``weekly`` on first iteration, then ``daily`` and then it will return always ``monthly``. We are using ``monthly`` as last value because ``never`` is too aggressive. If the tag is removed and a branch is created with the same name, we will want bots to revisit this. """ - changefreqs = ['daily', 'weekly'] + changefreqs = ['weekly', 'daily'] yield from itertools.chain(changefreqs, itertools.repeat('monthly')) if project.privacy_level == constants.PRIVATE: diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index 7a730e61fd0..093bb705c68 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -43,23 +43,23 @@ def comparable_version(version_string): """ Can be used as ``key`` argument to ``sorted``. - The ``LATEST`` version shall always beat other versions in comparison. - ``STABLE`` should be listed second. If we cannot figure out the version + The ``STABLE`` version shall always beat other versions in comparison. + ``LATEST`` should be listed second. If we cannot figure out the version number then we sort it to the bottom of the list. :param version_string: version as string object (e.g. '3.10.1' or 'latest') :type version_string: str or unicode - :returns: a comparable version object (e.g. 'latest' -> Version('99999.0')) + :returns: a comparable version object (e.g. 'latest' -> Version('9999.0')) :rtype: packaging.version.Version """ comparable = parse_version_failsafe(version_string) if not comparable: if version_string == LATEST_VERBOSE_NAME: - comparable = Version('99999.0') - elif version_string == STABLE_VERBOSE_NAME: comparable = Version('9999.0') + elif version_string == STABLE_VERBOSE_NAME: + comparable = Version('99999.0') else: comparable = Version('0.01') return comparable diff --git a/readthedocs/rtd_tests/tests/test_doc_serving.py b/readthedocs/rtd_tests/tests/test_doc_serving.py index 134b191235c..58581cb27ae 100644 --- a/readthedocs/rtd_tests/tests/test_doc_serving.py +++ b/readthedocs/rtd_tests/tests/test_doc_serving.py @@ -240,6 +240,15 @@ def test_sitemap_xml(self): project=self.public, active=True ) + stable_version = fixture.get( + Version, + identifier='stable', + verbose_name='stable', + slug='stable', + privacy_level=constants.PUBLIC, + project=self.public, + active=True + ) # This also creates a Version `latest` Automatically for this project translation = fixture.get( Project, @@ -269,7 +278,7 @@ def test_sitemap_xml(self): ), ) - # stable is marked as PRIVATE and should not appear here + # PRIVATE version should not appear here self.assertNotContains( response, self.public.get_docs_url( @@ -293,3 +302,25 @@ def test_sitemap_xml(self): # hreflang should use hyphen instead of underscore # in language and country value. (zh_CN should be zh-CN) self.assertContains(response, 'zh-CN') + + # Check if STABLE version has 'priority of 1 and changefreq of weekly. + self.assertEqual( + response.context['versions'][0]['loc'], + self.public.get_docs_url( + version_slug=stable_version.slug, + lang_slug=self.public.language, + private=False, + ),) + self.assertEqual(response.context['versions'][0]['priority'], 1) + self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly') + + # Check if LATEST version has priority of 0.9 and changefreq of daily. + self.assertEqual( + response.context['versions'][1]['loc'], + self.public.get_docs_url( + version_slug='latest', + lang_slug=self.public.language, + private=False, + ),) + self.assertEqual(response.context['versions'][1]['priority'], 0.9) + self.assertEqual(response.context['versions'][1]['changefreq'], 'daily') From f2f392b4abbe325e4935f5063f16311caf211dc5 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Sat, 25 May 2019 17:33:14 +0600 Subject: [PATCH 2/7] ordering fix --- readthedocs/core/views/serve.py | 4 ++-- readthedocs/projects/version_handling.py | 10 +++++----- readthedocs/rtd_tests/tests/test_doc_serving.py | 12 ++++++------ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 9f9775fca77..485efba167d 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -351,7 +351,7 @@ def priorities_generator(): It generates values from 1 to 0.1 by decreasing in 0.1 on each iteration. After 0.1 is reached, it will keep returning 0.1. """ - priorities = [1, 0.9, 0.8, 0.7, 0.6, 0.5, 0.4, 0.3, 0.2] + priorities = [0.9, 1, 0.8, 0.7, 0.6, 0.5, 0.4, 0.3, 0.2] yield from itertools.chain(priorities, itertools.repeat(0.1)) def hreflang_formatter(lang): @@ -376,7 +376,7 @@ def changefreqs_generator(): aggressive. If the tag is removed and a branch is created with the same name, we will want bots to revisit this. """ - changefreqs = ['weekly', 'daily'] + changefreqs = ['daily', 'weekly'] yield from itertools.chain(changefreqs, itertools.repeat('monthly')) if project.privacy_level == constants.PRIVATE: diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index 093bb705c68..7a730e61fd0 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -43,23 +43,23 @@ def comparable_version(version_string): """ Can be used as ``key`` argument to ``sorted``. - The ``STABLE`` version shall always beat other versions in comparison. - ``LATEST`` should be listed second. If we cannot figure out the version + The ``LATEST`` version shall always beat other versions in comparison. + ``STABLE`` should be listed second. If we cannot figure out the version number then we sort it to the bottom of the list. :param version_string: version as string object (e.g. '3.10.1' or 'latest') :type version_string: str or unicode - :returns: a comparable version object (e.g. 'latest' -> Version('9999.0')) + :returns: a comparable version object (e.g. 'latest' -> Version('99999.0')) :rtype: packaging.version.Version """ comparable = parse_version_failsafe(version_string) if not comparable: if version_string == LATEST_VERBOSE_NAME: - comparable = Version('9999.0') - elif version_string == STABLE_VERBOSE_NAME: comparable = Version('99999.0') + elif version_string == STABLE_VERBOSE_NAME: + comparable = Version('9999.0') else: comparable = Version('0.01') return comparable diff --git a/readthedocs/rtd_tests/tests/test_doc_serving.py b/readthedocs/rtd_tests/tests/test_doc_serving.py index 58581cb27ae..f3c76d57bd6 100644 --- a/readthedocs/rtd_tests/tests/test_doc_serving.py +++ b/readthedocs/rtd_tests/tests/test_doc_serving.py @@ -305,22 +305,22 @@ def test_sitemap_xml(self): # Check if STABLE version has 'priority of 1 and changefreq of weekly. self.assertEqual( - response.context['versions'][0]['loc'], + response.context['versions'][1]['loc'], self.public.get_docs_url( version_slug=stable_version.slug, lang_slug=self.public.language, private=False, ),) - self.assertEqual(response.context['versions'][0]['priority'], 1) - self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly') + self.assertEqual(response.context['versions'][1]['priority'], 1) + self.assertEqual(response.context['versions'][1]['changefreq'], 'weekly') # Check if LATEST version has priority of 0.9 and changefreq of daily. self.assertEqual( - response.context['versions'][1]['loc'], + response.context['versions'][0]['loc'], self.public.get_docs_url( version_slug='latest', lang_slug=self.public.language, private=False, ),) - self.assertEqual(response.context['versions'][1]['priority'], 0.9) - self.assertEqual(response.context['versions'][1]['changefreq'], 'daily') + self.assertEqual(response.context['versions'][0]['priority'], 0.9) + self.assertEqual(response.context['versions'][0]['changefreq'], 'daily') From 65bfd9b1c4d4c18c0e65c3afa28805d3be50ba98 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Sat, 25 May 2019 17:34:46 +0600 Subject: [PATCH 3/7] doc string fix --- readthedocs/core/views/serve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 485efba167d..1b3cd777c0e 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -369,7 +369,7 @@ def changefreqs_generator(): """ Generator returning ``changefreq`` needed by sitemap.xml. - It returns ``weekly`` on first iteration, then ``daily`` and then it + It returns ``daily`` on first iteration, then ``weekly`` and then it will return always ``monthly``. We are using ``monthly`` as last value because ``never`` is too From 6e7a87eb22fb6a16fe9941b4fdf050e4f160fe0d Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 29 May 2019 17:04:04 +0600 Subject: [PATCH 4/7] sort_by_priority function added to sitemap --- readthedocs/core/views/serve.py | 10 +++++++++- readthedocs/rtd_tests/tests/test_doc_serving.py | 12 ++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 1b3cd777c0e..212ff3fdb89 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -379,6 +379,14 @@ def changefreqs_generator(): changefreqs = ['daily', 'weekly'] yield from itertools.chain(changefreqs, itertools.repeat('monthly')) + def sort_by_priority(version_list): + """This will sort the versions by priority""" + return sorted( + version_list, + key=lambda version: version['priority'], + reverse=True + ) + if project.privacy_level == constants.PRIVATE: raise Http404 @@ -431,7 +439,7 @@ def changefreqs_generator(): versions.append(element) context = { - 'versions': versions, + 'versions': sort_by_priority(versions), } return render( request, diff --git a/readthedocs/rtd_tests/tests/test_doc_serving.py b/readthedocs/rtd_tests/tests/test_doc_serving.py index f3c76d57bd6..58581cb27ae 100644 --- a/readthedocs/rtd_tests/tests/test_doc_serving.py +++ b/readthedocs/rtd_tests/tests/test_doc_serving.py @@ -305,22 +305,22 @@ def test_sitemap_xml(self): # Check if STABLE version has 'priority of 1 and changefreq of weekly. self.assertEqual( - response.context['versions'][1]['loc'], + response.context['versions'][0]['loc'], self.public.get_docs_url( version_slug=stable_version.slug, lang_slug=self.public.language, private=False, ),) - self.assertEqual(response.context['versions'][1]['priority'], 1) - self.assertEqual(response.context['versions'][1]['changefreq'], 'weekly') + self.assertEqual(response.context['versions'][0]['priority'], 1) + self.assertEqual(response.context['versions'][0]['changefreq'], 'weekly') # Check if LATEST version has priority of 0.9 and changefreq of daily. self.assertEqual( - response.context['versions'][0]['loc'], + response.context['versions'][1]['loc'], self.public.get_docs_url( version_slug='latest', lang_slug=self.public.language, private=False, ),) - self.assertEqual(response.context['versions'][0]['priority'], 0.9) - self.assertEqual(response.context['versions'][0]['changefreq'], 'daily') + self.assertEqual(response.context['versions'][1]['priority'], 0.9) + self.assertEqual(response.context['versions'][1]['changefreq'], 'daily') From 6fecc6b92aaf5f1c15f65f458f8cc32fc68de400 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 29 May 2019 17:14:19 +0600 Subject: [PATCH 5/7] lint fix --- readthedocs/core/views/serve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 212ff3fdb89..6034242b60a 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -380,7 +380,7 @@ def changefreqs_generator(): yield from itertools.chain(changefreqs, itertools.repeat('monthly')) def sort_by_priority(version_list): - """This will sort the versions by priority""" + """Sorts the versions by priority. i.e: 1, 0.9, 0.8...""" return sorted( version_list, key=lambda version: version['priority'], From 55f6f3ea739175130203a4a4281aa64c845265e4 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 12 Jun 2019 00:33:08 +0600 Subject: [PATCH 6/7] Sorting by swapping positions --- readthedocs/core/views/serve.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 6034242b60a..3a15fd4aab6 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -38,6 +38,7 @@ from django.views.decorators.cache import cache_page from django.views.static import serve +from readthedocs.builds.constants import LATEST, STABLE from readthedocs.builds.models import Version from readthedocs.core.permissions import AdminPermission from readthedocs.core.resolver import resolve, resolve_path @@ -351,7 +352,7 @@ def priorities_generator(): It generates values from 1 to 0.1 by decreasing in 0.1 on each iteration. After 0.1 is reached, it will keep returning 0.1. """ - priorities = [0.9, 1, 0.8, 0.7, 0.6, 0.5, 0.4, 0.3, 0.2] + priorities = [1, 0.9, 0.8, 0.7, 0.6, 0.5, 0.4, 0.3, 0.2] yield from itertools.chain(priorities, itertools.repeat(0.1)) def hreflang_formatter(lang): @@ -369,24 +370,16 @@ def changefreqs_generator(): """ Generator returning ``changefreq`` needed by sitemap.xml. - It returns ``daily`` on first iteration, then ``weekly`` and then it + It returns ``weekly`` on first iteration, then ``daily`` and then it will return always ``monthly``. We are using ``monthly`` as last value because ``never`` is too aggressive. If the tag is removed and a branch is created with the same name, we will want bots to revisit this. """ - changefreqs = ['daily', 'weekly'] + changefreqs = ['weekly', 'daily'] yield from itertools.chain(changefreqs, itertools.repeat('monthly')) - def sort_by_priority(version_list): - """Sorts the versions by priority. i.e: 1, 0.9, 0.8...""" - return sorted( - version_list, - key=lambda version: version['priority'], - reverse=True - ) - if project.privacy_level == constants.PRIVATE: raise Http404 @@ -396,6 +389,18 @@ def sort_by_priority(version_list): only_active=True, ), ) + + # This is a hack to swap the latest version with + # stable version to get the stable version first in the sitemap. + # We want stable with priority=1 and changefreq='weekly' and + # latest with priority=0.9 and changefreq='daily' + # More details on this: https://github.com/rtfd/readthedocs.org/issues/5447 + if ( + sorted_versions[0].slug == LATEST and + sorted_versions[1].slug == STABLE + ): + sorted_versions[0], sorted_versions[1] = sorted_versions[1], sorted_versions[0] + versions = [] for version, priority, changefreq in zip( sorted_versions, @@ -439,7 +444,7 @@ def sort_by_priority(version_list): versions.append(element) context = { - 'versions': sort_by_priority(versions), + 'versions': versions, } return render( request, From 6175b66d08e1e7fed8372edc05194a03aefdc8d8 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 12 Jun 2019 17:30:50 +0600 Subject: [PATCH 7/7] index out of range issue fix --- readthedocs/core/views/serve.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index 3a15fd4aab6..6940bb78615 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -396,6 +396,7 @@ def changefreqs_generator(): # latest with priority=0.9 and changefreq='daily' # More details on this: https://github.com/rtfd/readthedocs.org/issues/5447 if ( + len(sorted_versions) >= 2 and sorted_versions[0].slug == LATEST and sorted_versions[1].slug == STABLE ):