From 3af2c583da1e7984d1e6e913713558494be2a440 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 13 Feb 2026 21:57:38 +0100 Subject: [PATCH 01/16] feat: implement eligibility REST API endpoint --- CHANGELOG.rst | 8 + learning_credentials/api/v1/permissions.py | 19 ++ learning_credentials/api/v1/serializers.py | 64 ++++- learning_credentials/api/v1/urls.py | 18 +- learning_credentials/api/v1/views.py | 278 +++++++++++++++++++- learning_credentials/models.py | 31 ++- learning_credentials/processors.py | 188 ++++++++++---- tests/conftest.py | 15 +- tests/test_models.py | 12 + tests/test_processors.py | 170 +++++++----- tests/test_views.py | 289 +++++++++++++++++++++ 11 files changed, 960 insertions(+), 132 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e17579a..8a6217f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,14 @@ Unreleased * +0.5.1 - 2026-02-13 +****************** + +Added +===== + +* Eligibility REST API endpoint. + 0.5.0 - 2026-01-29 ****************** diff --git a/learning_credentials/api/v1/permissions.py b/learning_credentials/api/v1/permissions.py index 80955db..8c7d942 100644 --- a/learning_credentials/api/v1/permissions.py +++ b/learning_credentials/api/v1/permissions.py @@ -19,6 +19,25 @@ from rest_framework.views import APIView +class IsAdminOrSelf(BasePermission): + """ + Permission to allow only admins or the user themselves to access the API. + + Non-staff users cannot pass a ``username`` that is not their own. + """ + + def has_permission(self, request: "Request", view: "APIView") -> bool: # noqa: ARG002 + """Check if the user is admin or accessing their own data.""" + if request.user.is_staff: + return True + + username = request.query_params.get("username") if request.method == "GET" else request.data.get("username") + + if username: + return request.user.username == username + return True + + class CanAccessLearningContext(BasePermission): """Permission to allow access to learning context if the user is enrolled.""" diff --git a/learning_credentials/api/v1/serializers.py b/learning_credentials/api/v1/serializers.py index e900165..ef55d47 100644 --- a/learning_credentials/api/v1/serializers.py +++ b/learning_credentials/api/v1/serializers.py @@ -1,15 +1,77 @@ """API serializers for learning credentials.""" +from typing import Any, ClassVar + from rest_framework import serializers from learning_credentials.models import Credential class CredentialSerializer(serializers.ModelSerializer): - """Serializer that returns credential metadata.""" + """Serializer that returns credential metadata (for the public verification endpoint).""" class Meta: """Serializer metadata.""" model = Credential fields = ('user_full_name', 'created', 'learning_context_name', 'status', 'invalidation_reason') + + +class CredentialModelSerializer(serializers.ModelSerializer): + """Serializer for Credential instances returned to authenticated users.""" + + credential_id = serializers.UUIDField(source='uuid', read_only=True) + credential_type = serializers.CharField(read_only=True) + context_key = serializers.CharField(source='learning_context_key', read_only=True) + created_date = serializers.DateTimeField(source='created', read_only=True) + download_url = serializers.URLField(read_only=True) + + class Meta: + """Serializer metadata.""" + + model = Credential + fields: ClassVar[list[str]] = [ + 'credential_id', + 'credential_type', + 'context_key', + 'status', + 'created_date', + 'download_url', + ] + read_only_fields: ClassVar[list[str]] = fields + + +class CredentialEligibilitySerializer(serializers.Serializer): + """Serializer for credential eligibility information with dynamic fields.""" + + credential_type_id = serializers.IntegerField() + name = serializers.CharField() + is_eligible = serializers.BooleanField() + existing_credential = serializers.UUIDField(required=False, allow_null=True) + existing_credential_url = serializers.URLField(required=False, allow_blank=True, allow_null=True) + + current_grades = serializers.DictField(required=False) + required_grades = serializers.DictField(required=False) + + current_completion = serializers.FloatField(required=False, allow_null=True) + required_completion = serializers.FloatField(required=False, allow_null=True) + + steps = serializers.DictField(required=False) + + def to_representation(self, instance: dict) -> dict[str, Any]: + """Remove null/empty fields from representation.""" + data = super().to_representation(instance) + return {key: value for key, value in data.items() if value is not None and value not in ({}, [])} + + +class CredentialEligibilityResponseSerializer(serializers.Serializer): + """Serializer for the complete credential eligibility response.""" + + context_key = serializers.CharField() + credentials = CredentialEligibilitySerializer(many=True) + + +class CredentialListResponseSerializer(serializers.Serializer): + """Serializer for credential list response.""" + + credentials = CredentialModelSerializer(many=True) diff --git a/learning_credentials/api/v1/urls.py b/learning_credentials/api/v1/urls.py index 7528e4e..0f1ed1e 100644 --- a/learning_credentials/api/v1/urls.py +++ b/learning_credentials/api/v1/urls.py @@ -2,7 +2,12 @@ from django.urls import path -from .views import CredentialConfigurationCheckView, CredentialMetadataView +from .views import ( + CredentialConfigurationCheckView, + CredentialEligibilityView, + CredentialListView, + CredentialMetadataView, +) urlpatterns = [ path( @@ -11,4 +16,15 @@ name='credential_configuration_check', ), path('metadata//', CredentialMetadataView.as_view(), name='credential-metadata'), + path( + 'eligibility//', + CredentialEligibilityView.as_view(), + name='credential-eligibility', + ), + path( + 'eligibility///', + CredentialEligibilityView.as_view(), + name='credential-generation', + ), + path('credentials/', CredentialListView.as_view(), name='credential-list'), ] diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 18fd8f6..e10c820 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -1,8 +1,11 @@ """API views for Learning Credentials.""" +import logging from typing import TYPE_CHECKING import edx_api_doc_tools as apidocs +from django.contrib.auth.models import User +from django.shortcuts import get_object_or_404 from edx_api_doc_tools import ParameterLocation from rest_framework import status from rest_framework.permissions import IsAuthenticated @@ -10,18 +13,26 @@ from rest_framework.views import APIView from learning_credentials.models import Credential, CredentialConfiguration +from learning_credentials.tasks import generate_credential_for_user_task -from .permissions import CanAccessLearningContext -from .serializers import CredentialSerializer +from .permissions import CanAccessLearningContext, IsAdminOrSelf +from .serializers import ( + CredentialEligibilityResponseSerializer, + CredentialListResponseSerializer, + CredentialModelSerializer, + CredentialSerializer, +) if TYPE_CHECKING: from rest_framework.request import Request +logger = logging.getLogger(__name__) + class CredentialConfigurationCheckView(APIView): """API view to check if any credentials are configured for a specific learning context.""" - permission_classes = (IsAuthenticated, CanAccessLearningContext) + permission_classes = (IsAuthenticated, IsAdminOrSelf, CanAccessLearningContext) @apidocs.schema( parameters=[ @@ -141,3 +152,264 @@ def get(self, _request: "Request", uuid: str) -> Response: serializer = CredentialSerializer(credential) return Response(serializer.data, status=status.HTTP_200_OK) + + +class CredentialEligibilityView(APIView): + """ + API view for credential eligibility checking and generation. + + **GET**: Returns detailed eligibility info for all configured credentials in a learning context. + **POST**: Triggers credential generation for an eligible user. + + Staff users can operate on behalf of other users via the ``username`` parameter. + """ + + permission_classes = (IsAuthenticated, IsAdminOrSelf, CanAccessLearningContext) + + def _get_eligibility_data(self, user: User, config: "CredentialConfiguration") -> dict: + """Calculate eligibility data for a credential configuration.""" + progress_data = config.get_user_eligibility_details(user_id=user.id) + + existing_credential = ( + Credential.objects.filter( + user_id=user.id, + configuration=config, + ) + .exclude(status=Credential.Status.ERROR) + .first() + ) + + return { + 'credential_type_id': config.credential_type.pk, + 'name': config.credential_type.name, + **progress_data, + 'existing_credential': existing_credential.uuid if existing_credential else None, + 'existing_credential_url': existing_credential.download_url if existing_credential else None, + } + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "learning_context_key", + ParameterLocation.PATH, + description=( + "Learning context identifier. Can be a course key (course-v1:OpenedX+DemoX+DemoCourse) " + "or learning path key (path-v1:OpenedX+DemoX+DemoPath+Demo)" + ), + ), + ], + responses={ + 200: CredentialEligibilityResponseSerializer, + 400: "Invalid context key format.", + 403: "User is not authenticated.", + 404: "Learning context not found or user does not have access.", + }, + ) + def get(self, request: "Request", learning_context_key: str) -> Response: + """ + Get credential eligibility for a learning context. + + Returns detailed eligibility information for all configured credentials, including: + + - Current grades and requirements for grade-based credentials + - Completion percentages for completion-based credentials + - Step-by-step progress for learning paths + - Existing credential info if already generated + + **Query Parameters** + + - ``username`` (staff only): View eligibility for a specific user. + + **Example Request** + + ``GET /api/learning_credentials/v1/eligibility/course-v1:OpenedX+DemoX+DemoCourse/`` + + **Example Response** + + .. code-block:: json + + { + "context_key": "course-v1:OpenedX+DemoX+DemoCourse", + "credentials": [ + { + "credential_type_id": 1, + "name": "Certificate of Achievement", + "is_eligible": true, + "existing_credential": null, + "current_grades": {"final exam": 86, "total": 82}, + "required_grades": {"final exam": 65, "total": 80} + } + ] + } + """ + username = request.query_params.get('username') + user = get_object_or_404(User, username=username) if username else request.user + + configurations = CredentialConfiguration.objects.filter( + learning_context_key=learning_context_key + ).select_related('credential_type') + + eligibility_data = [self._get_eligibility_data(user, config) for config in configurations] + + response_data = { + 'context_key': learning_context_key, + 'credentials': eligibility_data, + } + + serializer = CredentialEligibilityResponseSerializer(data=response_data) + serializer.is_valid(raise_exception=True) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "learning_context_key", + ParameterLocation.PATH, + description="Learning context identifier (e.g. course-v1:OpenedX+DemoX+DemoCourse)", + ), + apidocs.parameter( + "credential_type_id", + ParameterLocation.PATH, + int, + description="ID of the credential type to generate", + ), + ], + responses={ + 201: "Credential generation started.", + 400: "User is not eligible for this credential.", + 403: "User is not authenticated.", + 404: "Learning context or credential type not found.", + 409: "User already has a valid credential of this type.", + }, + ) + def post(self, request: "Request", learning_context_key: str, credential_type_id: int) -> Response: + """ + Trigger credential generation for an eligible user. + + The user must be eligible and must not already have an existing valid credential of this type. + + **Query Parameters** + + - ``username`` (staff only): Trigger generation for a specific user. + + **Example Request** + + ``POST /api/learning_credentials/v1/eligibility/course-v1:OpenedX+DemoX+DemoCourse/1/`` + + **Example Response** + + .. code-block:: json + + { + "detail": "Credential generation started." + } + """ + username = request.query_params.get('username') + user = get_object_or_404(User, username=username) if username else request.user + + config = get_object_or_404( + CredentialConfiguration.objects.select_related('credential_type'), + learning_context_key=learning_context_key, + credential_type_id=credential_type_id, + ) + + existing_credential = ( + Credential.objects.filter(user_id=user.id, configuration=config) + .exclude(status=Credential.Status.ERROR) + .first() + ) + + if existing_credential: + return Response( + {"detail": "User already has a credential of this type."}, + status=status.HTTP_409_CONFLICT, + ) + + if not config.get_eligible_user_ids(user_id=user.id): + return Response( + {"detail": "User is not eligible for this credential."}, + status=status.HTTP_400_BAD_REQUEST, + ) + + generate_credential_for_user_task.delay(config.id, user.id) + return Response({"detail": "Credential generation started."}, status=status.HTTP_201_CREATED) + + +class CredentialListView(APIView): + """ + API view to list user credentials. + + Staff users can view credentials for any user via the ``username`` parameter. + Non-staff users can only view their own credentials. + Optionally filter by ``learning_context_key``. + """ + + def get_permissions(self) -> list: + """Return permissions, adding context access check only when filtering by learning context.""" + permission_classes = [IsAuthenticated, IsAdminOrSelf] + + if self.request.query_params.get('learning_context_key'): + permission_classes.append(CanAccessLearningContext) + + return [permission() for permission in permission_classes] + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "learning_context_key", + ParameterLocation.QUERY, + description="Optional learning context to filter credentials.", + ), + apidocs.string_parameter( + "username", + ParameterLocation.QUERY, + description="Username to view credentials for (staff only).", + ), + ], + responses={ + 200: CredentialListResponseSerializer, + 403: "User is not authenticated or lacks permission.", + 404: "Specified user not found or learning context not accessible.", + }, + ) + def get(self, request: "Request") -> Response: + """ + Retrieve a list of credentials for the authenticated user or a specified user. + + **Query Parameters** + + - ``username`` (staff only): View credentials for a specific user. + - ``learning_context_key`` (optional): Filter credentials by learning context. + + **Example Request** + + ``GET /api/learning_credentials/v1/credentials/`` + + **Example Response** + + .. code-block:: json + + { + "credentials": [ + { + "credential_id": "123e4567-e89b-12d3-a456-426614174000", + "credential_type": "Certificate of Achievement", + "context_key": "course-v1:OpenedX+DemoX+DemoCourse", + "status": "available", + "created_date": "2024-08-20T10:30:00Z", + "download_url": "https://example.com/credentials/123e4567.pdf" + } + ] + } + """ + learning_context_key = request.query_params.get('learning_context_key') + username = request.query_params.get('username') + user = get_object_or_404(User, username=username) if username else request.user + + credentials_queryset = Credential.objects.filter(user_id=user.pk) + + if learning_context_key: + credentials_queryset = credentials_queryset.filter(learning_context_key=learning_context_key) + + credentials_data = CredentialModelSerializer(credentials_queryset, many=True).data + return Response({'credentials': credentials_data}) diff --git a/learning_credentials/models.py b/learning_credentials/models.py index 5481d5c..55ee2e2 100644 --- a/learning_credentials/models.py +++ b/learning_credentials/models.py @@ -7,7 +7,7 @@ import uuid as uuid_lib from importlib import import_module from pathlib import Path -from typing import TYPE_CHECKING, Self +from typing import TYPE_CHECKING, Any, Self import jsonfield from django.conf import settings @@ -183,11 +183,12 @@ def filter_out_user_ids_with_credentials(self, user_ids: list[int]) -> list[int] filtered_user_ids_set = set(user_ids) - set(users_ids_with_credentials) return list(filtered_user_ids_set) - def get_eligible_user_ids(self) -> list[int]: + def _call_retrieval_func(self, user_id: int | None = None) -> dict[int, dict[str, Any]]: """ - Get the list of eligible learners for the given course. + Call the retrieval function and return detailed results. - :return: A list of user IDs. + :param user_id: Optional. If provided, only check eligibility for this user. + :return: A dict mapping user IDs to their detailed progress information. """ func_path = self.credential_type.retrieval_func module_path, func_name = func_path.rsplit('.', 1) @@ -195,7 +196,27 @@ def get_eligible_user_ids(self) -> list[int]: func = getattr(module, func_name) custom_options = _deep_merge(self.credential_type.custom_options, self.custom_options) - return func(self.learning_context_key, custom_options) + return func(self.learning_context_key, custom_options, user_id=user_id) + + def get_eligible_user_ids(self, user_id: int | None = None) -> list[int]: + """ + Get the list of eligible learners for the given learning context. + + :param user_id: Optional. If provided, only check eligibility for this user. + :return: A list of eligible user IDs. + """ + results = self._call_retrieval_func(user_id) + return [uid for uid, details in results.items() if details.get('is_eligible', False)] + + def get_user_eligibility_details(self, user_id: int) -> dict[str, Any]: + """ + Get detailed eligibility information for a specific user. + + :param user_id: The user ID to check eligibility for. + :return: Dictionary containing eligibility details and progress information. + """ + results = self._call_retrieval_func(user_id) + return results.get(user_id, {'is_eligible': False}) def generate_credential_for_user(self, user_id: int, celery_task_id: int = 0) -> Credential: """ diff --git a/learning_credentials/processors.py b/learning_credentials/processors.py index f4aeca3..1e03226 100644 --- a/learning_credentials/processors.py +++ b/learning_credentials/processors.py @@ -4,6 +4,9 @@ The functions prefixed with `retrieve_` are automatically detected by the admin page and are used to retrieve the IDs of the users that meet the criteria for the credential type. +All processors return ``dict[int, dict[str, Any]]`` — a mapping from user ID to detailed progress info. +Each user's dict always includes an ``is_eligible`` boolean. + We will move this module to an external repository (a plugin). """ @@ -38,46 +41,63 @@ def _process_learning_context( learning_context_key: LearningContextKey, - course_processor: Callable[[CourseKey, dict[str, Any]], list[int]], + course_processor: Callable[[CourseKey, dict[str, Any], int | None], dict[int, dict[str, Any]]], options: dict[str, Any], -) -> list[int]: + user_id: int | None = None, +) -> dict[int, dict[str, Any]]: """ Process a learning context (course or learning path) using the given course processor function. For courses, runs the processor directly. For learning paths, runs the processor on each - course in the path with step-specific options (if available), and returns the intersection - of eligible users across all courses. + course in the path with step-specific options (if available), and returns detailed results + with per-step breakdown. Args: learning_context_key: A course key or learning path key to process - course_processor: A function that processes a single course and returns eligible user IDs + course_processor: A function that processes a single course and returns detailed progress for users options: Options to pass to the processor. For learning paths, may contain a "steps" key with step-specific options in the format: {"steps": {"": {...}}} + user_id: Optional. If provided, only process this specific user. Returns: - A list of eligible user IDs + A dict mapping user IDs to their detailed progress information. """ if learning_context_key.is_course: - return course_processor(learning_context_key, options) + return course_processor(learning_context_key, options, user_id) learning_path = LearningPath.objects.get(key=learning_context_key) - results = None - for course in learning_path.steps.all(): - course_options = options.get("steps", {}).get(str(course.course_key), options) - course_results = set(course_processor(course.course_key, course_options)) + step_results_by_course: dict[str, dict[int, dict[str, Any]]] = {} + all_user_ids: set[int] = set() + + # TODO: Use a single Completion Aggregator request when retrieving step results for a single user. + for step in learning_path.steps.all(): + course_options = options.get("steps", {}).get(str(step.course_key), options) + step_results = course_processor(step.course_key, course_options, user_id) - if results is None: - results = course_results - else: - results &= course_results + step_results_by_course[str(step.course_key)] = step_results + all_user_ids.update(step_results.keys()) # Filter out users who are not enrolled in the Learning Path. - results &= set( - learning_path.enrolled_users.filter(learningpathenrollment__is_active=True).values_list('id', flat=True), + all_user_ids &= set( + learning_path.enrolled_users.filter(learningpathenrollment__is_active=True).values_list('id', flat=True) ) - return list(results) if results else [] + final_results: dict[int, dict[str, Any]] = {} + for uid in all_user_ids: + overall_eligible = True + user_step_results: dict[str, dict[str, Any]] = {} + + for course_key, step_results in step_results_by_course.items(): + if uid in step_results: + user_step_results[course_key] = step_results[uid] + overall_eligible = overall_eligible and step_results[uid].get('is_eligible', False) + else: + overall_eligible = False + + final_results[uid] = {'is_eligible': overall_eligible, 'steps': user_step_results} + + return final_results def _get_category_weights(course_id: CourseKey) -> dict[str, float]: @@ -100,7 +120,7 @@ def _get_category_weights(course_id: CourseKey) -> dict[str, float]: return category_weight_ratios -def _get_grades_by_format(course_id: CourseKey, users: list[User]) -> dict[int, dict[str, int]]: +def _get_grades_by_format(course_id: CourseKey, users: list[User]) -> dict[int, dict[str, float]]: """ Get the grades for each user, categorized by assignment types. @@ -162,31 +182,63 @@ def _are_grades_passing_criteria( return total_score >= required_grades.get('total', 0) -def _retrieve_course_subsection_grades(course_id: CourseKey, options: dict[str, Any]) -> list[int]: - """Implementation for retrieving course grades.""" +def _calculate_grades_progress( + user_grades: dict[str, float], + required_grades: dict[str, float], + category_weights: dict[str, float], +) -> dict[str, Any]: + """ + Calculate detailed progress information for grade-based criteria. + + :param user_grades: The grades of the user, divided by category. + :param required_grades: The required grades for each category. + :param category_weights: The weight of each category. + :returns: Dict with is_eligible, current_grades, and required_grades. + """ + total_score = 0 + for category, score in user_grades.items(): + if category in category_weights: + total_score += score * category_weights[category] + + user_grades_with_total = {**user_grades, 'total': total_score} + is_eligible = _are_grades_passing_criteria(user_grades, required_grades, category_weights) + + return { + 'is_eligible': is_eligible, + 'current_grades': user_grades_with_total, + 'required_grades': required_grades, + } + + +def _retrieve_course_subsection_grades( + course_id: CourseKey, options: dict[str, Any], user_id: int | None = None +) -> dict[int, dict[str, Any]]: + """Retrieve detailed grade progress for enrolled users in a course.""" required_grades: dict[str, int] = options['required_grades'] required_grades = {key.lower(): value * 100 for key, value in required_grades.items()} - users = get_course_enrollments(course_id) + users = get_course_enrollments(course_id, user_id) grades = _get_grades_by_format(course_id, users) log.debug(grades) weights = _get_category_weights(course_id) - eligible_users = [] - for user_id, user_grades in grades.items(): - if _are_grades_passing_criteria(user_grades, required_grades, weights): - eligible_users.append(user_id) + results: dict[int, dict[str, Any]] = {} + for uid, user_grades in grades.items(): + results[uid] = _calculate_grades_progress(user_grades, required_grades, weights) - return eligible_users + return results -def retrieve_subsection_grades(learning_context_key: LearningContextKey, options: dict[str, Any]) -> list[int]: +def retrieve_subsection_grades( + learning_context_key: LearningContextKey, options: dict[str, Any], user_id: int | None = None +) -> dict[int, dict[str, Any]]: """ - Retrieve the users that have passing grades in all required categories. + Retrieve detailed grade progress for users in a learning context. :param learning_context_key: The learning context key (course or learning path). :param options: The custom options for the credential. - :returns: The IDs of the users that have passing grades in all required categories. + :param user_id: Optional. If provided, only process this specific user. + :returns: A dict mapping user IDs to their grade progress (is_eligible, current_grades, required_grades). Options: - required_grades: A dictionary of required grades for each category, where the keys are the category names and @@ -232,7 +284,7 @@ def retrieve_subsection_grades(learning_context_key: LearningContextKey, options } } """ - return _process_learning_context(learning_context_key, _retrieve_course_subsection_grades, options) + return _process_learning_context(learning_context_key, _retrieve_course_subsection_grades, options, user_id) def _prepare_request_to_completion_aggregator(course_id: CourseKey, query_params: dict, url: str) -> APIView: @@ -252,7 +304,7 @@ def _prepare_request_to_completion_aggregator(course_id: CourseKey, query_params drf_request = Request(django_request) # convert django.core.handlers.wsgi.WSGIRequest to DRF request view = CompletionDetailView() - view.request = drf_request + view.request = drf_request # type: ignore[assignment] # HACK: Bypass the API permissions. staff_user = get_user_model().objects.filter(is_staff=True).first() @@ -262,8 +314,10 @@ def _prepare_request_to_completion_aggregator(course_id: CourseKey, query_params return view -def _retrieve_course_completions(course_id: CourseKey, options: dict[str, Any]) -> list[int]: - """Implementation for retrieving course completions.""" +def _retrieve_course_completions( + course_id: CourseKey, options: dict[str, Any], user_id: int | None = None +) -> dict[int, dict[str, Any]]: + """Retrieve detailed completion progress for enrolled users in a course.""" # If it turns out to be too slow, we can: # 1. Modify the Completion Aggregator to emit a signal/event when a user achieves a certain completion threshold. # 2. Get this data from the `Aggregator` model. Filter by `aggregation name == 'course'`, `course_key`, `percent`. @@ -276,30 +330,46 @@ def _retrieve_course_completions(course_id: CourseKey, options: dict[str, Any]) # TODO: Extract the logic of this view into an API. The current approach is very hacky. view = _prepare_request_to_completion_aggregator(course_id, query_params.copy(), url) - completions = [] + completions: dict[str, float] = {} while True: - # noinspection PyUnresolvedReferences response = view.get(view.request, str(course_id)) log.debug(response.data) - completions.extend( - res['username'] for res in response.data['results'] if res['completion']['percent'] >= required_completion - ) + for res in response.data['results']: + completions[res['username']] = res['completion']['percent'] if not response.data['pagination']['next']: break query_params['page'] += 1 view = _prepare_request_to_completion_aggregator(course_id, query_params.copy(), url) - return list(get_user_model().objects.filter(username__in=completions).values_list('id', flat=True)) + # Map usernames to user IDs via enrolled users. + users = get_course_enrollments(course_id, user_id) + username_to_id = {user.username: user.id for user in users} + + results: dict[int, dict[str, Any]] = {} + for username, current_completion in completions.items(): + if username in username_to_id: + uid = username_to_id[username] + results[uid] = { + 'is_eligible': current_completion >= required_completion, + 'current_completion': current_completion, + 'required_completion': required_completion, + } + return results -def retrieve_completions(learning_context_key: LearningContextKey, options: dict[str, Any]) -> list[int]: + +def retrieve_completions( + learning_context_key: LearningContextKey, options: dict[str, Any], user_id: int | None = None +) -> dict[int, dict[str, Any]]: """ - Retrieve the course completions for all users through the Completion Aggregator API. + Retrieve detailed completion progress for users through the Completion Aggregator API. :param learning_context_key: The learning context key (course or learning path). :param options: The custom options for the credential. - :returns: The IDs of the users that have achieved the required completion percentage. + :param user_id: Optional. If provided, only process this specific user. + :returns: A dict mapping user IDs to their completion progress + (is_eligible, current_completion, required_completion). Options: - required_completion: The minimum required completion percentage. The default value is 0.9. @@ -319,19 +389,22 @@ def retrieve_completions(learning_context_key: LearningContextKey, options: dict } } """ - return _process_learning_context(learning_context_key, _retrieve_course_completions, options) + return _process_learning_context(learning_context_key, _retrieve_course_completions, options, user_id) -def retrieve_completions_and_grades(learning_context_key: LearningContextKey, options: dict[str, Any]) -> list[int]: +def retrieve_completions_and_grades( + learning_context_key: LearningContextKey, options: dict[str, Any], user_id: int | None = None +) -> dict[int, dict[str, Any]]: """ - Retrieve the users that meet both completion and grade criteria. + Retrieve detailed progress for users that must meet both completion and grade criteria. - This processor combines the functionality of retrieve_course_completions and retrieve_subsection_grades. + This processor combines the functionality of retrieve_completions and retrieve_subsection_grades. To be eligible, learners must satisfy both sets of criteria. :param learning_context_key: The learning context key (course or learning path). :param options: The custom options for the credential. - :returns: The IDs of the users that meet both sets of criteria. + :param user_id: Optional. If provided, only process this specific user. + :returns: A dict mapping user IDs to their combined progress. Options: - required_completion: The minimum required completion percentage (default: 0.9) @@ -372,7 +445,18 @@ def retrieve_completions_and_grades(learning_context_key: LearningContextKey, op } } """ - completion_eligible_users = set(retrieve_completions(learning_context_key, options)) - grades_eligible_users = set(retrieve_subsection_grades(learning_context_key, options)) - - return list(completion_eligible_users & grades_eligible_users) + completion_results = retrieve_completions(learning_context_key, options, user_id) + grade_results = retrieve_subsection_grades(learning_context_key, options, user_id) + + # Merge results for users present in both. + combined: dict[int, dict[str, Any]] = {} + for uid in set(completion_results) & set(grade_results): + comp = completion_results[uid] + grade = grade_results[uid] + combined[uid] = { + **comp, + **grade, + 'is_eligible': comp['is_eligible'] and grade['is_eligible'], + } + + return combined diff --git a/tests/conftest.py b/tests/conftest.py index 5ce58cc..ef0f13a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -102,9 +102,18 @@ def learning_path_with_courses(users: list[User]) -> LearningPath: # ============================================================================= -def _mock_retrieval_func(_context_id: LearningContextKey, _options: dict) -> list[int]: - """Mock retrieval function that returns a fixed list of user IDs.""" - return [1, 2, 3] +def _mock_retrieval_func( + _context_id: LearningContextKey, _options: dict, user_id: int | None = None +) -> dict[int, dict]: + """Mock retrieval function that returns detailed eligibility results.""" + all_results = { + 1: {'is_eligible': True}, + 2: {'is_eligible': True}, + 3: {'is_eligible': True}, + } + if user_id is not None: + return {user_id: all_results[user_id]} if user_id in all_results else {} + return all_results def _mock_generation_func(_credential: Credential, _options: dict, *, invalidate: bool = False) -> str: diff --git a/tests/test_models.py b/tests/test_models.py index b5ca0b2..f4e9256 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -134,6 +134,18 @@ def test_get_eligible_user_ids(self, mock_credential_config: CredentialConfigura eligible_user_ids = mock_credential_config.get_eligible_user_ids() assert eligible_user_ids == [1, 2, 3] + @pytest.mark.django_db + def test_get_user_eligibility_details(self, mock_credential_config: CredentialConfiguration): + """Test that get_user_eligibility_details returns details for a known user.""" + details = mock_credential_config.get_user_eligibility_details(user_id=1) + assert details == {'is_eligible': True} + + @pytest.mark.django_db + def test_get_user_eligibility_details_fallback(self, mock_credential_config: CredentialConfiguration): + """Test that get_user_eligibility_details returns default when user is not in results.""" + details = mock_credential_config.get_user_eligibility_details(user_id=999) + assert details == {'is_eligible': False} + @pytest.mark.django_db @patch('tests.conftest._mock_retrieval_func') def test_custom_options_deep_merge( diff --git a/tests/test_processors.py b/tests/test_processors.py index 39789f8..af4e70f 100644 --- a/tests/test_processors.py +++ b/tests/test_processors.py @@ -155,14 +155,12 @@ def test_are_grades_passing_criteria_invalid_grade_category(): @patch('learning_credentials.processors.get_course_enrollments') @patch('learning_credentials.processors._get_grades_by_format') @patch('learning_credentials.processors._get_category_weights') -@patch('learning_credentials.processors._are_grades_passing_criteria') def test_retrieve_subsection_grades( - mock_are_grades_passing_criteria: Mock, mock_get_category_weights: Mock, mock_get_grades_by_format: Mock, mock_get_course_enrollments: Mock, ): - """Test that the function returns the eligible users.""" + """Test that the function returns detailed eligibility results for users.""" course_id = Mock(spec=CourseKey) options = { 'required_grades': { @@ -173,29 +171,52 @@ def test_retrieve_subsection_grades( } users = [Mock(name="User1", id=101), Mock(name="User2", id=102)] grades = { - 101: {'homework': 0.5, 'exam': 0.9}, - 102: {'homework': 0.3, 'exam': 0.95}, + 101: {'homework': 80.0, 'exam': 95.0}, + 102: {'homework': 30.0, 'exam': 95.0}, } - required_grades = {'homework': 40.0, 'exam': 90.0, 'total': 80.0} weights = {'homework': 0.2, 'exam': 0.7, 'lab': 0.1} mock_get_course_enrollments.return_value = users mock_get_grades_by_format.return_value = grades mock_get_category_weights.return_value = weights - mock_are_grades_passing_criteria.side_effect = [True, False] result = retrieve_subsection_grades(course_id, options) - assert result == [101] - mock_get_course_enrollments.assert_called_once_with(course_id) + assert 101 in result + assert 102 in result + assert result[101]['is_eligible'] is True + assert result[102]['is_eligible'] is False + assert 'current_grades' in result[101] + assert 'required_grades' in result[101] + mock_get_course_enrollments.assert_called_once_with(course_id, None) mock_get_grades_by_format.assert_called_once_with(course_id, users) mock_get_category_weights.assert_called_once_with(course_id) - mock_are_grades_passing_criteria.assert_has_calls( - [ - call(grades[101], required_grades, weights), - call(grades[102], required_grades, weights), - ], - ) + + +@patch('learning_credentials.processors.get_course_enrollments') +@patch('learning_credentials.processors._get_grades_by_format') +@patch('learning_credentials.processors._get_category_weights') +def test_retrieve_subsection_grades_unknown_grade_category( + mock_get_category_weights: Mock, + mock_get_grades_by_format: Mock, + mock_get_course_enrollments: Mock, +): + """Test that grade categories not in the grading policy are skipped in total calculation.""" + course_id = Mock(spec=CourseKey) + + options = {'required_grades': {'exam': 0.9}} + users = [Mock(name="User1", id=101)] + grades = {101: {'homework': 80.0, 'lab': 100.0}} + weights = {'homework': 0.5, 'exam': 0.5} + + mock_get_course_enrollments.return_value = users + mock_get_grades_by_format.return_value = grades + mock_get_category_weights.return_value = weights + + result = retrieve_subsection_grades(course_id, options) + + assert result[101]['is_eligible'] is False + assert result[101]['current_grades']['total'] == 80.0 * 0.5 def test_prepare_request_to_completion_aggregator(): @@ -228,9 +249,11 @@ def test_prepare_request_to_completion_aggregator(): @patch('learning_credentials.processors._prepare_request_to_completion_aggregator') -@patch('learning_credentials.processors.get_user_model') -def test_retrieve_course_completions(mock_get_user_model: Mock, mock_prepare_request_to_completion_aggregator: Mock): - """Test that we retrieve the course completions for all users and return IDs of users who meet the criteria.""" +@patch('learning_credentials.processors.get_course_enrollments') +def test_retrieve_course_completions( + mock_get_course_enrollments: Mock, mock_prepare_request_to_completion_aggregator: Mock +): + """Test that we retrieve the course completions for all users and return detailed results.""" course_id = Mock(spec=CourseKey) options = {'required_completion': 0.8} completions_page1 = { @@ -244,6 +267,7 @@ def test_retrieve_course_completions(mock_get_user_model: Mock, mock_prepare_req 'results': [ {'username': 'user2', 'completion': {'percent': 0.7}}, {'username': 'user3', 'completion': {'percent': 0.8}}, + {'username': 'unenrolled_user', 'completion': {'percent': 0.95}}, ], } @@ -253,31 +277,18 @@ def test_retrieve_course_completions(mock_get_user_model: Mock, mock_prepare_req mock_view_page2.get.return_value.data = completions_page2 mock_prepare_request_to_completion_aggregator.side_effect = [mock_view_page1, mock_view_page2] - def filter_side_effect(*_args, **kwargs) -> list[int]: - """ - A mock side effect function for User.objects.filter(). - - It allows testing this code without a database access. - - :returns: The user IDs corresponding to the provided usernames. - """ - usernames = kwargs['username__in'] - - values_list_mock = Mock() - values_list_mock.return_value = [username_id_map[username] for username in usernames] - queryset_mock = Mock() - queryset_mock.values_list = values_list_mock - - return queryset_mock - - username_id_map = {"user1": 1, "user2": 2, "user3": 3} - mock_user_model = Mock() - mock_user_model.objects.filter.side_effect = filter_side_effect - mock_get_user_model.return_value = mock_user_model + # Mock enrolled users to map usernames to IDs. + mock_get_course_enrollments.return_value = [ + Mock(username='user1', id=1), + Mock(username='user2', id=2), + Mock(username='user3', id=3), + ] result = retrieve_completions(course_id, options) - assert result == [1, 3] + assert result[1] == {'is_eligible': True, 'current_completion': 0.9, 'required_completion': 0.8} + assert result[2] == {'is_eligible': False, 'current_completion': 0.7, 'required_completion': 0.8} + assert result[3] == {'is_eligible': True, 'current_completion': 0.8, 'required_completion': 0.8} mock_prepare_request_to_completion_aggregator.assert_has_calls( [ call(course_id, {'page_size': 1000, 'page': 1}, f'/completion-aggregator/v1/course/{course_id}/'), @@ -286,16 +297,31 @@ def filter_side_effect(*_args, **kwargs) -> list[int]: ) mock_view_page1.get.assert_called_once_with(mock_view_page1.request, str(course_id)) mock_view_page2.get.assert_called_once_with(mock_view_page2.request, str(course_id)) - mock_user_model.objects.filter.assert_called_once_with(username__in=['user1', 'user3']) @pytest.mark.parametrize( - ('completion_users', 'grades_users', 'expected_result'), + ('completion_results', 'grade_results', 'expected_eligible_ids'), [ - ([101, 102, 103], [102, 103, 104], [102, 103]), - ([101, 102], [103, 104], []), - ([101, 102], [101, 102], [101, 102]), - ([101, 102], [], []), + ( + {101: {'is_eligible': True}, 102: {'is_eligible': True}, 103: {'is_eligible': True}}, + {102: {'is_eligible': True}, 103: {'is_eligible': True}, 104: {'is_eligible': True}}, + {102, 103}, + ), + ( + {101: {'is_eligible': True}, 102: {'is_eligible': True}}, + {103: {'is_eligible': True}, 104: {'is_eligible': True}}, + set(), + ), + ( + {101: {'is_eligible': True}, 102: {'is_eligible': True}}, + {101: {'is_eligible': True}, 102: {'is_eligible': True}}, + {101, 102}, + ), + ( + {101: {'is_eligible': True}, 102: {'is_eligible': True}}, + {}, + set(), + ), ], ids=[ "Some users pass both criteria", @@ -309,22 +335,24 @@ def filter_side_effect(*_args, **kwargs) -> list[int]: def test_retrieve_course_completions_and_grades( mock_retrieve_completions: Mock, mock_retrieve_subsection_grades: Mock, - completion_users: list[int], - grades_users: list[int], - expected_result: list[int], + completion_results: dict, + grade_results: dict, + expected_eligible_ids: set[int], ): - """Test that the function returns the intersection of eligible users from both criteria.""" + """Test that the function merges results for users present in both criteria.""" course_id = Mock(spec=CourseKey) options = Mock() - mock_retrieve_completions.return_value = completion_users - mock_retrieve_subsection_grades.return_value = grades_users + mock_retrieve_completions.return_value = completion_results + mock_retrieve_subsection_grades.return_value = grade_results result = retrieve_completions_and_grades(course_id, options) - assert result == expected_result - mock_retrieve_completions.assert_called_once_with(course_id, options) - mock_retrieve_subsection_grades.assert_called_once_with(course_id, options) + assert set(result.keys()) == expected_eligible_ids + for uid in expected_eligible_ids: + assert result[uid]['is_eligible'] is True + mock_retrieve_completions.assert_called_once_with(course_id, options, None) + mock_retrieve_subsection_grades.assert_called_once_with(course_id, options, None) @pytest.mark.parametrize( @@ -338,28 +366,35 @@ def test_retrieve_course_completions_and_grades( @pytest.mark.django_db def test_retrieve_data_for_learning_path( patch_target: str, - function_to_test: Callable[[str, dict], list[int]], + function_to_test: Callable, learning_path_with_courses: LearningPath, users: list[User], ): - """Test retrieving data for a learning path.""" + """Test retrieving data for a learning path returns eligible users with step breakdown.""" with patch(patch_target) as mock_retrieve: options = {} - mock_retrieve.side_effect = ( - (users[i].id for i in (0, 1, 2, 4, 5)), # Users passing/completing course0 - (users[i].id for i in (0, 1, 2, 3, 4, 5)), # Users passing/completing course1 - (users[i].id for i in (0, 2, 3, 4, 5)), # Users passing/completing course2 - ) + + def make_results(user_indices: tuple) -> dict[int, dict]: + return {users[i].id: {'is_eligible': True} for i in user_indices} + + mock_retrieve.side_effect = [ + make_results((0, 1, 2, 4, 5)), # Users passing/completing course0 + make_results((0, 1, 2, 3, 4, 5)), # Users passing/completing course1 + make_results((0, 2, 3, 4, 5)), # Users passing/completing course2 + ] result = function_to_test(learning_path_with_courses.key, options) - assert sorted(result) == [users[0].id, users[2].id] + # users[0] and users[2] pass all 3 courses AND are enrolled+active in the learning path. + # users[4] passes all 3 but enrollment is inactive. users[5] is not enrolled at all. + eligible_ids = sorted(uid for uid, details in result.items() if details['is_eligible']) + assert eligible_ids == [users[0].id, users[2].id] assert mock_retrieve.call_count == 3 course_keys = [step.course_key for step in learning_path_with_courses.steps.all()] for i, course_key in enumerate(course_keys): call_args = mock_retrieve.call_args_list[i] - assert call_args[0] == (course_key, options) + assert call_args[0] == (course_key, options, None) @patch("learning_credentials.processors._retrieve_course_completions") @@ -369,6 +404,7 @@ def test_retrieve_data_for_learning_path_with_step_options( learning_path_with_courses: LearningPath, ): """Test retrieving data for a learning path with step-specific options.""" + mock_retrieve.return_value = {} course_keys = [step.course_key for step in learning_path_with_courses.steps.all()] options = { @@ -383,6 +419,6 @@ def test_retrieve_data_for_learning_path_with_step_options( retrieve_completions(learning_path_with_courses.key, options) assert mock_retrieve.call_count == 3 - assert mock_retrieve.call_args_list[0][0] == (course_keys[0], options["steps"][str(course_keys[0])]) - assert mock_retrieve.call_args_list[1][0] == (course_keys[1], options["steps"][str(course_keys[1])]) - assert mock_retrieve.call_args_list[2][0] == (course_keys[2], options) + assert mock_retrieve.call_args_list[0][0] == (course_keys[0], options["steps"][str(course_keys[0])], None) + assert mock_retrieve.call_args_list[1][0] == (course_keys[1], options["steps"][str(course_keys[1])], None) + assert mock_retrieve.call_args_list[2][0] == (course_keys[2], options, None) diff --git a/tests/test_views.py b/tests/test_views.py index dff7704..fb69681 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -12,6 +12,7 @@ from rest_framework.test import APIClient from learning_credentials.models import Credential, CredentialConfiguration +from test_utils.factories import UserFactory if TYPE_CHECKING: from django.contrib.auth.models import User @@ -261,3 +262,291 @@ def test_invalidated_credential_metadata(self, user: User, credential: Credentia assert response.status_code == status.HTTP_200_OK assert response.data['status'] == Credential.Status.INVALIDATED assert response.data['invalidation_reason'] == "Reissued due to name change." + + +@pytest.mark.django_db +class TestCredentialEligibilityView: + """Tests for the CredentialEligibilityView (GET eligibility and POST generation).""" + + def _make_get_request( + self, user: User | None, learning_context_key: LearningContextKey, **query_params + ) -> Response: + """Helper to make GET request to the eligibility endpoint.""" + client = _get_api_client(user) + url = reverse( + 'learning_credentials_api_v1:credential-eligibility', + kwargs={'learning_context_key': str(learning_context_key)}, + ) + return client.get(url, query_params) + + def _make_post_request( + self, user: User | None, learning_context_key: LearningContextKey, credential_type_id: int + ) -> Response: + """Helper to make POST request to the generation endpoint.""" + client = _get_api_client(user) + url = reverse( + 'learning_credentials_api_v1:credential-generation', + kwargs={ + 'learning_context_key': str(learning_context_key), + 'credential_type_id': credential_type_id, + }, + ) + return client.post(url) + + # --- GET tests --- + + def test_get_unauthenticated_returns_403(self, course_key: CourseKey): + """Test that unauthenticated users get 403.""" + response = self._make_get_request(None, course_key) + assert response.status_code == status.HTTP_403_FORBIDDEN + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_no_configurations(self, mock_enrollments: Mock, user: User, course_key: CourseKey): + """Test that an empty credentials list is returned when no configurations exist.""" + mock_enrollments.return_value = [user] + response = self._make_get_request(user, course_key) + + assert response.status_code == status.HTTP_200_OK + assert response.data == {'context_key': str(course_key), 'credentials': []} + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_eligible_user( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test eligibility response for an eligible user with no existing credential.""" + mock_enrollments.return_value = [user] + with patch.object(CredentialConfiguration, 'get_user_eligibility_details', return_value={'is_eligible': True}): + response = self._make_get_request(user, course_key) + + assert response.status_code == status.HTTP_200_OK + credentials = response.data['credentials'] + assert len(credentials) == 1 + cred = credentials[0] + assert cred['credential_type_id'] == mock_credential_config.credential_type.pk + assert cred['name'] == mock_credential_config.credential_type.name + assert cred['is_eligible'] is True + # None values should be stripped by serializer's to_representation. + assert 'existing_credential' not in cred + assert 'existing_credential_url' not in cred + + @pytest.mark.usefixtures('mock_credential_config') + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_strips_empty_dicts(self, mock_enrollments: Mock, user: User, course_key: CourseKey): + """Test that the serializer strips empty dict values from the response.""" + mock_enrollments.return_value = [user] + with patch.object( + CredentialConfiguration, + 'get_user_eligibility_details', + return_value={'is_eligible': False, 'current_grades': {}, 'steps': {}}, + ): + response = self._make_get_request(user, course_key) + + assert response.status_code == status.HTTP_200_OK + cred = response.data['credentials'][0] + assert cred['is_eligible'] is False + assert 'current_grades' not in cred + assert 'steps' not in cred + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_with_existing_credential( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, credential: Credential + ): + """Test that existing credential info is included in the response.""" + mock_enrollments.return_value = [user] + with patch.object(CredentialConfiguration, 'get_user_eligibility_details', return_value={'is_eligible': True}): + response = self._make_get_request(user, course_key) + + assert response.status_code == status.HTTP_200_OK + cred = response.data['credentials'][0] + assert str(cred['existing_credential']) == str(credential.uuid) + assert cred['existing_credential_url'] == credential.download_url + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_excludes_error_credentials( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test that credentials with ERROR status are not returned as existing.""" + mock_enrollments.return_value = [user] + Credential.objects.create( + user=user, + configuration=mock_credential_config, + status=Credential.Status.ERROR, + ) + with patch.object(CredentialConfiguration, 'get_user_eligibility_details', return_value={'is_eligible': True}): + response = self._make_get_request(user, course_key) + + assert response.status_code == status.HTTP_200_OK + cred = response.data['credentials'][0] + assert 'existing_credential' not in cred + + @pytest.mark.usefixtures('mock_credential_config') + def test_staff_get_for_other_user(self, staff_user: User, user: User, course_key: CourseKey): + """Test that staff can view eligibility for another user via username param.""" + with patch.object(CredentialConfiguration, 'get_user_eligibility_details', return_value={'is_eligible': False}): + response = self._make_get_request(staff_user, course_key, username=user.username) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data['credentials']) == 1 + + def test_non_staff_cannot_get_for_other_user(self, user: User, course_key: CourseKey): + """Test that non-staff users cannot view eligibility for another user.""" + other_user = UserFactory() + response = self._make_get_request(user, course_key, username=other_user.username) + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.usefixtures('mock_credential_config') + def test_get_user_not_found_returns_404(self, staff_user: User, course_key: CourseKey): + """Test that 404 is returned when the specified username doesn't exist.""" + response = self._make_get_request(staff_user, course_key, username='nonexistent_user') + assert response.status_code == status.HTTP_404_NOT_FOUND + + # --- POST tests --- + + def test_post_unauthenticated_returns_403( + self, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test that unauthenticated users get 403 on POST.""" + response = self._make_post_request(None, course_key, mock_credential_config.credential_type_id) + assert response.status_code == status.HTTP_403_FORBIDDEN + + @patch('learning_credentials.api.v1.views.generate_credential_for_user_task') + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_post_starts_generation( + self, + mock_enrollments: Mock, + mock_task: Mock, + user: User, + course_key: CourseKey, + mock_credential_config: CredentialConfiguration, + ): + """Test that POST triggers credential generation for an eligible user.""" + mock_enrollments.return_value = [user] + with patch.object(CredentialConfiguration, 'get_eligible_user_ids', return_value=[user.id]): + response = self._make_post_request(user, course_key, mock_credential_config.credential_type_id) + + assert response.status_code == status.HTTP_201_CREATED + assert response.data == {'detail': 'Credential generation started.'} + mock_task.delay.assert_called_once_with(mock_credential_config.id, user.id) + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_post_existing_credential_returns_409( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, credential: Credential + ): + """Test that POST returns 409 when user already has a valid credential.""" + mock_enrollments.return_value = [user] + response = self._make_post_request(user, course_key, credential.configuration.credential_type_id) + + assert response.status_code == status.HTTP_409_CONFLICT + assert 'already has a credential' in response.data['detail'] + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_post_ineligible_user_returns_400( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test that POST returns 400 when user is not eligible.""" + mock_enrollments.return_value = [user] + with patch.object(CredentialConfiguration, 'get_eligible_user_ids', return_value=[]): + response = self._make_post_request(user, course_key, mock_credential_config.credential_type_id) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert 'not eligible' in response.data['detail'] + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_post_config_not_found_returns_404(self, mock_enrollments: Mock, user: User, course_key: CourseKey): + """Test that POST returns 404 when credential configuration doesn't exist.""" + mock_enrollments.return_value = [user] + response = self._make_post_request(user, course_key, 99999) + assert response.status_code == status.HTTP_404_NOT_FOUND + + +@pytest.mark.django_db +class TestCredentialListView: + """Tests for the CredentialListView.""" + + def _make_request(self, user: User | None, **query_params) -> Response: + """Helper to make GET request to the credential list endpoint.""" + client = _get_api_client(user) + url = reverse('learning_credentials_api_v1:credential-list') + return client.get(url, query_params) + + def test_unauthenticated_returns_403(self): + """Test that unauthenticated users get 403.""" + response = self._make_request(None) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_list_own_credentials(self, user: User, credential: Credential): + """Test that a user can list their own credentials.""" + response = self._make_request(user) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data['credentials']) == 1 + assert str(response.data['credentials'][0]['credential_id']) == str(credential.uuid) + + def test_list_empty(self, user: User): + """Test that an empty list is returned when user has no credentials.""" + response = self._make_request(user) + + assert response.status_code == status.HTTP_200_OK + assert response.data == {'credentials': []} + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_filter_by_learning_context( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test filtering credentials by learning context key.""" + mock_enrollments.return_value = [user] + Credential.objects.create( + user=user, + configuration=mock_credential_config, + learning_context_key=course_key, + status=Credential.Status.AVAILABLE, + download_url='http://example.com/cred.pdf', + ) + response = self._make_request(user, learning_context_key=str(course_key)) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data['credentials']) == 1 + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_filter_by_learning_context_no_match( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test that filtering by a different learning context returns no credentials.""" + mock_enrollments.return_value = [user] + Credential.objects.create( + user=user, + configuration=mock_credential_config, + learning_context_key=course_key, + status=Credential.Status.AVAILABLE, + ) + response = self._make_request(user, learning_context_key='course-v1:Other+Course+Key') + + assert response.status_code == status.HTTP_200_OK + assert response.data == {'credentials': []} + + @pytest.mark.usefixtures('credential') + def test_non_staff_can_get_own_by_username(self, user: User): + """Test that a non-staff user can pass their own username.""" + response = self._make_request(user, username=user.username) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data['credentials']) == 1 + + def test_non_staff_cannot_view_other_user(self, user: User): + """Test that a non-staff user cannot view another user's credentials.""" + other_user = UserFactory() + response = self._make_request(user, username=other_user.username) + assert response.status_code == status.HTTP_403_FORBIDDEN + + @pytest.mark.usefixtures('credential') + def test_staff_view_other_user(self, staff_user: User, user: User): + """Test that staff can view another user's credentials.""" + response = self._make_request(staff_user, username=user.username) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data['credentials']) == 1 + + def test_username_not_found_returns_404(self, staff_user: User): + """Test that 404 is returned when the specified username doesn't exist.""" + response = self._make_request(staff_user, username='nonexistent_user') + assert response.status_code == status.HTTP_404_NOT_FOUND From 51c6170b7859179332facd654f0ab9ac3913dfb0 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 13 Feb 2026 22:01:33 +0100 Subject: [PATCH 02/16] temp: bump version to create a pre-release --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1ff57f2..eb5c3c2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "learning-credentials" -version = "0.5.0" +version = "0.5.1rc1" description = "A pluggable service for preparing Open edX credentials." dynamic = ["readme"] requires-python = ">=3.11" diff --git a/uv.lock b/uv.lock index 7818580..56a132e 100644 --- a/uv.lock +++ b/uv.lock @@ -1820,7 +1820,7 @@ wheels = [ [[package]] name = "learning-credentials" -version = "0.5.0" +version = "0.5.1rc1" source = { virtual = "." } dependencies = [ { name = "celery" }, From 774d0ec3032f22bf9bf9befad43b2de169e742b6 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 21:45:42 +0100 Subject: [PATCH 03/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/serializers.py | 1 + learning_credentials/api/v1/views.py | 1 + 2 files changed, 2 insertions(+) diff --git a/learning_credentials/api/v1/serializers.py b/learning_credentials/api/v1/serializers.py index ef55d47..e3f1bb2 100644 --- a/learning_credentials/api/v1/serializers.py +++ b/learning_credentials/api/v1/serializers.py @@ -46,6 +46,7 @@ class CredentialEligibilitySerializer(serializers.Serializer): credential_type_id = serializers.IntegerField() name = serializers.CharField() + is_generation_enabled = serializers.BooleanField() is_eligible = serializers.BooleanField() existing_credential = serializers.UUIDField(required=False, allow_null=True) existing_credential_url = serializers.URLField(required=False, allow_blank=True, allow_null=True) diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index e10c820..723ceff 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -182,6 +182,7 @@ def _get_eligibility_data(self, user: User, config: "CredentialConfiguration") - return { 'credential_type_id': config.credential_type.pk, 'name': config.credential_type.name, + 'is_generation_enabled': config.periodic_task.enabled, **progress_data, 'existing_credential': existing_credential.uuid if existing_credential else None, 'existing_credential_url': existing_credential.download_url if existing_credential else None, From 4d0b39b28a18d0c261b414eef11a8545f7f9bcae Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 21:45:54 +0100 Subject: [PATCH 04/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/views.py | 14 ++++++++++++ tests/test_views.py | 32 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 723ceff..6d1dfe2 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -198,6 +198,14 @@ def _get_eligibility_data(self, user: User, config: "CredentialConfiguration") - "or learning path key (path-v1:OpenedX+DemoX+DemoPath+Demo)" ), ), + apidocs.string_parameter( + "retrieval_func", + ParameterLocation.QUERY, + description=( + "Filter by credential type retrieval function " + "(e.g. learning_credentials.processors.retrieve_subsection_grades)." + ), + ), ], responses={ 200: CredentialEligibilityResponseSerializer, @@ -220,6 +228,8 @@ def get(self, request: "Request", learning_context_key: str) -> Response: **Query Parameters** - ``username`` (staff only): View eligibility for a specific user. + - ``retrieval_func``: Filter by credential type retrieval function + (e.g. ``learning_credentials.processors.retrieve_subsection_grades``). **Example Request** @@ -250,6 +260,10 @@ def get(self, request: "Request", learning_context_key: str) -> Response: learning_context_key=learning_context_key ).select_related('credential_type') + retrieval_func = request.query_params.get('retrieval_func') + if retrieval_func: + configurations = configurations.filter(credential_type__retrieval_func=retrieval_func) + eligibility_data = [self._get_eligibility_data(user, config) for config in configurations] response_data = { diff --git a/tests/test_views.py b/tests/test_views.py index fb69681..bd547b3 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -329,6 +329,38 @@ def test_get_eligible_user( assert 'existing_credential' not in cred assert 'existing_credential_url' not in cred + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_filters_by_retrieval_func( + self, + mock_enrollments: Mock, + user: User, + course_key: CourseKey, + mock_credential_config: CredentialConfiguration, + completion_config: CredentialConfiguration, + ): + """Test that the retrieval_func query parameter filters configurations.""" + mock_enrollments.return_value = [user] + with patch.object(CredentialConfiguration, 'get_user_eligibility_details', return_value={'is_eligible': False}): + response = self._make_get_request( + user, course_key, retrieval_func=mock_credential_config.credential_type.retrieval_func + ) + + assert response.status_code == status.HTTP_200_OK + credentials = response.data['credentials'] + assert len(credentials) == 1 + assert credentials[0]['credential_type_id'] == mock_credential_config.credential_type.pk + + @patch('learning_credentials.api.v1.permissions.get_course_enrollments') + def test_get_filter_by_retrieval_func_no_match( + self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration + ): + """Test that filtering by a non-matching retrieval_func returns no credentials.""" + mock_enrollments.return_value = [user] + response = self._make_get_request(user, course_key, retrieval_func='nonexistent.module.func') + + assert response.status_code == status.HTTP_200_OK + assert response.data == {'context_key': str(course_key), 'credentials': []} + @pytest.mark.usefixtures('mock_credential_config') @patch('learning_credentials.api.v1.permissions.get_course_enrollments') def test_get_strips_empty_dicts(self, mock_enrollments: Mock, user: User, course_key: CourseKey): From 6d338444637c824953b41f3cfad9ee660fa69eb6 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 21:46:29 +0100 Subject: [PATCH 05/16] fixup! temp: bump version to create a pre-release --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index eb5c3c2..2ae08da 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "learning-credentials" -version = "0.5.1rc1" +version = "0.5.1rc2" description = "A pluggable service for preparing Open edX credentials." dynamic = ["readme"] requires-python = ">=3.11" diff --git a/uv.lock b/uv.lock index 56a132e..6f736db 100644 --- a/uv.lock +++ b/uv.lock @@ -1820,7 +1820,7 @@ wheels = [ [[package]] name = "learning-credentials" -version = "0.5.1rc1" +version = "0.5.1rc2" source = { virtual = "." } dependencies = [ { name = "celery" }, From 7a4d162d3cfc1a1655fcb84b433ad0478a18ac4b Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 22:15:07 +0100 Subject: [PATCH 06/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/views.py | 74 ---------------------------- tests/test_views.py | 58 ---------------------- 2 files changed, 132 deletions(-) diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 6d1dfe2..5432b79 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -275,80 +275,6 @@ def get(self, request: "Request", learning_context_key: str) -> Response: serializer.is_valid(raise_exception=True) return Response(serializer.data) - @apidocs.schema( - parameters=[ - apidocs.string_parameter( - "learning_context_key", - ParameterLocation.PATH, - description="Learning context identifier (e.g. course-v1:OpenedX+DemoX+DemoCourse)", - ), - apidocs.parameter( - "credential_type_id", - ParameterLocation.PATH, - int, - description="ID of the credential type to generate", - ), - ], - responses={ - 201: "Credential generation started.", - 400: "User is not eligible for this credential.", - 403: "User is not authenticated.", - 404: "Learning context or credential type not found.", - 409: "User already has a valid credential of this type.", - }, - ) - def post(self, request: "Request", learning_context_key: str, credential_type_id: int) -> Response: - """ - Trigger credential generation for an eligible user. - - The user must be eligible and must not already have an existing valid credential of this type. - - **Query Parameters** - - - ``username`` (staff only): Trigger generation for a specific user. - - **Example Request** - - ``POST /api/learning_credentials/v1/eligibility/course-v1:OpenedX+DemoX+DemoCourse/1/`` - - **Example Response** - - .. code-block:: json - - { - "detail": "Credential generation started." - } - """ - username = request.query_params.get('username') - user = get_object_or_404(User, username=username) if username else request.user - - config = get_object_or_404( - CredentialConfiguration.objects.select_related('credential_type'), - learning_context_key=learning_context_key, - credential_type_id=credential_type_id, - ) - - existing_credential = ( - Credential.objects.filter(user_id=user.id, configuration=config) - .exclude(status=Credential.Status.ERROR) - .first() - ) - - if existing_credential: - return Response( - {"detail": "User already has a credential of this type."}, - status=status.HTTP_409_CONFLICT, - ) - - if not config.get_eligible_user_ids(user_id=user.id): - return Response( - {"detail": "User is not eligible for this credential."}, - status=status.HTTP_400_BAD_REQUEST, - ) - - generate_credential_for_user_task.delay(config.id, user.id) - return Response({"detail": "Credential generation started."}, status=status.HTTP_201_CREATED) - class CredentialListView(APIView): """ diff --git a/tests/test_views.py b/tests/test_views.py index bd547b3..814e9f9 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -432,64 +432,6 @@ def test_get_user_not_found_returns_404(self, staff_user: User, course_key: Cour response = self._make_get_request(staff_user, course_key, username='nonexistent_user') assert response.status_code == status.HTTP_404_NOT_FOUND - # --- POST tests --- - - def test_post_unauthenticated_returns_403( - self, course_key: CourseKey, mock_credential_config: CredentialConfiguration - ): - """Test that unauthenticated users get 403 on POST.""" - response = self._make_post_request(None, course_key, mock_credential_config.credential_type_id) - assert response.status_code == status.HTTP_403_FORBIDDEN - - @patch('learning_credentials.api.v1.views.generate_credential_for_user_task') - @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_post_starts_generation( - self, - mock_enrollments: Mock, - mock_task: Mock, - user: User, - course_key: CourseKey, - mock_credential_config: CredentialConfiguration, - ): - """Test that POST triggers credential generation for an eligible user.""" - mock_enrollments.return_value = [user] - with patch.object(CredentialConfiguration, 'get_eligible_user_ids', return_value=[user.id]): - response = self._make_post_request(user, course_key, mock_credential_config.credential_type_id) - - assert response.status_code == status.HTTP_201_CREATED - assert response.data == {'detail': 'Credential generation started.'} - mock_task.delay.assert_called_once_with(mock_credential_config.id, user.id) - - @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_post_existing_credential_returns_409( - self, mock_enrollments: Mock, user: User, course_key: CourseKey, credential: Credential - ): - """Test that POST returns 409 when user already has a valid credential.""" - mock_enrollments.return_value = [user] - response = self._make_post_request(user, course_key, credential.configuration.credential_type_id) - - assert response.status_code == status.HTTP_409_CONFLICT - assert 'already has a credential' in response.data['detail'] - - @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_post_ineligible_user_returns_400( - self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration - ): - """Test that POST returns 400 when user is not eligible.""" - mock_enrollments.return_value = [user] - with patch.object(CredentialConfiguration, 'get_eligible_user_ids', return_value=[]): - response = self._make_post_request(user, course_key, mock_credential_config.credential_type_id) - - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert 'not eligible' in response.data['detail'] - - @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_post_config_not_found_returns_404(self, mock_enrollments: Mock, user: User, course_key: CourseKey): - """Test that POST returns 404 when credential configuration doesn't exist.""" - mock_enrollments.return_value = [user] - response = self._make_post_request(user, course_key, 99999) - assert response.status_code == status.HTTP_404_NOT_FOUND - @pytest.mark.django_db class TestCredentialListView: From f5de9d695164ac3686b638259ee5eeb150be5f55 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 22:20:49 +0100 Subject: [PATCH 07/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/serializers.py | 32 +------- learning_credentials/api/v1/urls.py | 8 +- learning_credentials/api/v1/views.py | 88 +------------------- tests/test_views.py | 93 ---------------------- 4 files changed, 3 insertions(+), 218 deletions(-) diff --git a/learning_credentials/api/v1/serializers.py b/learning_credentials/api/v1/serializers.py index e3f1bb2..d324309 100644 --- a/learning_credentials/api/v1/serializers.py +++ b/learning_credentials/api/v1/serializers.py @@ -1,6 +1,6 @@ """API serializers for learning credentials.""" -from typing import Any, ClassVar +from typing import Any from rest_framework import serializers @@ -17,30 +17,6 @@ class Meta: fields = ('user_full_name', 'created', 'learning_context_name', 'status', 'invalidation_reason') -class CredentialModelSerializer(serializers.ModelSerializer): - """Serializer for Credential instances returned to authenticated users.""" - - credential_id = serializers.UUIDField(source='uuid', read_only=True) - credential_type = serializers.CharField(read_only=True) - context_key = serializers.CharField(source='learning_context_key', read_only=True) - created_date = serializers.DateTimeField(source='created', read_only=True) - download_url = serializers.URLField(read_only=True) - - class Meta: - """Serializer metadata.""" - - model = Credential - fields: ClassVar[list[str]] = [ - 'credential_id', - 'credential_type', - 'context_key', - 'status', - 'created_date', - 'download_url', - ] - read_only_fields: ClassVar[list[str]] = fields - - class CredentialEligibilitySerializer(serializers.Serializer): """Serializer for credential eligibility information with dynamic fields.""" @@ -70,9 +46,3 @@ class CredentialEligibilityResponseSerializer(serializers.Serializer): context_key = serializers.CharField() credentials = CredentialEligibilitySerializer(many=True) - - -class CredentialListResponseSerializer(serializers.Serializer): - """Serializer for credential list response.""" - - credentials = CredentialModelSerializer(many=True) diff --git a/learning_credentials/api/v1/urls.py b/learning_credentials/api/v1/urls.py index 0f1ed1e..833e8f0 100644 --- a/learning_credentials/api/v1/urls.py +++ b/learning_credentials/api/v1/urls.py @@ -2,12 +2,7 @@ from django.urls import path -from .views import ( - CredentialConfigurationCheckView, - CredentialEligibilityView, - CredentialListView, - CredentialMetadataView, -) +from .views import CredentialConfigurationCheckView, CredentialEligibilityView, CredentialMetadataView urlpatterns = [ path( @@ -26,5 +21,4 @@ CredentialEligibilityView.as_view(), name='credential-generation', ), - path('credentials/', CredentialListView.as_view(), name='credential-list'), ] diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 5432b79..edebec6 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -13,15 +13,9 @@ from rest_framework.views import APIView from learning_credentials.models import Credential, CredentialConfiguration -from learning_credentials.tasks import generate_credential_for_user_task from .permissions import CanAccessLearningContext, IsAdminOrSelf -from .serializers import ( - CredentialEligibilityResponseSerializer, - CredentialListResponseSerializer, - CredentialModelSerializer, - CredentialSerializer, -) +from .serializers import CredentialEligibilityResponseSerializer, CredentialSerializer if TYPE_CHECKING: from rest_framework.request import Request @@ -274,83 +268,3 @@ def get(self, request: "Request", learning_context_key: str) -> Response: serializer = CredentialEligibilityResponseSerializer(data=response_data) serializer.is_valid(raise_exception=True) return Response(serializer.data) - - -class CredentialListView(APIView): - """ - API view to list user credentials. - - Staff users can view credentials for any user via the ``username`` parameter. - Non-staff users can only view their own credentials. - Optionally filter by ``learning_context_key``. - """ - - def get_permissions(self) -> list: - """Return permissions, adding context access check only when filtering by learning context.""" - permission_classes = [IsAuthenticated, IsAdminOrSelf] - - if self.request.query_params.get('learning_context_key'): - permission_classes.append(CanAccessLearningContext) - - return [permission() for permission in permission_classes] - - @apidocs.schema( - parameters=[ - apidocs.string_parameter( - "learning_context_key", - ParameterLocation.QUERY, - description="Optional learning context to filter credentials.", - ), - apidocs.string_parameter( - "username", - ParameterLocation.QUERY, - description="Username to view credentials for (staff only).", - ), - ], - responses={ - 200: CredentialListResponseSerializer, - 403: "User is not authenticated or lacks permission.", - 404: "Specified user not found or learning context not accessible.", - }, - ) - def get(self, request: "Request") -> Response: - """ - Retrieve a list of credentials for the authenticated user or a specified user. - - **Query Parameters** - - - ``username`` (staff only): View credentials for a specific user. - - ``learning_context_key`` (optional): Filter credentials by learning context. - - **Example Request** - - ``GET /api/learning_credentials/v1/credentials/`` - - **Example Response** - - .. code-block:: json - - { - "credentials": [ - { - "credential_id": "123e4567-e89b-12d3-a456-426614174000", - "credential_type": "Certificate of Achievement", - "context_key": "course-v1:OpenedX+DemoX+DemoCourse", - "status": "available", - "created_date": "2024-08-20T10:30:00Z", - "download_url": "https://example.com/credentials/123e4567.pdf" - } - ] - } - """ - learning_context_key = request.query_params.get('learning_context_key') - username = request.query_params.get('username') - user = get_object_or_404(User, username=username) if username else request.user - - credentials_queryset = Credential.objects.filter(user_id=user.pk) - - if learning_context_key: - credentials_queryset = credentials_queryset.filter(learning_context_key=learning_context_key) - - credentials_data = CredentialModelSerializer(credentials_queryset, many=True).data - return Response({'credentials': credentials_data}) diff --git a/tests/test_views.py b/tests/test_views.py index 814e9f9..4dcc66a 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -431,96 +431,3 @@ def test_get_user_not_found_returns_404(self, staff_user: User, course_key: Cour """Test that 404 is returned when the specified username doesn't exist.""" response = self._make_get_request(staff_user, course_key, username='nonexistent_user') assert response.status_code == status.HTTP_404_NOT_FOUND - - -@pytest.mark.django_db -class TestCredentialListView: - """Tests for the CredentialListView.""" - - def _make_request(self, user: User | None, **query_params) -> Response: - """Helper to make GET request to the credential list endpoint.""" - client = _get_api_client(user) - url = reverse('learning_credentials_api_v1:credential-list') - return client.get(url, query_params) - - def test_unauthenticated_returns_403(self): - """Test that unauthenticated users get 403.""" - response = self._make_request(None) - assert response.status_code == status.HTTP_403_FORBIDDEN - - def test_list_own_credentials(self, user: User, credential: Credential): - """Test that a user can list their own credentials.""" - response = self._make_request(user) - - assert response.status_code == status.HTTP_200_OK - assert len(response.data['credentials']) == 1 - assert str(response.data['credentials'][0]['credential_id']) == str(credential.uuid) - - def test_list_empty(self, user: User): - """Test that an empty list is returned when user has no credentials.""" - response = self._make_request(user) - - assert response.status_code == status.HTTP_200_OK - assert response.data == {'credentials': []} - - @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_filter_by_learning_context( - self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration - ): - """Test filtering credentials by learning context key.""" - mock_enrollments.return_value = [user] - Credential.objects.create( - user=user, - configuration=mock_credential_config, - learning_context_key=course_key, - status=Credential.Status.AVAILABLE, - download_url='http://example.com/cred.pdf', - ) - response = self._make_request(user, learning_context_key=str(course_key)) - - assert response.status_code == status.HTTP_200_OK - assert len(response.data['credentials']) == 1 - - @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_filter_by_learning_context_no_match( - self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration - ): - """Test that filtering by a different learning context returns no credentials.""" - mock_enrollments.return_value = [user] - Credential.objects.create( - user=user, - configuration=mock_credential_config, - learning_context_key=course_key, - status=Credential.Status.AVAILABLE, - ) - response = self._make_request(user, learning_context_key='course-v1:Other+Course+Key') - - assert response.status_code == status.HTTP_200_OK - assert response.data == {'credentials': []} - - @pytest.mark.usefixtures('credential') - def test_non_staff_can_get_own_by_username(self, user: User): - """Test that a non-staff user can pass their own username.""" - response = self._make_request(user, username=user.username) - - assert response.status_code == status.HTTP_200_OK - assert len(response.data['credentials']) == 1 - - def test_non_staff_cannot_view_other_user(self, user: User): - """Test that a non-staff user cannot view another user's credentials.""" - other_user = UserFactory() - response = self._make_request(user, username=other_user.username) - assert response.status_code == status.HTTP_403_FORBIDDEN - - @pytest.mark.usefixtures('credential') - def test_staff_view_other_user(self, staff_user: User, user: User): - """Test that staff can view another user's credentials.""" - response = self._make_request(staff_user, username=user.username) - - assert response.status_code == status.HTTP_200_OK - assert len(response.data['credentials']) == 1 - - def test_username_not_found_returns_404(self, staff_user: User): - """Test that 404 is returned when the specified username doesn't exist.""" - response = self._make_request(staff_user, username='nonexistent_user') - assert response.status_code == status.HTTP_404_NOT_FOUND From 3b7f75dde71a0c5779e321fce177ce6d627ddf1c Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 22:29:41 +0100 Subject: [PATCH 08/16] fixup! feat: implement eligibility REST API endpoint --- CHANGELOG.rst | 11 +++++++++-- learning_credentials/api/v1/permissions.py | 4 +--- learning_credentials/api/v1/views.py | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8a6217f..1ffefb3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,13 +16,20 @@ Unreleased * -0.5.1 - 2026-02-13 +0.5.1 - 2026-03-17 ****************** Added ===== -* Eligibility REST API endpoint. +* Credential eligibility check endpoint (``GET /api/learning_credentials/v1/eligibility//``) + with detailed progress information. + +Changed +======= + +* Processor functions now return ``dict[int, dict[str, Any]]`` with detailed eligibility information instead of ``list[int]`` of eligible user IDs. +* Processor functions now accept an optional ``user_id`` parameter for single-user eligibility checks. 0.5.0 - 2026-01-29 ****************** diff --git a/learning_credentials/api/v1/permissions.py b/learning_credentials/api/v1/permissions.py index 8c7d942..564658f 100644 --- a/learning_credentials/api/v1/permissions.py +++ b/learning_credentials/api/v1/permissions.py @@ -31,9 +31,7 @@ def has_permission(self, request: "Request", view: "APIView") -> bool: # noqa: if request.user.is_staff: return True - username = request.query_params.get("username") if request.method == "GET" else request.data.get("username") - - if username: + if username := request.query_params.get("username"): return request.user.username == username return True diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index edebec6..1005275 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -26,7 +26,7 @@ class CredentialConfigurationCheckView(APIView): """API view to check if any credentials are configured for a specific learning context.""" - permission_classes = (IsAuthenticated, IsAdminOrSelf, CanAccessLearningContext) + permission_classes = (IsAuthenticated, CanAccessLearningContext) @apidocs.schema( parameters=[ From a22883bfab5590eafbd4394f76fc04c3f894be7f Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 23:00:41 +0100 Subject: [PATCH 09/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/views.py | 19 ++++++++---------- tests/test_views.py | 29 +++++----------------------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 1005275..8e9f1e4 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -160,18 +160,10 @@ class CredentialEligibilityView(APIView): permission_classes = (IsAuthenticated, IsAdminOrSelf, CanAccessLearningContext) - def _get_eligibility_data(self, user: User, config: "CredentialConfiguration") -> dict: + def _get_eligibility_data(self, user: User, config: "CredentialConfiguration", credentials: list) -> dict: """Calculate eligibility data for a credential configuration.""" progress_data = config.get_user_eligibility_details(user_id=user.id) - - existing_credential = ( - Credential.objects.filter( - user_id=user.id, - configuration=config, - ) - .exclude(status=Credential.Status.ERROR) - .first() - ) + existing_credential = next((cred for cred in credentials if cred.configuration_id == config.id), None) return { 'credential_type_id': config.credential_type.pk, @@ -258,7 +250,12 @@ def get(self, request: "Request", learning_context_key: str) -> Response: if retrieval_func: configurations = configurations.filter(credential_type__retrieval_func=retrieval_func) - eligibility_data = [self._get_eligibility_data(user, config) for config in configurations] + # Pre-fetch all credentials for this user and learning context to avoid N+1 queries in the loop below. + credentials = Credential.objects.filter(user_id=user.id, configuration__in=configurations).exclude( + status__in=[Credential.Status.ERROR, Credential.Status.INVALIDATED] + ) + + eligibility_data = [self._get_eligibility_data(user, config, credentials) for config in configurations] response_data = { 'context_key': learning_context_key, diff --git a/tests/test_views.py b/tests/test_views.py index 4dcc66a..9fa33f7 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -266,7 +266,7 @@ def test_invalidated_credential_metadata(self, user: User, credential: Credentia @pytest.mark.django_db class TestCredentialEligibilityView: - """Tests for the CredentialEligibilityView (GET eligibility and POST generation).""" + """Tests for the CredentialEligibilityView.""" def _make_get_request( self, user: User | None, learning_context_key: LearningContextKey, **query_params @@ -279,22 +279,6 @@ def _make_get_request( ) return client.get(url, query_params) - def _make_post_request( - self, user: User | None, learning_context_key: LearningContextKey, credential_type_id: int - ) -> Response: - """Helper to make POST request to the generation endpoint.""" - client = _get_api_client(user) - url = reverse( - 'learning_credentials_api_v1:credential-generation', - kwargs={ - 'learning_context_key': str(learning_context_key), - 'credential_type_id': credential_type_id, - }, - ) - return client.post(url) - - # --- GET tests --- - def test_get_unauthenticated_returns_403(self, course_key: CourseKey): """Test that unauthenticated users get 403.""" response = self._make_get_request(None, course_key) @@ -394,16 +378,13 @@ def test_get_with_existing_credential( assert cred['existing_credential_url'] == credential.download_url @patch('learning_credentials.api.v1.permissions.get_course_enrollments') - def test_get_excludes_error_credentials( + def test_get_excludes_error_and_invalidated_credentials( self, mock_enrollments: Mock, user: User, course_key: CourseKey, mock_credential_config: CredentialConfiguration ): - """Test that credentials with ERROR status are not returned as existing.""" + """Test that credentials with ERROR or INVALIDATED status are not returned as existing.""" mock_enrollments.return_value = [user] - Credential.objects.create( - user=user, - configuration=mock_credential_config, - status=Credential.Status.ERROR, - ) + mock_credential_config.credential_set.create(user=user, status=Credential.Status.ERROR) + mock_credential_config.credential_set.create(user=user, status=Credential.Status.INVALIDATED) with patch.object(CredentialConfiguration, 'get_user_eligibility_details', return_value={'is_eligible': True}): response = self._make_get_request(user, course_key) From 70695e46f2f9320b66b0037bf55b5f5609e35645 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Tue, 17 Mar 2026 22:31:19 +0100 Subject: [PATCH 10/16] fixup! temp: bump version to create a pre-release --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2ae08da..62e1360 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "learning-credentials" -version = "0.5.1rc2" +version = "0.5.1rc3" description = "A pluggable service for preparing Open edX credentials." dynamic = ["readme"] requires-python = ">=3.11" diff --git a/uv.lock b/uv.lock index 6f736db..adefafd 100644 --- a/uv.lock +++ b/uv.lock @@ -1820,7 +1820,7 @@ wheels = [ [[package]] name = "learning-credentials" -version = "0.5.1rc2" +version = "0.5.1rc3" source = { virtual = "." } dependencies = [ { name = "celery" }, From fd1ff19d065519c9929dcf96d0cfee9033531629 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 25 Mar 2026 18:03:28 +0100 Subject: [PATCH 11/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/urls.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/learning_credentials/api/v1/urls.py b/learning_credentials/api/v1/urls.py index 833e8f0..43cabc0 100644 --- a/learning_credentials/api/v1/urls.py +++ b/learning_credentials/api/v1/urls.py @@ -16,9 +16,4 @@ CredentialEligibilityView.as_view(), name='credential-eligibility', ), - path( - 'eligibility///', - CredentialEligibilityView.as_view(), - name='credential-generation', - ), ] From 00459c36311cd6a49722d697e8497daee6ad6e07 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 25 Mar 2026 18:33:05 +0100 Subject: [PATCH 12/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/processors.py | 21 +++++++++++++++------ tests/test_processors.py | 29 +++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/learning_credentials/processors.py b/learning_credentials/processors.py index 1e03226..027072c 100644 --- a/learning_credentials/processors.py +++ b/learning_credentials/processors.py @@ -308,7 +308,7 @@ def _prepare_request_to_completion_aggregator(course_id: CourseKey, query_params # HACK: Bypass the API permissions. staff_user = get_user_model().objects.filter(is_staff=True).first() - view._effective_user = staff_user # noqa: SLF001 + view.request.user = staff_user log.debug('Finished preparing the request for retrieving the completion.') return view @@ -324,10 +324,19 @@ def _retrieve_course_completions( required_completion = options.get('required_completion', 0.9) + # Map usernames to user IDs via enrolled users. + users = get_course_enrollments(course_id, user_id) + username_to_id = {user.username: user.id for user in users} + url = f'/completion-aggregator/v1/course/{course_id}/' # The API supports up to 10k results per page, but we limit it to 1k to avoid performance issues. query_params = {'page_size': 1000, 'page': 1} + if user_id and user_id in username_to_id.values(): + # If we are processing a single user, we can directly query their completion to get live data. + # This is because the Completion Aggregator API calculates stale completions for single user queries. + query_params['username'] = users[0].username + # TODO: Extract the logic of this view into an API. The current approach is very hacky. view = _prepare_request_to_completion_aggregator(course_id, query_params.copy(), url) completions: dict[str, float] = {} @@ -336,16 +345,16 @@ def _retrieve_course_completions( response = view.get(view.request, str(course_id)) log.debug(response.data) for res in response.data['results']: - completions[res['username']] = res['completion']['percent'] + try: + completions[res['username']] = res['completion']['percent'] + except KeyError: + # If we request completion for a single user, the API does not return the username in the response. + completions[users[0].username] = res['completion']['percent'] if not response.data['pagination']['next']: break query_params['page'] += 1 view = _prepare_request_to_completion_aggregator(course_id, query_params.copy(), url) - # Map usernames to user IDs via enrolled users. - users = get_course_enrollments(course_id, user_id) - username_to_id = {user.username: user.id for user in users} - results: dict[int, dict[str, Any]] = {} for username, current_completion in completions.items(): if username in username_to_id: diff --git a/tests/test_processors.py b/tests/test_processors.py index af4e70f..877f491 100644 --- a/tests/test_processors.py +++ b/tests/test_processors.py @@ -238,8 +238,7 @@ def test_prepare_request_to_completion_aggregator(): mock_view_class.assert_called_once() assert view.request.course_id == course_id - # noinspection PyUnresolvedReferences - assert view._effective_user is staff_user + assert view.request.user is staff_user assert isinstance(view, mock_view_class.return_value.__class__) # Create a QueryDict from the query_params dictionary. @@ -299,6 +298,32 @@ def test_retrieve_course_completions( mock_view_page2.get.assert_called_once_with(mock_view_page2.request, str(course_id)) +@patch('learning_credentials.processors._prepare_request_to_completion_aggregator') +@patch('learning_credentials.processors.get_course_enrollments') +def test_retrieve_course_completions_single_user( + mock_enrollments: Mock, mock_aggregator_request: Mock, course_key: CourseKey +): + """Test that single-user queries add username to params and handle missing username in response.""" + options = {'required_completion': 0.8} + user = Mock(username='user1', id=1) + mock_enrollments.return_value = [user] + + # The Completion Aggregator API omits the username for single-user queries. + completions = {'pagination': {'next': None}, 'results': [{'completion': {'percent': 0.95}}]} + mock_view = Mock() + mock_view.get.return_value.data = completions + mock_aggregator_request.return_value = mock_view + + result = retrieve_completions(course_key, options, user_id=user.id) + + assert result[user.id] == {'is_eligible': True, 'current_completion': 0.95, 'required_completion': 0.8} + mock_aggregator_request.assert_called_once_with( + course_key, + {'page_size': 1000, 'page': 1, 'username': user.username}, + f'/completion-aggregator/v1/course/{course_key}/', + ) + + @pytest.mark.parametrize( ('completion_results', 'grade_results', 'expected_eligible_ids'), [ From a9526c8e440cf2c581ad3d86b0660ca46948e518 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 25 Mar 2026 18:33:09 +0100 Subject: [PATCH 13/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/views.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 8e9f1e4..44431bc 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -1,6 +1,5 @@ """API views for Learning Credentials.""" -import logging from typing import TYPE_CHECKING import edx_api_doc_tools as apidocs @@ -20,8 +19,6 @@ if TYPE_CHECKING: from rest_framework.request import Request -logger = logging.getLogger(__name__) - class CredentialConfigurationCheckView(APIView): """API view to check if any credentials are configured for a specific learning context.""" From da84b4ff9bc94096dcc76f99eb1c9bf8e0ed6402 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 26 Mar 2026 21:32:26 +0100 Subject: [PATCH 14/16] fixup! feat: implement eligibility REST API endpoint --- learning_credentials/api/v1/views.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/learning_credentials/api/v1/views.py b/learning_credentials/api/v1/views.py index 44431bc..7b3d258 100644 --- a/learning_credentials/api/v1/views.py +++ b/learning_credentials/api/v1/views.py @@ -3,7 +3,7 @@ from typing import TYPE_CHECKING import edx_api_doc_tools as apidocs -from django.contrib.auth.models import User +from django.contrib.auth import get_user_model from django.shortcuts import get_object_or_404 from edx_api_doc_tools import ParameterLocation from rest_framework import status @@ -17,6 +17,7 @@ from .serializers import CredentialEligibilityResponseSerializer, CredentialSerializer if TYPE_CHECKING: + from django.contrib.auth.models import User from rest_framework.request import Request @@ -157,10 +158,12 @@ class CredentialEligibilityView(APIView): permission_classes = (IsAuthenticated, IsAdminOrSelf, CanAccessLearningContext) - def _get_eligibility_data(self, user: User, config: "CredentialConfiguration", credentials: list) -> dict: + def _get_eligibility_data( + self, user: "User", config: CredentialConfiguration, credentials_by_config_id: dict[int, Credential] + ) -> dict: """Calculate eligibility data for a credential configuration.""" progress_data = config.get_user_eligibility_details(user_id=user.id) - existing_credential = next((cred for cred in credentials if cred.configuration_id == config.id), None) + existing_credential = credentials_by_config_id.get(config.id) return { 'credential_type_id': config.credential_type.pk, @@ -237,11 +240,11 @@ def get(self, request: "Request", learning_context_key: str) -> Response: } """ username = request.query_params.get('username') - user = get_object_or_404(User, username=username) if username else request.user + user = get_object_or_404(get_user_model(), username=username) if username else request.user configurations = CredentialConfiguration.objects.filter( learning_context_key=learning_context_key - ).select_related('credential_type') + ).select_related('credential_type', 'periodic_task') retrieval_func = request.query_params.get('retrieval_func') if retrieval_func: @@ -251,8 +254,11 @@ def get(self, request: "Request", learning_context_key: str) -> Response: credentials = Credential.objects.filter(user_id=user.id, configuration__in=configurations).exclude( status__in=[Credential.Status.ERROR, Credential.Status.INVALIDATED] ) + credentials_by_config_id = {credential.configuration_id: credential for credential in credentials} - eligibility_data = [self._get_eligibility_data(user, config, credentials) for config in configurations] + eligibility_data = [ + self._get_eligibility_data(user, config, credentials_by_config_id) for config in configurations + ] response_data = { 'context_key': learning_context_key, From ba52f72caca419a530e033a1392a988b260ce040 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 26 Mar 2026 21:36:17 +0100 Subject: [PATCH 15/16] fixup! temp: bump version to create a pre-release --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 62e1360..c626569 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "learning-credentials" -version = "0.5.1rc3" +version = "0.5.1rc4" description = "A pluggable service for preparing Open edX credentials." dynamic = ["readme"] requires-python = ">=3.11" From 72724d1de2366038a35c6d2bad42b5a469be7b6e Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 26 Mar 2026 21:37:20 +0100 Subject: [PATCH 16/16] fixup! temp: bump version to create a pre-release --- uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index adefafd..6c1950e 100644 --- a/uv.lock +++ b/uv.lock @@ -1820,7 +1820,7 @@ wheels = [ [[package]] name = "learning-credentials" -version = "0.5.1rc3" +version = "0.5.1rc4" source = { virtual = "." } dependencies = [ { name = "celery" },