From bb181e2b0384e61d71e36ee6d31e310ff03c279f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Je=C5=99=C3=A1bek=20=28Jiri=20Jerabek=29?= Date: Wed, 11 Oct 2023 15:55:36 +0200 Subject: [PATCH] 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 | 1 + galaxy_ng/app/api/v1/filtersets.py | 26 ++++++++-- galaxy_ng/app/api/v1/viewsets/roles.py | 7 --- .../tests/integration/api/test_community.py | 52 +++++++++++++++++++ 4 files changed, 75 insertions(+), 11 deletions(-) create mode 100644 CHANGES/2760.misc diff --git a/CHANGES/2760.misc b/CHANGES/2760.misc new file mode 100644 index 0000000000..548b19b2b6 --- /dev/null +++ b/CHANGES/2760.misc @@ -0,0 +1 @@ +Added ``download_count`` ordering to legacy roles api diff --git a/galaxy_ng/app/api/v1/filtersets.py b/galaxy_ng/app/api/v1/filtersets.py index 58de6de48c..8c734e7eaa 100644 --- a/galaxy_ng/app/api/v1/filtersets.py +++ b/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) diff --git a/galaxy_ng/app/api/v1/viewsets/roles.py b/galaxy_ng/app/api/v1/viewsets/roles.py index 951ba96a36..7638c35715 100644 --- a/galaxy_ng/app/api/v1/viewsets/roles.py +++ b/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 diff --git a/galaxy_ng/tests/integration/api/test_community.py b/galaxy_ng/tests/integration/api/test_community.py index 60bead6b25..c5f29d5c8c 100644 --- a/galaxy_ng/tests/integration/api/test_community.py +++ b/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)