From 1a41a4de3600b55707c0548ed528f31e14e21408 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 16 Oct 2024 12:27:51 -0500 Subject: [PATCH 1/2] Addons: always sort versions in descending order We were mixing ascending and descending when listing the versions, we now always sort in descending order. Even if the version fails to parse, we still sort it in descending order as fallback. Closes https://github.com/readthedocs/readthedocs.org/issues/11689 --- readthedocs/projects/version_handling.py | 7 +- readthedocs/proxito/tests/test_hosting.py | 155 ++++++++++++++++++++++ readthedocs/proxito/views/hosting.py | 2 +- 3 files changed, 160 insertions(+), 4 deletions(-) diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index 3fb61b95f72..b9c7ff39b06 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -187,15 +187,15 @@ def sort_versions_generic( alphabetically_sorted_version_list = sorted( version_list, key=operator.attrgetter("slug"), + reverse=True, ) initial_versions = [] valid_versions = [] invalid_versions = [] - for i, version in enumerate(alphabetically_sorted_version_list): + for version in alphabetically_sorted_version_list: if latest_stable_at_beginning: if version.slug in (STABLE, LATEST): - # It relies on the version list sorted alphabetically first ("l" comes first than "s") initial_versions.append((version, version.slug)) continue @@ -215,7 +215,8 @@ def sort_versions_generic( invalid_versions.append((version, None)) all_versions = ( - initial_versions + # It relies on the version list sorted alphabetically first ("l" comes first than "s") + sorted(initial_versions, key=operator.itemgetter(1)) + sorted(valid_versions, key=operator.itemgetter(1), reverse=True) + invalid_versions ) diff --git a/readthedocs/proxito/tests/test_hosting.py b/readthedocs/proxito/tests/test_hosting.py index 2ec44656332..70de2902d1e 100644 --- a/readthedocs/proxito/tests/test_hosting.py +++ b/readthedocs/proxito/tests/test_hosting.py @@ -13,6 +13,9 @@ from readthedocs.builds.constants import LATEST from readthedocs.builds.models import Build, Version from readthedocs.projects.constants import ( + ADDONS_FLYOUT_SORTING_ALPHABETICALLY, + ADDONS_FLYOUT_SORTING_CALVER, + ADDONS_FLYOUT_SORTING_PYTHON_PACKAGING, MULTIPLE_VERSIONS_WITH_TRANSLATIONS, PRIVATE, PUBLIC, @@ -841,3 +844,155 @@ def test_number_of_queries_url_translations(self): }, ) assert r.status_code == 200 + + def test_version_ordering(self): + for slug in ["1.0", "1.2", "1.12", "2.0", "2020.01.05", "a-slug", "z-slug"]: + fixture.get( + Version, + project=self.project, + privacy_level=PUBLIC, + slug=slug, + verbose_name=slug, + built=True, + active=True, + ) + self.project.update_stable_version() + self.project.versions.update( + privacy_level=PUBLIC, + built=True, + active=True, + ) + + kwargs = { + "path": reverse("proxito_readthedocs_docs_addons"), + "data": { + "url": "https://project.dev.readthedocs.io/en/latest/", + "client-version": "0.6.0", + "api-version": "1.0.0", + }, + "secure": True, + "headers": { + "host": "project.dev.readthedocs.io", + }, + } + + # Default ordering (SemVer) + expected = [ + "latest", + "stable", + "2020.01.05", + "2.0", + "1.12", + "1.2", + "1.0", + "z-slug", + "a-slug", + ] + r = self.client.get(**kwargs) + assert r.status_code == 200 + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + self.project.refresh_from_db() + addons = self.project.addons + + # The order of latest and stable doesn't change when using semver. + addons.flyout_sorting_latest_stable_at_beginning = False + addons.save() + r = self.client.get(**kwargs) + assert r.status_code == 200 + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + addons.flyout_sorting = ADDONS_FLYOUT_SORTING_ALPHABETICALLY + addons.flyout_sorting_latest_stable_at_beginning = True + addons.save() + expected = [ + "z-slug", + "stable", + "latest", + "a-slug", + "2020.01.05", + "2.0", + "1.2", + "1.12", + "1.0", + ] + r = self.client.get(**kwargs) + assert r.status_code == 200 + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + # The order of latest and stable doesn't change when using alphabetical sorting. + addons.flyout_sorting_latest_stable_at_beginning = False + addons.save() + r = self.client.get(**kwargs) + assert r.status_code == 200 + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + addons.flyout_sorting = ADDONS_FLYOUT_SORTING_PYTHON_PACKAGING + addons.flyout_sorting_latest_stable_at_beginning = True + addons.save() + r = self.client.get(**kwargs) + assert r.status_code == 200 + expected = [ + "latest", + "stable", + "2020.01.05", + "2.0", + "1.12", + "1.2", + "1.0", + "z-slug", + "a-slug", + ] + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + addons.flyout_sorting_latest_stable_at_beginning = False + addons.save() + r = self.client.get(**kwargs) + assert r.status_code == 200 + expected = [ + "2020.01.05", + "2.0", + "1.12", + "1.2", + "1.0", + "z-slug", + "stable", + "latest", + "a-slug", + ] + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + addons.flyout_sorting = ADDONS_FLYOUT_SORTING_CALVER + addons.flyout_sorting_latest_stable_at_beginning = True + addons.save() + r = self.client.get(**kwargs) + assert r.status_code == 200 + expected = [ + "latest", + "stable", + "2020.01.05", + "z-slug", + "a-slug", + "2.0", + "1.2", + "1.12", + "1.0", + ] + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected + + addons.flyout_sorting_latest_stable_at_beginning = False + addons.save() + r = self.client.get(**kwargs) + assert r.status_code == 200 + expected = [ + "2020.01.05", + "z-slug", + "stable", + "latest", + "a-slug", + "2.0", + "1.2", + "1.12", + "1.0", + ] + assert [v["slug"] for v in r.json()["versions"]["active"]] == expected diff --git a/readthedocs/proxito/views/hosting.py b/readthedocs/proxito/views/hosting.py index 06abc2bacb6..1204b7f9c9c 100644 --- a/readthedocs/proxito/views/hosting.py +++ b/readthedocs/proxito/views/hosting.py @@ -336,7 +336,7 @@ def _v1(self, project, version, build, filename, url, request): versions_active_built_not_hidden = ( self._get_versions(request, project) .select_related("project") - .order_by("slug") + .order_by("-slug") ) sorted_versions_active_built_not_hidden = versions_active_built_not_hidden From a087727212c5a62bb4c048877b34637f5a3fca31 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 16 Oct 2024 13:47:33 -0500 Subject: [PATCH 2/2] Fix tests --- .../projects/tests/test_version_handling.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/readthedocs/projects/tests/test_version_handling.py b/readthedocs/projects/tests/test_version_handling.py index a83d24e47b2..0f70ef16376 100644 --- a/readthedocs/projects/tests/test_version_handling.py +++ b/readthedocs/projects/tests/test_version_handling.py @@ -40,12 +40,12 @@ def test_sort_versions_python_packaging(self): # `latest` and `stable` are at the beginning "latest", "2.5.3", - "1.1", "1.1.0", + "1.1", "v1.0", # Invalid versions are at the end sorted alphabetically. - "another-invalid", "invalid", + "another-invalid", ] for slug in slugs: @@ -73,13 +73,13 @@ def test_sort_versions_python_packaging_latest_stable_not_at_beginning(self): expected = [ "2.5.3", - "1.1", "1.1.0", + "1.1", "v1.0", # Invalid versions are at the end sorted alphabetically. - "another-invalid", - "invalid", "latest", + "invalid", + "another-invalid", ] for slug in slugs: @@ -121,14 +121,14 @@ def test_sort_versions_calver(self): "2022.01.22", "2021.01.22", # invalid ones (alphabetically) - "1.1", - "1.1.0", - "2.5.3", - "2001-02-27", - "2001.02.2", - "2001.16.32", - "another-invalid", "invalid", + "another-invalid", + "2001.16.32", + "2001.02.2", + "2001-02-27", + "2.5.3", + "1.1.0", + "1.1", ] for slug in slugs: @@ -175,13 +175,13 @@ def test_sort_versions_custom_pattern(self): "v1.1", "v1.0", # invalid ones (alphabetically) - "1.1", - "2.5.3", - "2022.01.22", - "another-invalid", - "invalid", - "v1.1.0", "v2.3rc1", + "v1.1.0", + "invalid", + "another-invalid", + "2022.01.22", + "2.5.3", + "1.1", ] for slug in slugs: