Add download_count ordering to legacy roles endpoint (#1922)

* add download_count ordering to legacy roles api
* add test and fix import
* add modified ordering

Issue: AAH-2760

CHANGES/2760.misc

@@ -0,0 +1 @@
+Added ``download_count`` ordering to legacy roles api

galaxy_ng/app/api/v1/filtersets.py

@@ -1,4 +1,5 @@
 from django.db.models import Q
+from django.db.models import Case, Value, When
 from django_filters import filters
 from django_filters.rest_framework import filterset
 
@@ -72,6 +73,21 @@ def username_filter(self, queryset, name, value):
         return queryset.filter(username=username)
 
 
+class LegacyRoleFilterOrdering(filters.OrderingFilter):
+    def filter(self, qs, value):
+        if value is not None and any(v in ["download_count", "-download_count"] for v in value):
+            order = "-" if "-download_count" in value else ""
+
+            return qs.annotate(
+                download_count=Case(
+                    When(legacyroledownloadcount=None, then=Value(0)),
+                    default="legacyroledownloadcount__count",
+                )
+            ).order_by(f"{order}download_count")
+
+        return super().filter(qs, value)
+
+
 class LegacyRoleFilter(filterset.FilterSet):
 
     github_user = filters.CharFilter(method='github_user_filter')
@@ -82,16 +98,18 @@ class LegacyRoleFilter(filterset.FilterSet):
     owner__username = filters.CharFilter(method='owner__username_filter')
     namespace = filters.CharFilter(method='namespace_filter')
 
-    sort = filters.OrderingFilter(
+    order_by = LegacyRoleFilterOrdering(
         fields=(
-            ('created', 'full_metadata__created'),
-            # ('name', 'name')
+            ('name', 'name'),
+            ('created', 'created'),
+            ('modified', 'modified'),
+            ('download_count', 'download_count')
         )
     )
 
     class Meta:
         model = LegacyRole
-        fields = ['created', 'name']
+        fields = ['created', 'name', "modified"]
 
     def github_user_filter(self, queryset, name, value):
         return queryset.filter(namespace__name=value)

galaxy_ng/app/api/v1/viewsets/roles.py

@@ -50,7 +50,6 @@ class LegacyRolesViewSet(viewsets.ModelViewSet):
     """A list of legacy roles."""
 
     queryset = LegacyRole.objects.all().order_by('created')
-    ordering_fields = ('created')
     ordering = ('created')
     filter_backends = (DjangoFilterBackend,)
     filterset_class = LegacyRoleFilter
@@ -61,12 +60,6 @@ class LegacyRolesViewSet(viewsets.ModelViewSet):
     permission_classes = [LegacyAccessPolicy]
     authentication_classes = GALAXY_AUTHENTICATION_CLASSES
 
-    def get_queryset(self, *args, **kwargs):
-        order_by = self.request.query_params.get('order_by')
-        if order_by is not None:
-            return self.queryset.order_by(order_by)
-        return self.queryset
-
     def list(self, request):
 
         # this is the naive logic used in the original galaxy to assume a role

galaxy_ng/tests/integration/api/test_community.py

@@ -647,3 +647,55 @@ def get_roles(page_size=1, order_by='created'):
 
     # cleanup
     clean_all_roles(ansible_config)
+
+
+@pytest.mark.deployment_community
+def test_legacy_roles_ordering(ansible_config):
+    """ Tests if sorting is working correctly on v1 roles """
+
+    config = ansible_config("admin")
+    api_client = get_client(
+        config=config,
+        request_token=False,
+        require_auth=True
+    )
+
+    # clean all roles
+    clean_all_roles(ansible_config)
+
+    # start the sync
+    pargs = json.dumps({"limit": 5}).encode('utf-8')
+    resp = api_client('/api/v1/sync/', method='POST', args=pargs)
+    assert isinstance(resp, dict)
+    assert resp.get('task') is not None
+    wait_for_v1_task(resp=resp, api_client=api_client)
+
+    resp = api_client('/api/v1/roles/')
+    assert resp['count'] == 5
+
+    roles = resp["results"]
+
+    # default sorting should be "created"
+    created = [r["created"] for r in roles]
+    assert created == sorted(created)
+
+    names = [r["name"] for r in roles]
+    download_count = [r["download_count"] for r in roles]
+
+    # verify download_count sorting is correct
+    resp = api_client('/api/v1/roles/?order_by=download_count')
+    sorted_dc = [r["download_count"] for r in resp["results"]]
+    assert sorted_dc == sorted(download_count)
+
+    resp = api_client('/api/v1/roles/?order_by=-download_count')
+    sorted_dc = [r["download_count"] for r in resp["results"]]
+    assert sorted_dc == sorted(download_count, reverse=True)
+
+    # verify name sorting is correct
+    resp = api_client('/api/v1/roles/?order_by=name')
+    sorted_names = [r["name"] for r in resp["results"]]
+    assert sorted_names == sorted(names)
+
+    resp = api_client('/api/v1/roles/?order_by=-name')
+    sorted_names = [r["name"] for r in resp["results"]]
+    assert sorted_names == sorted(names, reverse=True)