Skip to content

Commit

Permalink
fix: new approach to generate learner_state_count in admin assignme…
Browse files Browse the repository at this point in the history
…nts list API; add addtl ordering and filtering support (#324)
  • Loading branch information
adamstankiewicz authored Nov 8, 2023
1 parent ee87baa commit a064436
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 15 deletions.
5 changes: 5 additions & 0 deletions enterprise_access/apps/api/filters/base.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
"""
Base FilterSet utility classes.
"""
from django_filters import BaseInFilter, CharFilter
from django_filters import rest_framework as drf_filters


class CharInFilter(BaseInFilter, CharFilter):
pass


class HelpfulFilterSet(drf_filters.FilterSet):
"""
Using an explicit FilterSet object works nicely with drf-spectacular
Expand Down
5 changes: 4 additions & 1 deletion enterprise_access/apps/api/filters/content_assignments.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
API Filters for resources defined in the ``assignment_policy`` app.
"""
from ...content_assignments.models import AssignmentConfiguration, LearnerContentAssignment
from .base import HelpfulFilterSet
from .base import CharInFilter, HelpfulFilterSet


class AssignmentConfigurationFilter(HelpfulFilterSet):
Expand All @@ -18,11 +18,14 @@ class LearnerContentAssignmentAdminFilter(HelpfulFilterSet):
"""
Base filter for LearnerContentAssignment views.
"""
learner_state = CharInFilter(field_name='learner_state', lookup_expr='in')

class Meta:
model = LearnerContentAssignment
fields = [
'content_key',
'learner_email',
'lms_user_id',
'state',
'learner_state',
]
30 changes: 27 additions & 3 deletions enterprise_access/apps/api/v1/tests/test_assignment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,9 @@ def test_list(self, role_context_dict):
assert actual_assignment_uuids == expected_assignment_uuids

expected_learner_state_counts = [
{'count': 1, 'learner_state': 'failed'},
{'count': 1, 'learner_state': 'waiting'},
{'count': 1, 'learner_state': 'notifying'},
{'count': 1, 'learner_state': 'failed'},
]
assert response_json['learner_state_counts'] == expected_learner_state_counts

Expand Down Expand Up @@ -556,6 +556,30 @@ def test_cancel_non_cancelable_returns_422(self):
self.assignment_allocated_post_link.refresh_from_db()
assert self.assignment_accepted.state == LearnerContentAssignmentStateChoices.ACCEPTED

def test_learner_state_query_param_filter(self):
"""
Test that the list view supports filtering on one or more ``learner_state`` values via a query parameter.
"""
# Set the JWT-based auth that we'll use for every request.
self.set_jwt_cookie([{'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}])

# Fetch our list of assignments associated with the test enterprise.
assignments_for_enterprise_customer = LearnerContentAssignment.objects.filter(
assignment_configuration__enterprise_customer_uuid=TEST_ENTERPRISE_UUID
)
# Double check we have stuff to work with
assert assignments_for_enterprise_customer.count() > 1

# Hit the view with a learner_state query param.
learner_states_to_query = [AssignmentLearnerStates.WAITING, AssignmentLearnerStates.FAILED]
learner_state_query_param_value = ",".join(learner_states_to_query)
response = self.client.get(
ADMIN_ASSIGNMENTS_LIST_ENDPOINT + f"?learner_state={learner_state_query_param_value}"
)
# Assert the results only contain the requested ``learner_state`` values.
for assignment in response.json().get('results'):
assert assignment.get('learner_state') in learner_states_to_query

def test_assignment_search_query_param(self):
"""
Test that the list view follows the default Django API filtering with the usage of the ``search`` query param.
Expand All @@ -577,7 +601,7 @@ def test_assignment_search_query_param(self):
ADMIN_ASSIGNMENTS_LIST_ENDPOINT + f"?search={first_assignment.content_title}"
)
# Assert any of the results contain the content title matching the first assignment's.
for assignment in response.data.get('results'):
for assignment in response.json().get('results'):
assert assignment.get('content_title') == first_assignment.content_title

# Hit the view with a search query param for the learner email of another assignment.
Expand All @@ -586,7 +610,7 @@ def test_assignment_search_query_param(self):
ADMIN_ASSIGNMENTS_LIST_ENDPOINT + f"?search={second_assignment.learner_email}"
)
# Assert any of the results contain the learner email matching the second assignment's.
for assignment in response.data.get('results'):
for assignment in response.json().get('results'):
assert assignment.get('learner_email') == second_assignment.learner_email

# Hit the view with a search query param for a random string that should not match any assignments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
Admin-facing REST API views for LearnerContentAssignments in the content_assignments app.
"""
import logging
from collections import Counter

from django.db.models import Count
from drf_spectacular.utils import extend_schema
from edx_rbac.decorators import permission_required
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
Expand All @@ -15,8 +15,8 @@
from enterprise_access.apps.api import filters, serializers, utils
from enterprise_access.apps.api.v1.views.utils import PaginationWithPageCount
from enterprise_access.apps.content_assignments import api as assignments_api
from enterprise_access.apps.content_assignments.constants import AssignmentActions, LearnerContentAssignmentStateChoices
from enterprise_access.apps.content_assignments.models import LearnerContentAssignment, LearnerContentAssignmentAction
from enterprise_access.apps.content_assignments.constants import AssignmentLearnerStates
from enterprise_access.apps.content_assignments.models import LearnerContentAssignment
from enterprise_access.apps.core.constants import (
CONTENT_ASSIGNMENT_ADMIN_READ_PERMISSION,
CONTENT_ASSIGNMENT_ADMIN_WRITE_PERMISSION
Expand Down Expand Up @@ -89,7 +89,7 @@ class LearnerContentAssignmentAdminViewSet(

# Settings that control list ordering, powered by OrderingFilter.
# Fields in `ordering_fields` are what we allow to be passed to the "?ordering=" query param.
ordering_fields = ['recent_action_time', 'learner_state_sort_order']
ordering_fields = ['recent_action_time', 'learner_state_sort_order', 'content_quantity']
# `ordering` defines the default order.
ordering = ['-recent_action_time']

Expand Down Expand Up @@ -122,7 +122,7 @@ def get_queryset(self):
# * learner_state_sort_order
# * recent_action
# * recent_action_time
queryset = LearnerContentAssignment.annotate_dynamic_fields_onto_queryset(queryset)
queryset = LearnerContentAssignment.annotate_dynamic_fields_onto_queryset(queryset).prefetch_related('actions')

return queryset

Expand Down Expand Up @@ -151,13 +151,18 @@ def list(self, request, *args, **kwargs):
Lists ``LearnerContentAssignment`` records, filtered by the given query parameters.
"""
response = super().list(request, *args, **kwargs)
queryset = self.get_queryset()
learner_state_counts = queryset.values('learner_state') \
.exclude(learner_state__isnull=True) \
.annotate(count=Count('uuid', distinct=True)) \
.order_by('-count')

# Add the learner_state_overview to the default response.
# Compute the learner_state_counts for the filtered queryset.
queryset = self.filter_queryset(self.get_queryset())
learner_state_counter = Counter(
queryset.exclude(learner_state__isnull=True).values_list('learner_state', flat=True)
)
learner_state_counts = [
{'learner_state': state, 'count': count}
for state, count in learner_state_counter.most_common()
]

# Add the learner_state_counts to the default response.
response.data['learner_state_counts'] = learner_state_counts
return response

Expand Down

0 comments on commit a064436

Please sign in to comment.