feat: implement eligibility REST API endpoint#136
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds new REST API endpoints for credential eligibility checking and generation, marking version 0.5.1rc1. The changes refactor the internal processor functions to return detailed eligibility information (as dictionaries) rather than simple lists of eligible user IDs, enabling more informative API responses. The PR introduces three new endpoints: one for checking eligibility with detailed progress information, one for triggering credential generation, and one for listing user credentials.
Changes:
- Refactored processor functions to return
dict[int, dict[str, Any]]with detailed eligibility and progress information instead oflist[int] - Added credential eligibility check endpoint (GET) with detailed progress data including current grades, completion percentages, and existing credential information
- Added credential generation endpoint (POST) for eligible users to request credential generation
- Added credential list endpoint (GET) with optional filtering by learning context
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Updated version lock to 0.5.1rc1 |
| pyproject.toml | Bumped version to 0.5.1rc1 |
| CHANGELOG.rst | Added changelog entry for 0.5.1 release |
| learning_credentials/processors.py | Refactored all processor functions to return detailed dict results instead of user ID lists; added user_id parameter for single-user queries |
| learning_credentials/models.py | Added _call_retrieval_func and get_user_eligibility_details methods; updated get_eligible_user_ids to extract eligible IDs from detailed results |
| learning_credentials/api/v1/views.py | Added CredentialEligibilityView for GET/POST eligibility operations and CredentialListView for listing credentials; updated CredentialConfigurationCheckView permissions |
| learning_credentials/api/v1/serializers.py | Added CredentialModelSerializer, CredentialEligibilitySerializer, CredentialEligibilityResponseSerializer, and CredentialListResponseSerializer |
| learning_credentials/api/v1/permissions.py | Added IsAdminOrSelf permission class for username-based access control |
| learning_credentials/api/v1/urls.py | Added URL patterns for eligibility check, credential generation, and credential list endpoints |
| tests/test_views.py | Added comprehensive test suites for CredentialEligibilityView and CredentialListView |
| tests/test_processors.py | Updated tests to match new processor return type and added tests for detailed result structure |
| tests/test_models.py | Added tests for get_user_eligibility_details method |
| tests/conftest.py | Updated mock retrieval function to return dict format with user_id parameter support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8388035 to
6d8a532
Compare
|
Note for future reference: credential list view has been removed in 8d770c1 because we do not need it at the moment. |
d8a6d98 to
e693087
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
10d29d5 to
da84b4f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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'] |
There was a problem hiding this comment.
Type annotations are inconsistent here: required_grades is annotated as dict[str, int] but values are treated as floats (multiplied by 100 from a [0,1] float input) and then passed to functions that expect dict[str, float]. Adjust the annotation (and/or intermediate variable typing) to dict[str, float] to match runtime behavior.
| required_grades: dict[str, int] = options['required_grades'] | |
| required_grades: dict[str, float] = options['required_grades'] |
| 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. |
There was a problem hiding this comment.
This class docstring claims a POST endpoint exists (“Triggers credential generation for an eligible user”), but the view currently only implements get(). Either implement post() or remove the POST description to avoid advertising a non-existent API surface.
| 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. | |
| API view for credential eligibility checking. | |
| **GET**: Returns detailed eligibility info for all configured credentials in a learning context. |
| "Filter by credential type retrieval function " | ||
| "(e.g. learning_credentials.processors.retrieve_subsection_grades)." | ||
| ), | ||
| ), |
There was a problem hiding this comment.
The endpoint supports a username query param (and the docstring documents it), but the OpenAPI schema parameters listed here only include learning_context_key and retrieval_func. Add the username query parameter to the @apidocs.schema(parameters=...) so generated docs match actual behavior/permissions.
| ), | |
| ), | |
| apidocs.string_parameter( | |
| "username", | |
| ParameterLocation.QUERY, | |
| description=( | |
| "Operate on behalf of the specified user. " | |
| "Staff users may specify any username; non-staff users are limited to their own." | |
| ), | |
| ), |
|
|
||
| * | ||
|
|
||
| 0.5.1 - 2026-03-17 |
There was a problem hiding this comment.
The changelog entry is for version 0.5.1, but the project/package version in pyproject.toml/uv.lock is 0.5.1rc4. Align the changelog heading with the actual version being released (or bump the package version to match) to avoid publishing inconsistent release metadata.
| 0.5.1 - 2026-03-17 | |
| 0.5.1rc4 - 2026-03-17 |
| [project] | ||
| name = "learning-credentials" | ||
| version = "0.5.0" | ||
| version = "0.5.1rc4" |
There was a problem hiding this comment.
version is set to 0.5.1rc4 here, but CHANGELOG.rst introduces a 0.5.1 section. Please ensure the package version and changelog version stay in sync (either use 0.5.1rc4 in the changelog, or bump the project version to 0.5.1).
| version = "0.5.1rc4" | |
| version = "0.5.1" |
| 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. |
There was a problem hiding this comment.
The module docstring still says processors "retrieve the IDs of the users that meet the criteria", but processors now return a dict of per-user progress (including ineligible users) per the new contract below. Update/remove that sentence so the documentation matches the actual return type/behavior.
| 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. | |
| The functions prefixed with `retrieve_` are automatically detected by the admin page and are used to compute | |
| per-user progress for the credential type, following the contract described below. | |
| All processors return ``dict[int, dict[str, Any]]`` — a mapping from user ID to detailed progress info for all | |
| relevant users, including those who are not eligible. Each user's dict always includes an ``is_eligible`` boolean. |
Merge checklist:
Check off if complete or not applicable: