Skip to content

feat: implement eligibility REST API endpoint#136

Open
Agrendalath wants to merge 16 commits intomainfrom
agrendalath/bb-9902-generation-api
Open

feat: implement eligibility REST API endpoint#136
Agrendalath wants to merge 16 commits intomainfrom
agrendalath/bb-9902-generation-api

Conversation

@Agrendalath
Copy link
Copy Markdown
Member

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets

@Agrendalath Agrendalath requested a review from Copilot February 13, 2026 21:01
@Agrendalath Agrendalath self-assigned this Feb 13, 2026
@Agrendalath Agrendalath changed the title Agrendalath/bb 9902 generation api feat: implement eligibility REST API endpoint Feb 13, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 of list[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.

@Agrendalath Agrendalath force-pushed the agrendalath/bb-9902-generation-api branch 2 times, most recently from 8388035 to 6d8a532 Compare March 16, 2026 15:47
@Agrendalath
Copy link
Copy Markdown
Member Author

Note for future reference: credential list view has been removed in 8d770c1 because we do not need it at the moment.

@Agrendalath Agrendalath force-pushed the agrendalath/bb-9902-generation-api branch 2 times, most recently from d8a6d98 to e693087 Compare March 17, 2026 22:00
@Agrendalath Agrendalath requested a review from Copilot March 17, 2026 22:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Agrendalath Agrendalath force-pushed the agrendalath/bb-9902-generation-api branch from 10d29d5 to da84b4f Compare March 26, 2026 20:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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']
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
required_grades: dict[str, int] = options['required_grades']
required_grades: dict[str, float] = options['required_grades']

Copilot uses AI. Check for mistakes.
Comment on lines +151 to +154
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.
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.

Copilot uses AI. Check for mistakes.
"Filter by credential type retrieval function "
"(e.g. learning_credentials.processors.retrieve_subsection_grades)."
),
),
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
),
),
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."
),
),

Copilot uses AI. Check for mistakes.

*

0.5.1 - 2026-03-17
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
0.5.1 - 2026-03-17
0.5.1rc4 - 2026-03-17

Copilot uses AI. Check for mistakes.
[project]
name = "learning-credentials"
version = "0.5.0"
version = "0.5.1rc4"
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
version = "0.5.1rc4"
version = "0.5.1"

Copilot uses AI. Check for mistakes.
Comment on lines 4 to +8
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.
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants