Make sure None value for score are always ordered last #95

Signed-off-by: tdruez <tdruez@nexb.com>

component_catalog/filters.py

@@ -8,6 +8,7 @@
 
 from django import forms
 from django.contrib.admin.options import IncorrectLookupParameters
+from django.db.models import F
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
 
@@ -284,6 +285,30 @@ def show_last_modified_date(self):
         return not self.sort_value or self.has_sort_by("last_modified_date")
 
 
+class NullsLastOrderingFilter(django_filters.OrderingFilter):
+    """
+    A custom ordering filter that ensures null values are sorted last.
+
+    When sorting by fields with potential null values, this filter modifies the
+    ordering to use Django's `nulls_last` clause for better handling of null values,
+    whether in ascending or descending order.
+    """
+
+    def filter(self, qs, value):
+        if not value:
+            return qs
+
+        ordering = []
+        for field in value:
+            if field.startswith("-"):
+                field_name = field[1:]
+                ordering.append(F(field_name).desc(nulls_last=True))
+            else:
+                ordering.append(F(field).asc(nulls_last=True))
+
+        return qs.order_by(*ordering)
+
+
 vulnerability_score_ranges = {
     "low": (0.1, 3),
     "medium": (4.0, 6.9),
@@ -302,7 +327,7 @@ class VulnerabilityFilterSet(DataspacedFilterSet):
         label=_("Search"),
         search_fields=["vulnerability_id", "aliases"],
     )
-    sort = DefaultOrderingFilter(
+    sort = NullsLastOrderingFilter(
         label=_("Sort"),
         fields=[
             "max_score",

component_catalog/tests/test_filters.py

@@ -420,6 +420,7 @@ def setUp(self):
             self.dataspace, max_score=5.5, aliases=["ALIAS-V2"]
         )
         self.vulnerability3 = make_vulnerability(self.dataspace, max_score=2.0)
+        self.vulnerability4 = make_vulnerability(self.dataspace, max_score=None)
 
     def test_vulnerability_filterset_search(self):
         data = {"q": self.vulnerability1.vulnerability_id}
@@ -430,15 +431,25 @@ def test_vulnerability_filterset_search(self):
         filterset = VulnerabilityFilterSet(dataspace=self.dataspace, data=data)
         self.assertQuerySetEqual(filterset.qs, [self.vulnerability2])
 
-    def test_vulnerability_filterset_sort(self):
+    def test_vulnerability_filterset_sort_nulls_last_ordering(self):
         data = {"sort": "max_score"}
         filterset = VulnerabilityFilterSet(dataspace=self.dataspace, data=data)
-        expected = [self.vulnerability3, self.vulnerability2, self.vulnerability1]
+        expected = [
+            self.vulnerability3,
+            self.vulnerability2,
+            self.vulnerability1,
+            self.vulnerability4,  # The max_score=None are always last
+        ]
         self.assertQuerySetEqual(filterset.qs, expected)
 
         data = {"sort": "-max_score"}
         filterset = VulnerabilityFilterSet(dataspace=self.dataspace, data=data)
-        expected = [self.vulnerability1, self.vulnerability2, self.vulnerability3]
+        expected = [
+            self.vulnerability1,
+            self.vulnerability2,
+            self.vulnerability3,
+            self.vulnerability4,  # The max_score=None are always last
+        ]
         self.assertQuerySetEqual(filterset.qs, expected)
 
     def test_vulnerability_filterset_max_score(self):

component_catalog/views.py

@@ -22,6 +22,7 @@
 from django.core import signing
 from django.core.validators import EMPTY_VALUES
 from django.db.models import Count
+from django.db.models import F
 from django.db.models import Prefetch
 from django.http import FileResponse
 from django.http import Http404
@@ -2518,7 +2519,7 @@ def get_queryset(self):
             .with_affected_products_count()
             .with_affected_packages_count()
             .order_by(
-                "-max_score",
+                F("max_score").desc(nulls_last=True),
                 "-min_score",
             )
         )