Simplified/centralised version dropdowns population (#1500)

.pre-commit-config.yaml

@@ -5,7 +5,7 @@ exclude: .*migrations\/.*|static\/img\/.*|static\/animations\/.*|static\/js\/boo
 
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v5.0.0
     hooks:
       - id: check-case-conflict
       - id: check-merge-conflict
@@ -15,16 +15,16 @@ repos:
       - id: trailing-whitespace
         exclude: ^core/tests/content/boost_release[a-zA-Z_]+.html
   - repo: https://github.com/ambv/black
-    rev: 23.3.0
+    rev: 24.10.0
     hooks:
       - id: black
   - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: v0.0.272
+    rev: v0.8.1
     hooks:
       - id: ruff
         args: [--fix]
   - repo: https://github.com/rtts/djhtml
-    rev: 3.0.6
+    rev: 3.0.7
     hooks:
       - id: djhtml
         entry: djhtml --tabwidth 2

config/urls.py

@@ -89,6 +89,7 @@
 urlpatterns = (
     [
         path("", HomepageView.as_view(), name="home"),
+        # todo: can we get rid of this?
         path("homepage-beta/", HomepageBetaView.as_view(), name="home-beta"),
         path("admin/", admin.site.urls),
         path("oauth2/", include("oauth2_provider.urls", namespace="oauth2_provider")),

libraries/forms.py

@@ -233,9 +233,9 @@ class CreateReportForm(CreateReportFullForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.fields[
-            "library_1"
-        ].help_text = "If none are selected, all libraries will be selected."
+        self.fields["library_1"].help_text = (
+            "If none are selected, all libraries will be selected."
+        )
 
     @property
     def cache_key(self):

libraries/mixins.py

@@ -1,7 +1,11 @@
+from functools import partial
+
 import structlog
+from django.shortcuts import get_object_or_404
 from django.urls import reverse
 
 from libraries.constants import LATEST_RELEASE_URL_PATH_STR
+from libraries.models import Library
 from versions.models import Version
 
 logger = structlog.get_logger()
@@ -25,24 +29,49 @@ def get_context_data(self, **kwargs):
 
 
 class BoostVersionMixin:
-    def get_context_data(self, **kwargs):
-        context = super().get_context_data(**kwargs)
-        # todo: replace get_current_library_version on LibraryDetail with this +
-        #  prefetch_related
-        context.update(
+    def dispatch(self, request, *args, **kwargs):
+        if not self.extra_context:
+            self.extra_context = {}
+
+        if not self.extra_context.get("current_version"):
+            self.extra_context["current_version"] = Version.objects.most_recent()
+
+        self.extra_context.update(
             {
                 "version_str": self.kwargs.get("version_slug"),
                 "LATEST_RELEASE_URL_PATH_STR": LATEST_RELEASE_URL_PATH_STR,
             }
         )
-        if not context.get("current_version"):
-            context["current_version"] = Version.objects.most_recent()
-
-        if context["version_str"] == LATEST_RELEASE_URL_PATH_STR:
-            context["selected_version"] = context["current_version"]
-        elif context["version_str"]:
-            context["selected_version"] = Version.objects.get(
-                slug=context["version_str"]
+        if self.extra_context["version_str"] == LATEST_RELEASE_URL_PATH_STR:
+            self.extra_context["selected_version"] = self.extra_context[
+                "current_version"
+            ]
+        elif self.extra_context["version_str"]:
+            self.extra_context["selected_version"] = get_object_or_404(
+                Version, slug=self.extra_context["version_str"]
             )
 
-        return context
+        version_path_kwargs = {
+            "release-detail": {},
+            "libraries-list": {
+                "filter_out_has_no_libraries": True,
+                "force_version_inclusion": self.extra_context["current_version"],
+            },
+            "library-detail": {
+                "flag_versions_without_library": partial(
+                    get_object_or_404, Library, slug=self.kwargs.get("library_slug")
+                )
+            },
+        }.get(self.request.resolver_match.view_name, {})
+        # we need this step to process any partials
+        processed_version_path_kwargs = {
+            key: (value() if callable(value) else value)
+            for key, value in version_path_kwargs.items()
+        }
+
+        self.extra_context["versions"] = Version.objects.get_dropdown_versions(
+            **processed_version_path_kwargs
+        )
+        # here we hack extra_context into the request so we can access for cookie checks
+        request.extra_context = self.extra_context
+        return super().dispatch(request, *args, **kwargs)

libraries/tests/test_views.py

@@ -167,17 +167,22 @@ def test_library_detail(library_version, tp):
     tp.response_200(response)
 
 
-def test_library_detail_404(library, tp):
-    """GET /libraries/latest/{bad_library_slug}/"""
+def test_library_detail_404(library, old_version, tp):
+    """GET /library/latest/{bad_library_slug}/"""
     # 404 due to bad slug
     url = tp.reverse("library-detail", "latest", "bananas")
     response = tp.get(url)
     tp.response_404(response)
 
-    # 404 due to no existing version
-    url = tp.reverse("library-detail", "latest", library.slug)
+
+def test_library_detail_missing_version(library, old_version, tp):
+    # custom error due to no existing version
+    url = tp.reverse("library-detail", old_version.display_name, library.slug)
     response = tp.get(url)
-    tp.response_404(response)
+    assert (
+        "There was no version of the Boost.MultiArray library in the 1.70.0 version of "
+        "Boost." in response.content.decode("utf-8")
+    )
 
 
 def test_library_docs_redirect(tp, library, library_version):

libraries/utils.py

@@ -142,7 +142,10 @@ def get_category(request):
 
 
 def determine_selected_boost_version(request_value, request):
-    valid_versions = Version.objects.version_dropdown_strict()
+    # use the versions in the request if they are available otherwise fall back to DB
+    valid_versions = getattr(request, "extra_context", {}).get(
+        "versions", Version.objects.get_dropdown_versions()
+    )
     version_slug = request_value or get_version_from_cookie(request)
     if version_slug in [v.slug for v in valid_versions] + [LATEST_RELEASE_URL_PATH_STR]:
         return version_slug
@@ -153,7 +156,7 @@ def determine_selected_boost_version(request_value, request):
 
 def set_selected_boost_version(version_slug: str, response) -> None:
     """Update the selected version in the cookies."""
-    valid_versions = Version.objects.version_dropdown_strict()
+    valid_versions = Version.objects.get_dropdown_versions()
     if version_slug in [v.slug for v in valid_versions]:
         response.set_cookie(SELECTED_BOOST_VERSION_COOKIE_NAME, version_slug)
     elif version_slug == LATEST_RELEASE_URL_PATH_STR:

libraries/views.py

@@ -98,9 +98,6 @@ def get_queryset(self):
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**self.kwargs)
         context["categories"] = self.get_categories(context["selected_version"])
-        context["versions"] = self.get_versions(
-            current_version=context["current_version"]
-        )
         # todo: add tests for sort order
         if self.kwargs.get("category_slug"):
             context["category"] = Category.objects.get(
@@ -116,27 +113,6 @@ def get_categories(self, version=None):
             .order_by("name")
         )
 
-    def get_versions(self, current_version):
-        """
-        Return a queryset of all versions to display in the version dropdown.
-        """
-        versions = Version.objects.version_dropdown_strict()
-
-        # Annotate each version with the number of libraries it has
-        versions = versions.annotate(
-            library_count=Count("library_version", distinct=True)
-        )
-
-        # Filter out versions with no libraries
-        versions = versions.filter(library_count__gt=0)
-
-        # Confirm the most recent v is in the queryset, even if it has no libraries
-        if current_version and current_version not in versions:
-            versions = versions | Version.objects.filter(pk=current_version.pk)
-
-        versions.prefetch_related("library_version")
-        return versions
-
     def dispatch(self, request, *args, **kwargs):
         """Set the selected version in the cookies."""
         response = super().dispatch(request, *args, **kwargs)
@@ -238,21 +214,23 @@ class LibraryDetail(VersionAlertMixin, BoostVersionMixin, DetailView):
     model = Library
     template_name = "libraries/detail.html"
     redirect_to_docs = False
+    slug_url_kwarg = "library_slug"
 
     def get_context_data(self, **kwargs):
         """Set the form action to the main libraries page"""
         context = super().get_context_data(**kwargs)
-        # Get fields related to Boost versions
-        context["versions"] = (
-            Version.objects.version_dropdown_strict()
-            .filter(library_version__library=self.object)
-            .distinct()
-        )
+        context["library_view_str"] = get_prioritized_library_view(self.request)
+        # Get versions, flag when the current library isn't in each version
         context["LATEST_RELEASE_URL_PATH_NAME"] = LATEST_RELEASE_URL_PATH_STR
-        # Get general data and version-sensitive data
-        library_version = LibraryVersion.objects.get(
-            library=self.get_object(), version=context["selected_version"]
-        )
+        if not self.object:
+            raise Http404("No library found matching the query")
+        try:
+            library_version = LibraryVersion.objects.get(
+                library=self.object, version=context["selected_version"]
+            )
+        except LibraryVersion.DoesNotExist:
+            return context
+
         context["library_version"] = library_version
         context["documentation_url"] = get_documentation_url(
             library_version, context["version_str"] == LATEST_RELEASE_URL_PATH_STR
@@ -305,7 +283,6 @@ def get_context_data(self, **kwargs):
             self.object.get_description(client, tag=context["selected_version"].name)
             or README_MISSING
         )
-        context["library_view_str"] = get_prioritized_library_view(self.request)
         return context
 
     def get_commit_data_by_release(self):
@@ -325,24 +302,6 @@ def get_commit_data_by_release(self):
             for x in reversed(list(qs))
         ]
 
-    def get_object(self):
-        """Get the current library object from the slug in the URL.
-        If present, use the version_slug to get the right LibraryVersion of the library.
-        Otherwise, default to the most recent version."""
-        library_slug = self.kwargs.get("library_slug")
-        version = self.get_version()
-
-        if not LibraryVersion.objects.filter(
-            version=version, library__slug__iexact=library_slug
-        ).exists():
-            raise Http404("No library found matching the query")
-
-        try:
-            obj = self.get_queryset().get(slug__iexact=library_slug)
-        except self.model.DoesNotExist:
-            raise Http404("No library found matching the query")
-        return obj
-
     def get_author_tag(self):
         """Format the authors for the author meta tag in the template."""
         authors = self.object.authors.all()