Addons: always sort versions in descending order (#11691)

stsewd · web-flow · commit 9451d9cf73f8 · 2024-10-21T15:15:38.000-05:00
* 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 #11689 * Fix tests
diff --git 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:
diff --git 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
@@ -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
@@ -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
 

Original file line number	Diff line number	Diff line change
`@@ -336,7 +336,7 @@ def _v1(self, project, version, build, filename, url, request):`
`336`	`336`	`versions_active_built_not_hidden = (`
`337`	`337`	`self._get_versions(request, project)`
`338`	`338`	`.select_related("project")`
`339`		`- .order_by("slug")`
	`339`	`+ .order_by("-slug")`
`340`	`340`	`)`
`341`	`341`	`sorted_versions_active_built_not_hidden = versions_active_built_not_hidden`
`342`	`342`