Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: fix exception handling in program cert revocation

Browse Source 
To determine whether or not we need to  revoke any program certificates, we first run get_revokable_program_uuids. This calls get_certified_programs, which calls get_credentials, which uses the OpenEdx Core  utility method get_api_data. get_api_data makes the API call inside a try block, does raise_for_status  also inside the try block, and then catches the exception, logs it, and returns an empty result to the caller.

This means that on a failure to call the credentials API, get_credentials can’t  tell the difference between a failure to hit the API (because credentials is, as it sometimes is during a notify_programs run, overloaded), or a learner with no program certificates. In this particular case, this is absolute failure, incorrect behavior.

* Adds a new flag, `raise_on_error`  which will make `get_api_data` log the exception and then re-raise the HTTPError,  defaulting to false in order to avoid changing the behavior on any other callers
* Also: my editor reformatted all of the touched files to our modern code standards, and it seemed appropriate to let it do that.
* Also: added type hints in some cases, because they helped me write the code and debug. Our test suite definitely  reports mypy  results on type errors so we are verifying that hints are correct.

FIXES: APER-3146
This commit is contained in:
Deborah Kaplan
2024-01-26 22:20:30 +00:00
parent 5c58eb47bd
commit ae3ce9c498
5 changed files with 840 additions and 488 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
openedx/core/djangoapps/credentials/utils.py
View File

@@ -1,5 +1,6 @@
"""Helper functions for working with Credentials."""
import logging
from typing import Dict, List
from urllib.parse import urljoin
import requests
@@ -42,7 +43,7 @@ def get_credentials_api_client(user):
Arguments:
user (User): The user to authenticate as when requesting credentials.
"""
scopes = ['email', 'profile', 'user_id']
scopes = ["email", "profile", "user_id"]
jwt = create_jwt_for_user(user, scopes=scopes)
client = requests.Session()
@@ -65,7 +66,12 @@ def get_credentials_api_base_url(org=None):
return url
def get_credentials(user, program_uuid=None, credential_type=None):
def get_credentials(
user: User,
program_uuid: str = None,
credential_type: str = None,
raise_on_error: bool = False,
) -> List[Dict]:
"""
Given a user, get credentials earned from the credentials service.
@@ -75,6 +81,7 @@ def get_credentials(user, program_uuid=None, credential_type=None):
Keyword Arguments:
program_uuid (str): UUID of the program whose credential to retrieve.
credential_type (str): Which type of credentials to return (course-run or program)
raise_on_error (bool): Reraise errors back to the caller, instead if returning empty results.
Returns:
list of dict, representing credentials returned by the Credentials
@@ -82,31 +89,38 @@ def get_credentials(user, program_uuid=None, credential_type=None):
"""
credential_configuration = CredentialsApiConfig.current()
querystring = {'username': user.username, 'status': 'awarded', 'only_visible': 'True'}
querystring = {
"username": user.username,
"status": "awarded",
"only_visible": "True",
}
if program_uuid:
querystring['program_uuid'] = program_uuid
querystring["program_uuid"] = program_uuid
if credential_type:
querystring['type'] = credential_type
querystring["type"] = credential_type
# Bypass caching for staff users, who may be generating credentials and
# want to see them displayed immediately.
use_cache = credential_configuration.is_cache_enabled and not user.is_staff
cache_key = f'{credential_configuration.CACHE_KEY}.{user.username}' if use_cache else None
cache_key = (
f"{credential_configuration.CACHE_KEY}.{user.username}" if use_cache else None
)
if cache_key and program_uuid:
cache_key = f'{cache_key}.{program_uuid}'
cache_key = f"{cache_key}.{program_uuid}"
api_client = get_credentials_api_client(user)
base_api_url = get_credentials_api_base_url()
return get_api_data(
credential_configuration,
'credentials',
"credentials",
api_client=api_client,
base_api_url=base_api_url,
querystring=querystring,
cache_key=cache_key
cache_key=cache_key,
raise_on_error=raise_on_error,
)
@@ -122,11 +136,13 @@ def get_courses_completion_status(username, course_run_ids):
"""
credential_configuration = CredentialsApiConfig.current()
if not credential_configuration.enabled:
log.warning('%s configuration is disabled.', credential_configuration.API_NAME)
log.warning("%s configuration is disabled.", credential_configuration.API_NAME)
return [], False
completion_status_url = (f'{settings.CREDENTIALS_INTERNAL_SERVICE_URL}/api'
'/credentials/v1/learner_cert_status/')
completion_status_url = (
f"{settings.CREDENTIALS_INTERNAL_SERVICE_URL}/api"
"/credentials/v1/learner_cert_status/"
)
try:
api_client = get_credentials_api_client(
User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME)
@@ -134,30 +150,36 @@ def get_courses_completion_status(username, course_run_ids):
api_response = api_client.post(
completion_status_url,
json={
'username': username,
'course_runs': course_run_ids,
}
"username": username,
"course_runs": course_run_ids,
},
)
api_response.raise_for_status()
course_completion_response = api_response.json()
except Exception as exc: # pylint: disable=broad-except
log.exception("An unexpected error occurred while reqeusting course completion statuses "
"for user [%s] for course_run_ids [%s] with exc [%s]:",
username,
course_run_ids,
exc
)
log.exception(
"An unexpected error occurred while reqeusting course completion statuses "
"for user [%s] for course_run_ids [%s] with exc [%s]:",
username,
course_run_ids,
exc,
)
return [], True
log.info("Course completion status response for user [%s] for course_run_ids [%s] is [%s]",
username,
course_run_ids,
course_completion_response)
log.info(
"Course completion status response for user [%s] for course_run_ids [%s] is [%s]",
username,
course_run_ids,
course_completion_response,
)
# Yes, This is course_credentials_data. The key is named status but
# it contains all the courses data from credentials.
course_credentials_data = course_completion_response.get('status', [])
course_credentials_data = course_completion_response.get("status", [])
if course_credentials_data is not None:
filtered_records = [course_data['course_run']['key'] for course_data in course_credentials_data if
course_data['course_run']['key'] in course_run_ids and
course_data['status'] == settings.CREDENTIALS_COURSE_COMPLETION_STATE]
filtered_records = [
course_data["course_run"]["key"]
for course_data in course_credentials_data
if course_data["course_run"]["key"] in course_run_ids
and course_data["status"] == settings.CREDENTIALS_COURSE_COMPLETION_STATE
]
return filtered_records, False
return [], False

346
openedx/core/djangoapps/programs/tasks.py
View File

@@ -1,6 +1,7 @@
"""
This file contains celery tasks for programs-related functionality.
"""
from typing import Dict, List
from urllib.parse import urljoin
from celery import shared_task
@@ -13,7 +14,6 @@ from django.core.exceptions import ObjectDoesNotExist
from edx_django_utils.monitoring import set_code_owner_attribute
from opaque_keys.edx.keys import CourseKey
from requests.exceptions import HTTPError
from xmodule.data import CertificatesDisplayBehaviors
from common.djangoapps.course_modes.models import CourseMode
from lms.djangoapps.certificates.api import available_date_for_certificate
@@ -27,6 +27,7 @@ from openedx.core.djangoapps.credentials.utils import (
)
from openedx.core.djangoapps.programs.utils import ProgramProgressMeter
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from xmodule.data import CertificatesDisplayBehaviors
User = get_user_model()
@@ -37,9 +38,9 @@ LOGGER = get_task_logger(__name__)
# unwanted behavior: infinite retries.
MAX_RETRIES = 11
PROGRAM_CERTIFICATE = 'program'
COURSE_CERTIFICATE = 'course-run'
DATE_FORMAT = '%Y-%m-%dT%H:%M:%SZ'
PROGRAM_CERTIFICATE = "program"
COURSE_CERTIFICATE = "course-run"
DATE_FORMAT = "%Y-%m-%dT%H:%M:%SZ"
def get_completed_programs(site, student):
@@ -77,22 +78,28 @@ def get_inverted_programs(student):
return inverted_programs
def get_certified_programs(student):
def get_certified_programs(student: User, raise_on_error: bool = False) -> List[str]:
"""
Find the UUIDs of all the programs for which the student has already been awarded
a certificate.
Args:
student:
User object representing the student
student: User object representing the student
Keyword Arguments:
raise_on_error (bool): Reraise errors back to the caller, instead if returning empty results.
Returns:
str[]: UUIDs of the programs for which the student has been awarded a certificate
"""
certified_programs = []
for credential in get_credentials(student, credential_type='program'):
certified_programs.append(credential['credential']['program_uuid'])
for credential in get_credentials(
student,
credential_type="program",
raise_on_error=raise_on_error,
):
certified_programs.append(credential["credential"]["program_uuid"])
return certified_programs
@@ -118,26 +125,22 @@ def award_program_certificate(client, user, program_uuid, visible_date):
response = client.post(
api_url,
json={
'username': user.username,
'lms_user_id': user.id,
'credential': {
'type': PROGRAM_CERTIFICATE,
'program_uuid': program_uuid
},
'attributes': [
{
'name': 'visible_date',
'value': visible_date.strftime(DATE_FORMAT)
}
]
}
"username": user.username,
"lms_user_id": user.id,
"credential": {"type": PROGRAM_CERTIFICATE, "program_uuid": program_uuid},
"attributes": [
{"name": "visible_date", "value": visible_date.strftime(DATE_FORMAT)}
],
},
)
response.raise_for_status()
@shared_task(bind=True, ignore_result=True)
@set_code_owner_attribute
def award_program_certificates(self, username): # lint-amnesty, pylint: disable=too-many-statements
def award_program_certificates(
self, username
): # lint-amnesty, pylint: disable=too-many-statements
"""
This task is designed to be called whenever a student's completion status
changes with respect to one or more courses (primarily, when a course
@@ -161,41 +164,42 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
None
"""
def _retry_with_custom_exception(username, reason, countdown):
exception = MaxRetriesExceededError(
f"Failed to award program certificate for user {username}. Reason: {reason}"
)
return self.retry(
exc=exception,
countdown=countdown,
max_retries=MAX_RETRIES
)
return self.retry(exc=exception, countdown=countdown, max_retries=MAX_RETRIES)
LOGGER.info(f"Running task award_program_certificates for username {username}")
programs_without_certificates = configuration_helpers.get_value('programs_without_certificates', [])
programs_without_certificates = configuration_helpers.get_value(
"programs_without_certificates", []
)
if programs_without_certificates:
if str(programs_without_certificates[0]).lower() == "all":
# this check will prevent unnecessary logging for partners without program certificates
return
countdown = 2 ** self.request.retries
countdown = 2**self.request.retries
# If the credentials config model is disabled for this
# feature, it may indicate a condition where processing of such tasks
# has been temporarily disabled. Since this is a recoverable situation,
# mark this task for retry instead of failing it altogether.
if not CredentialsApiConfig.current().is_learner_issuance_enabled:
error_msg = (
"Task award_program_certificates cannot be executed when credentials issuance is disabled in API config"
)
error_msg = "Task award_program_certificates cannot be executed when credentials issuance is disabled in API config"
LOGGER.warning(error_msg)
raise _retry_with_custom_exception(username=username, reason=error_msg, countdown=countdown)
raise _retry_with_custom_exception(
username=username, reason=error_msg, countdown=countdown
)
try:
try:
student = User.objects.get(username=username)
except User.DoesNotExist:
LOGGER.exception(f"Task award_program_certificates was called with invalid username {username}")
LOGGER.exception(
f"Task award_program_certificates was called with invalid username {username}"
)
# Don't retry for this case - just conclude the task.
return
completed_programs = {}
@@ -204,7 +208,9 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
if not completed_programs:
# No reason to continue beyond this point unless/until this
# task gets updated to support revocation of program certs.
LOGGER.info(f"Task award_program_certificates was called for user {username} with no completed programs")
LOGGER.info(
f"Task award_program_certificates was called for user {username} with no completed programs"
)
return
# Determine which program certificates the user has already been awarded, if any.
@@ -212,12 +218,16 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
# we will skip all the programs which have already been awarded and we want to skip the programs
# which are exit in site configuration in 'programs_without_certificates' list.
awarded_and_skipped_program_uuids = list(set(existing_program_uuids + list(programs_without_certificates)))
awarded_and_skipped_program_uuids = list(
set(existing_program_uuids + list(programs_without_certificates))
)
except Exception as exc:
error_msg = f"Failed to determine program certificates to be awarded for user {username}. {exc}"
LOGGER.exception(error_msg)
raise _retry_with_custom_exception(username=username, reason=error_msg, countdown=countdown) from exc
raise _retry_with_custom_exception(
username=username, reason=error_msg, countdown=countdown
) from exc
# For each completed program for which the student doesn't already have a
# certificate, award one now.
@@ -225,7 +235,9 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
# This logic is important, because we will retry the whole task if awarding any particular program cert fails.
#
# N.B. the list is sorted to facilitate deterministic ordering, e.g. for tests.
new_program_uuids = sorted(list(set(completed_programs.keys()) - set(awarded_and_skipped_program_uuids)))
new_program_uuids = sorted(
list(set(completed_programs.keys()) - set(awarded_and_skipped_program_uuids))
)
if new_program_uuids:
try:
credentials_client = get_credentials_api_client(
@@ -235,20 +247,28 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
error_msg = "Failed to create a credentials API client to award program certificates"
LOGGER.exception(error_msg)
# Retry because a misconfiguration could be fixed
raise _retry_with_custom_exception(username=username, reason=error_msg, countdown=countdown) from exc
raise _retry_with_custom_exception(
username=username, reason=error_msg, countdown=countdown
) from exc
failed_program_certificate_award_attempts = []
for program_uuid in new_program_uuids:
visible_date = completed_programs[program_uuid]
try:
LOGGER.info(f"Visible date for user {username} : program {program_uuid} is {visible_date}")
award_program_certificate(credentials_client, student, program_uuid, visible_date)
LOGGER.info(f"Awarded certificate for program {program_uuid} to user {username}")
LOGGER.info(
f"Visible date for user {username} : program {program_uuid} is {visible_date}"
)
award_program_certificate(
credentials_client, student, program_uuid, visible_date
)
LOGGER.info(
f"Awarded certificate for program {program_uuid} to user {username}"
)
except HTTPError as exc:
if exc.response.status_code == 404:
LOGGER.exception(
f"Certificate for program {program_uuid} could not be found. " +
f"Unable to award certificate to user {username}. The program might not be configured."
f"Certificate for program {program_uuid} could not be found. "
+ f"Unable to award certificate to user {username}. The program might not be configured."
)
elif exc.response.status_code == 429:
rate_limit_countdown = 60
@@ -261,7 +281,7 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
raise _retry_with_custom_exception(
username=username,
reason=error_msg,
countdown=rate_limit_countdown
countdown=rate_limit_countdown,
) from exc
else:
LOGGER.exception(
@@ -270,12 +290,16 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
)
except Exception: # pylint: disable=broad-except
# keep trying to award other certs, but retry the whole task to fix any missing entries
LOGGER.exception(f"Failed to award certificate for program {program_uuid} to user {username}.")
LOGGER.exception(
f"Failed to award certificate for program {program_uuid} to user {username}."
)
failed_program_certificate_award_attempts.append(program_uuid)
if failed_program_certificate_award_attempts:
# N.B. This logic assumes that this task is idempotent
LOGGER.info(f"Retrying task to award failed certificates to user {username}")
LOGGER.info(
f"Retrying task to award failed certificates to user {username}"
)
# The error message may change on each reattempt but will never be raised until
# the max number of retries have been exceeded. It is unlikely that this list
# will change by the time it reaches its maximimum number of attempts.
@@ -283,14 +307,20 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable
f"Failed to award certificate for user {username} "
f"for programs {failed_program_certificate_award_attempts}"
)
raise _retry_with_custom_exception(username=username, reason=error_msg, countdown=countdown)
raise _retry_with_custom_exception(
username=username, reason=error_msg, countdown=countdown
)
else:
LOGGER.info(f"User {username} is not eligible for any new program certificates")
LOGGER.info(f"Successfully completed the task award_program_certificates for username {username}")
LOGGER.info(
f"Successfully completed the task award_program_certificates for username {username}"
)
def post_course_certificate_configuration(client, cert_config, certificate_available_date=None):
def post_course_certificate_configuration(
client, cert_config, certificate_available_date=None
):
"""
Make a POST request to the Credentials IDA's `course_certificates` endpoint (/api/v2/course_certificates/). This
endpoint manages the course certificate configurations within the Credentials IDA.
@@ -303,16 +333,18 @@ def post_course_certificate_configuration(client, cert_config, certificate_avail
in the form of an ISO 8601 DateTime String.
"""
credentials_api_base_url = get_credentials_api_base_url()
credentials_api_url = urljoin(f"{credentials_api_base_url}/", "course_certificates/")
credentials_api_url = urljoin(
f"{credentials_api_base_url}/", "course_certificates/"
)
response = client.post(
credentials_api_url,
json={
"course_id": cert_config['course_id'],
"certificate_type": cert_config['mode'],
"course_id": cert_config["course_id"],
"certificate_type": cert_config["mode"],
"certificate_available_date": certificate_available_date,
"is_active": True
}
"is_active": True,
},
)
# Sometimes helpful error context is swallowed when calling `raise_for_status()`. We try to print out any additional
@@ -328,7 +360,9 @@ def post_course_certificate_configuration(client, cert_config, certificate_avail
response.raise_for_status()
def post_course_certificate(client, username, certificate, visible_date, date_override=None, org=None):
def post_course_certificate(
client, username, certificate, visible_date, date_override=None, org=None
):
"""
POST a certificate that has been updated to Credentials
"""
@@ -338,21 +372,22 @@ def post_course_certificate(client, username, certificate, visible_date, date_ov
response = client.post(
api_url,
json={
'username': username,
'status': 'awarded' if certificate.is_valid() else 'revoked', # Only need the two options at this time
'credential': {
'course_run_key': str(certificate.course_id),
'mode': certificate.mode,
'type': COURSE_CERTIFICATE,
"username": username,
"status": "awarded"
if certificate.is_valid()
else "revoked", # Only need the two options at this time
"credential": {
"course_run_key": str(certificate.course_id),
"mode": certificate.mode,
"type": COURSE_CERTIFICATE,
},
'date_override': {'date': date_override.strftime(DATE_FORMAT)} if date_override else None,
'attributes': [
{
'name': 'visible_date',
'value': visible_date.strftime(DATE_FORMAT)
}
]
}
"date_override": {"date": date_override.strftime(DATE_FORMAT)}
if date_override
else None,
"attributes": [
{"name": "visible_date", "value": visible_date.strftime(DATE_FORMAT)}
],
},
)
response.raise_for_status()
@@ -361,9 +396,7 @@ def post_course_certificate(client, username, certificate, visible_date, date_ov
@shared_task(bind=True, ignore_result=True)
@set_code_owner_attribute
def update_credentials_course_certificate_configuration_available_date(
self,
course_key,
certificate_available_date=None
self, course_key, certificate_available_date=None
):
"""
This task will update the CourseCertificate configuration's available date
@@ -384,10 +417,14 @@ def update_credentials_course_certificate_configuration_available_date(
course_key = str(course_key)
course_modes = CourseMode.objects.filter(course_id=course_key)
# There should only ever be one certificate relevant mode per course run
modes = [mode.slug for mode in course_modes if mode.slug in CourseMode.CERTIFICATE_RELEVANT_MODES]
modes = [
mode.slug
for mode in course_modes
if mode.slug in CourseMode.CERTIFICATE_RELEVANT_MODES
]
if len(modes) != 1:
LOGGER.exception(
f'Either course {course_key} has no certificate mode or multiple modes. Task failed.'
f"Either course {course_key} has no certificate mode or multiple modes. Task failed."
)
return
@@ -395,13 +432,13 @@ def update_credentials_course_certificate_configuration_available_date(
User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME),
)
cert_config = {
'course_id': course_key,
'mode': modes[0],
"course_id": course_key,
"mode": modes[0],
}
post_course_certificate_configuration(
client=credentials_client,
cert_config=cert_config,
certificate_available_date=certificate_available_date
certificate_available_date=certificate_available_date,
)
@@ -421,34 +458,29 @@ def award_course_certificate(self, username, course_run_key):
username (str): The user to award the Credentials course cert to
course_run_key (str): The course run key to award the certificate for
"""
def _retry_with_custom_exception(username, course_run_key, reason, countdown):
exception = MaxRetriesExceededError(
f"Failed to award course certificate for user {username} for course {course_run_key}. Reason: {reason}"
)
return self.retry(
exc=exception,
countdown=countdown,
max_retries=MAX_RETRIES
)
return self.retry(exc=exception, countdown=countdown, max_retries=MAX_RETRIES)
LOGGER.info(f"Running task award_course_certificate for username {username}")
countdown = 2 ** self.request.retries
countdown = 2**self.request.retries
# If the credentials config model is disabled for this
# feature, it may indicate a condition where processing of such tasks
# has been temporarily disabled. Since this is a recoverable situation,
# mark this task for retry instead of failing it altogether.
if not CredentialsApiConfig.current().is_learner_issuance_enabled:
error_msg = (
"Task award_course_certificate cannot be executed when credentials issuance is disabled in API config"
)
error_msg = "Task award_course_certificate cannot be executed when credentials issuance is disabled in API config"
LOGGER.warning(error_msg)
raise _retry_with_custom_exception(
username=username,
course_run_key=course_run_key,
reason=error_msg,
countdown=countdown
countdown=countdown,
)
try:
@@ -456,14 +488,15 @@ def award_course_certificate(self, username, course_run_key):
try:
user = User.objects.get(username=username)
except User.DoesNotExist:
LOGGER.exception(f"Task award_course_certificate was called with invalid username {username}")
LOGGER.exception(
f"Task award_course_certificate was called with invalid username {username}"
)
# Don't retry for this case - just conclude the task.
return
# Get the cert for the course key and username if it's both passing and available in professional/verified
try:
certificate = GeneratedCertificate.eligible_certificates.get(
user=user.id,
course_id=course_key
user=user.id, course_id=course_key
)
except GeneratedCertificate.DoesNotExist:
LOGGER.exception(
@@ -505,10 +538,17 @@ def award_course_certificate(self, username, course_run_key):
date_override = None
post_course_certificate(
credentials_client, username, certificate, visible_date, date_override, org=course_key.org
credentials_client,
username,
certificate,
visible_date,
date_override,
org=course_key.org,
)
LOGGER.info(f"Awarded certificate for course {course_key} to user {username}")
LOGGER.info(
f"Awarded certificate for course {course_key} to user {username}"
)
except Exception as exc:
error_msg = f"Failed to determine course certificates to be awarded for user {username}."
LOGGER.exception(error_msg)
@@ -516,11 +556,13 @@ def award_course_certificate(self, username, course_run_key):
username=username,
course_run_key=course_run_key,
reason=error_msg,
countdown=countdown
countdown=countdown,
) from exc
def get_revokable_program_uuids(course_specific_programs, student):
def get_revokable_program_uuids(
course_specific_programs: List[Dict], student: User
) -> List[str]:
"""
Get program uuids for which certificate to be revoked.
@@ -532,14 +574,19 @@ def get_revokable_program_uuids(course_specific_programs, student):
student (User): Representing the student whose programs to check for.
Returns:
list if program UUIDs for which certificates to be revoked
list of program UUIDs for which certificates to be revoked
Raises:
HttpError, if the API call generated by get_certified_programs fails
"""
program_uuids_to_revoke = []
existing_program_uuids = get_certified_programs(student)
# Get any programs where the user has already been rewarded a certificate
# Failed API calls with get_certified_programs should raise exceptions,
# because an empty response would dangerously imply a false negative.
existing_program_uuids = get_certified_programs(student, raise_on_error=True)
for program in course_specific_programs:
if program['uuid'] in existing_program_uuids:
program_uuids_to_revoke.append(program['uuid'])
if program["uuid"] in existing_program_uuids:
program_uuids_to_revoke.append(program["uuid"])
return program_uuids_to_revoke
@@ -561,20 +608,19 @@ def revoke_program_certificate(client, username, program_uuid):
response = client.post(
api_url,
json={
'username': username,
'status': 'revoked',
'credential': {
'type': PROGRAM_CERTIFICATE,
'program_uuid': program_uuid
}
}
"username": username,
"status": "revoked",
"credential": {"type": PROGRAM_CERTIFICATE, "program_uuid": program_uuid},
},
)
response.raise_for_status()
@shared_task(bind=True, ignore_result=True)
@set_code_owner_attribute
def revoke_program_certificates(self, username, course_key): # lint-amnesty, pylint: disable=too-many-statements
def revoke_program_certificates(
self, username, course_key
): # lint-amnesty, pylint: disable=too-many-statements
"""
This task is designed to be called whenever a student's course certificate is
revoked.
@@ -594,38 +640,36 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
None
"""
def _retry_with_custom_exception(username, course_key, reason, countdown):
exception = MaxRetriesExceededError(
f"Failed to revoke program certificate for user {username} for course {course_key}. Reason: {reason}"
)
return self.retry(
exc=exception,
countdown=countdown,
max_retries=MAX_RETRIES
)
return self.retry(exc=exception, countdown=countdown, max_retries=MAX_RETRIES)
countdown = 2 ** self.request.retries
countdown = 2**self.request.retries
# If the credentials config model is disabled for this
# feature, it may indicate a condition where processing of such tasks
# has been temporarily disabled. Since this is a recoverable situation,
# mark this task for retry instead of failing it altogether.
if not CredentialsApiConfig.current().is_learner_issuance_enabled:
error_msg = (
"Task revoke_program_certificates cannot be executed when credentials issuance is disabled in API config"
)
error_msg = "Task revoke_program_certificates cannot be executed when credentials issuance is disabled in API config"
LOGGER.warning(error_msg)
raise _retry_with_custom_exception(
username=username,
course_key=course_key,
reason=error_msg,
countdown=countdown
countdown=countdown,
)
try:
student = User.objects.get(username=username)
except User.DoesNotExist:
LOGGER.exception(f"Task revoke_program_certificates was called with invalid username {username}", username)
LOGGER.exception(
f"Task revoke_program_certificates was called with invalid username {username}",
username,
)
# Don't retry for this case - just conclude the task.
return
@@ -641,7 +685,9 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
return
# Determine which program certificates the user has already been awarded, if any.
program_uuids_to_revoke = get_revokable_program_uuids(course_specific_programs, student)
program_uuids_to_revoke = get_revokable_program_uuids(
course_specific_programs, student
)
except Exception as exc:
error_msg = (
f"Failed to determine program certificates to be revoked for user {username} "
@@ -652,7 +698,7 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
username=username,
course_key=course_key,
reason=error_msg,
countdown=countdown
countdown=countdown,
) from exc
if program_uuids_to_revoke:
@@ -664,13 +710,17 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
error_msg = "Failed to create a credentials API client to revoke program certificates"
LOGGER.exception(error_msg)
# Retry because a misconfiguration could be fixed
raise _retry_with_custom_exception(username, course_key, reason=exc, countdown=countdown) from exc
raise _retry_with_custom_exception(
username, course_key, reason=exc, countdown=countdown
) from exc
failed_program_certificate_revoke_attempts = []
for program_uuid in program_uuids_to_revoke:
try:
revoke_program_certificate(credentials_client, username, program_uuid)
LOGGER.info(f"Revoked certificate for program {program_uuid} for user {username}")
LOGGER.info(
f"Revoked certificate for program {program_uuid} for user {username}"
)
except HTTPError as exc:
if exc.response.status_code == 404:
LOGGER.exception(
@@ -689,7 +739,7 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
username,
course_key,
reason=error_msg,
countdown=rate_limit_countdown
countdown=rate_limit_countdown,
) from exc
else:
LOGGER.exception(
@@ -697,12 +747,16 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
)
except Exception: # pylint: disable=broad-except
# keep trying to revoke other certs, but retry the whole task to fix any missing entries
LOGGER.warning(f"Failed to revoke certificate for program {program_uuid} of user {username}.")
LOGGER.warning(
f"Failed to revoke certificate for program {program_uuid} of user {username}."
)
failed_program_certificate_revoke_attempts.append(program_uuid)
if failed_program_certificate_revoke_attempts:
# N.B. This logic assumes that this task is idempotent
LOGGER.info(f"Retrying task to revoke failed certificates to user {username}")
LOGGER.info(
f"Retrying task to revoke failed certificates to user {username}"
)
# The error message may change on each reattempt but will never be raised until
# the max number of retries have been exceeded. It is unlikely that this list
# will change by the time it reaches its maximimum number of attempts.
@@ -711,15 +765,14 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py
f"for programs {failed_program_certificate_revoke_attempts}"
)
raise _retry_with_custom_exception(
username,
course_key,
reason=error_msg,
countdown=countdown
username, course_key, reason=error_msg, countdown=countdown
)
else:
LOGGER.info(f"There is no program certificates for user {username} to revoke")
LOGGER.info(f"Successfully completed the task revoke_program_certificates for username {username}")
LOGGER.info(
f"Successfully completed the task revoke_program_certificates for username {username}"
)
@shared_task(bind=True, ignore_result=True)
@@ -738,7 +791,7 @@ def update_certificate_visible_date_on_course_update(self, course_key):
Arguments:
course_key(str): The course identifier
"""
countdown = 2 ** self.request.retries
countdown = 2**self.request.retries
# If the CredentialsApiConfig configuration model is disabled for this feature, it may indicate a condition where
# processing of such tasks has been temporarily disabled. Since this is a recoverable situation, mark this task for
@@ -758,10 +811,9 @@ def update_certificate_visible_date_on_course_update(self, course_key):
# Retrieve a list of all usernames of learners who have a certificate record in this course-run. The
# Credentials IDA REST API still requires a username as the main identifier for the learner.
users_with_certificates_in_course = (
GeneratedCertificate
.eligible_available_certificates
.filter(course_id=course_key)
.values_list('user__username', flat=True)
GeneratedCertificate.eligible_available_certificates.filter(
course_id=course_key
).values_list("user__username", flat=True)
)
LOGGER.info(
@@ -786,7 +838,7 @@ def update_certificate_available_date_on_course_update(self, course_key):
Args:
course_key(str): The course identifier
"""
countdown = 2 ** self.request.retries
countdown = 2**self.request.retries
# If the CredentialsApiConfig configuration model is disabled for this feature, it may indicate a condition where
# processing of such tasks has been temporarily disabled. Since this is a recoverable situation, mark this task for
@@ -808,17 +860,17 @@ def update_certificate_available_date_on_course_update(self, course_key):
# - The course-run is instructor-paced, AND
# - The `certificates_display_behavior` is set to "end_with_date",
if (
course_overview and
course_overview.self_paced is False and
course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE
course_overview
and course_overview.self_paced is False
and course_overview.certificates_display_behavior
== CertificatesDisplayBehaviors.END_WITH_DATE
):
LOGGER.info(
f"Queueing task to update the `certificate_available_date` of course-run {course_key} to "
f"[{course_overview.certificate_available_date}] in the Credentials service"
)
update_credentials_course_certificate_configuration_available_date.delay(
str(course_key),
str(course_overview.certificate_available_date)
str(course_key), str(course_overview.certificate_available_date)
)
# OR,
# - The course-run is self-paced, AND
@@ -829,15 +881,17 @@ def update_certificate_available_date_on_course_update(self, course_key):
# hidden. This is due to the Credentials IDA not understanding the concept of course pacing. Thus, we need a way
# to remove this value from self-paced courses in Credentials.
elif (
course_overview and
course_overview.self_paced is True and
course_overview.certificate_available_date is None
course_overview
and course_overview.self_paced is True
and course_overview.certificate_available_date is None
):
LOGGER.info(
"Queueing task to remove the `certificate_available_date` in the Credentials service for course-run "
f"{course_key}"
)
update_credentials_course_certificate_configuration_available_date.delay(str(course_key), None)
update_credentials_course_certificate_configuration_available_date.delay(
str(course_key), None
)
# ELSE, we don't meet the criteria to update the course cert config in the Credentials IDA
else:
LOGGER.warning(

605
openedx/core/djangoapps/programs/tests/test_tasks.py
View File

@@ -19,23 +19,31 @@ from django.test import TestCase, override_settings
from edx_rest_api_client.auth import SuppliedJwtAuth
from requests.exceptions import HTTPError
from testfixtures import LogCapture
from xmodule.data import CertificatesDisplayBehaviors
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.tests.factories import CertificateDateOverrideFactory, GeneratedCertificateFactory
from lms.djangoapps.certificates.tests.factories import (
CertificateDateOverrideFactory,
GeneratedCertificateFactory,
)
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.content.course_overviews.tests.factories import (
CourseOverviewFactory,
)
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationFactory
from openedx.core.djangoapps.programs import tasks
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
from openedx.core.djangoapps.site_configuration.tests.factories import (
SiteConfigurationFactory,
SiteFactory,
)
from openedx.core.djangolib.testing.utils import skip_unless_lms
from xmodule.data import CertificatesDisplayBehaviors
log = logging.getLogger(__name__)
CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials.example.com'
TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks'
CREDENTIALS_INTERNAL_SERVICE_URL = "https://credentials.example.com"
TASKS_MODULE = "openedx.core.djangoapps.programs.tasks"
@skip_unless_lms
@@ -49,84 +57,101 @@ class GetAwardedCertificateProgramsTestCase(TestCase):
Helper to make dummy results from the credentials API
"""
result = {
'id': 1,
'username': 'dummy-username',
'credential': {
'credential_id': None,
'program_uuid': None,
"id": 1,
"username": "dummy-username",
"credential": {
"credential_id": None,
"program_uuid": None,
},
'status': 'dummy-status',
'uuid': 'dummy-uuid',
'certificate_url': 'http://credentials.edx.org/credentials/dummy-uuid/'
"status": "dummy-status",
"uuid": "dummy-uuid",
"certificate_url": "http://credentials.edx.org/credentials/dummy-uuid/",
}
result.update(**kwargs)
return result
@mock.patch(TASKS_MODULE + '.get_credentials')
@mock.patch(TASKS_MODULE + ".get_credentials")
def test_get_certified_programs(self, mock_get_credentials):
"""
Ensure the API is called and results handled correctly.
"""
student = UserFactory(username='test-username')
student = UserFactory(username="test-username")
mock_get_credentials.return_value = [
self.make_credential_result(status='awarded', credential={'program_uuid': 1}),
self.make_credential_result(
status="awarded", credential={"program_uuid": 1}
),
]
result = tasks.get_certified_programs(student)
assert mock_get_credentials.call_args[0] == (student,)
assert mock_get_credentials.call_args[1] == {'credential_type': 'program'}
assert mock_get_credentials.call_args[1] == {"credential_type": "program"}
assert result == [1]
def test_data_retrieval_raise_on_error(self, mock_raise):
"""
Verify that when raise_on_error is set and the API call gives an error response,
get_certified_programs reraises HttpError.
"""
mock_raise.return_value = HTTPError("An Error occured")
student = UserFactory(username="test-username")
with self.assertRaises(HTTPError):
tasks.get_certified_programs(student, raise_on_error=True)
@skip_unless_lms
class AwardProgramCertificateTestCase(TestCase):
"""
Test the award_program_certificate function
"""
@httpretty.activate
@mock.patch('openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url')
@mock.patch("openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url")
def test_award_program_certificate(self, mock_get_api_base_url):
"""
Ensure the correct API call gets made
"""
mock_get_api_base_url.return_value = 'http://test-server/'
student = UserFactory(username='test-username', email='test-email@email.com')
mock_get_api_base_url.return_value = "http://test-server/"
student = UserFactory(username="test-username", email="test-email@email.com")
test_client = requests.Session()
test_client.auth = SuppliedJwtAuth('test-token')
test_client.auth = SuppliedJwtAuth("test-token")
httpretty.register_uri(
httpretty.POST,
'http://test-server/credentials/',
"http://test-server/credentials/",
)
tasks.award_program_certificate(test_client, student, 123, datetime(2010, 5, 30))
tasks.award_program_certificate(
test_client, student, 123, datetime(2010, 5, 30)
)
expected_body = {
'username': student.username,
'lms_user_id': student.id,
'credential': {
'program_uuid': 123,
'type': tasks.PROGRAM_CERTIFICATE,
"username": student.username,
"lms_user_id": student.id,
"credential": {
"program_uuid": 123,
"type": tasks.PROGRAM_CERTIFICATE,
},
'attributes': [
"attributes": [
{
'name': 'visible_date',
'value': '2010-05-30T00:00:00Z',
"name": "visible_date",
"value": "2010-05-30T00:00:00Z",
}
]
],
}
last_request_body = httpretty.last_request().body.decode('utf-8')
last_request_body = httpretty.last_request().body.decode("utf-8")
assert json.loads(last_request_body) == expected_body
@skip_unless_lms
@ddt.ddt
@mock.patch(TASKS_MODULE + '.award_program_certificate')
@mock.patch(TASKS_MODULE + '.get_certified_programs')
@mock.patch(TASKS_MODULE + '.get_completed_programs')
@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username')
class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase):
@mock.patch(TASKS_MODULE + ".award_program_certificate")
@mock.patch(TASKS_MODULE + ".get_certified_programs")
@mock.patch(TASKS_MODULE + ".get_completed_programs")
@override_settings(CREDENTIALS_SERVICE_USERNAME="test-service-username")
class AwardProgramCertificatesTestCase(
CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase
):
"""
Tests for the 'award_program_certificates' celery task.
"""
@@ -134,11 +159,11 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
def setUp(self):
super().setUp()
self.create_credentials_config()
self.student = UserFactory.create(username='test-student')
self.student = UserFactory.create(username="test-student")
self.site = SiteFactory()
self.site_configuration = SiteConfigurationFactory(site=self.site)
self.catalog_integration = self.create_catalog_integration()
ApplicationFactory.create(name='credentials')
ApplicationFactory.create(name="credentials")
UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)
def test_completion_check(
@@ -177,20 +202,26 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
tasks.award_program_certificates.delay(self.student.username).get()
actual_program_uuids = [call[0][2] for call in mock_award_program_certificate.call_args_list]
actual_program_uuids = [
call[0][2] for call in mock_award_program_certificate.call_args_list
]
assert actual_program_uuids == expected_awarded_program_uuids
actual_visible_dates = [call[0][3] for call in mock_award_program_certificate.call_args_list]
actual_visible_dates = [
call[0][3] for call in mock_award_program_certificate.call_args_list
]
assert actual_visible_dates == expected_awarded_program_uuids
# program uuids are same as mock dates
@mock.patch('openedx.core.djangoapps.site_configuration.helpers.get_current_site_configuration')
@mock.patch(
"openedx.core.djangoapps.site_configuration.helpers.get_current_site_configuration"
)
def test_awarding_certs_with_skip_program_certificate(
self,
mocked_get_current_site_configuration,
mock_get_completed_programs,
mock_get_certified_programs,
mock_award_program_certificate,
self,
mocked_get_current_site_configuration,
mock_get_completed_programs,
mock_get_certified_programs,
mock_award_program_certificate,
):
"""
Checks that the Credentials API is used to award certificates for
@@ -204,9 +235,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
mock_get_certified_programs.return_value = [1]
# programs to be skipped
self.site_configuration.site_values = {
"programs_without_certificates": [2]
}
self.site_configuration.site_values = {"programs_without_certificates": [2]}
self.site_configuration.save()
mocked_get_current_site_configuration.return_value = self.site_configuration
@@ -215,28 +244,31 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
expected_awarded_program_uuids = [3, 4]
tasks.award_program_certificates.delay(self.student.username).get()
actual_program_uuids = [call[0][2] for call in mock_award_program_certificate.call_args_list]
actual_program_uuids = [
call[0][2] for call in mock_award_program_certificate.call_args_list
]
assert actual_program_uuids == expected_awarded_program_uuids
actual_visible_dates = [call[0][3] for call in mock_award_program_certificate.call_args_list]
actual_visible_dates = [
call[0][3] for call in mock_award_program_certificate.call_args_list
]
assert actual_visible_dates == expected_awarded_program_uuids
# program uuids are same as mock dates
@ddt.data(
('credentials', 'enable_learner_issuance'),
("credentials", "enable_learner_issuance"),
)
@ddt.unpack
def test_retry_if_config_disabled(
self,
disabled_config_type,
disabled_config_attribute,
*mock_helpers
self, disabled_config_type, disabled_config_attribute, *mock_helpers
):
"""
Checks that the task is aborted if any relevant api configs are
disabled.
"""
getattr(self, f'create_{disabled_config_type}_config')(**{disabled_config_attribute: False})
with mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning:
getattr(self, f"create_{disabled_config_type}_config")(
**{disabled_config_attribute: False}
)
with mock.patch(TASKS_MODULE + ".LOGGER.warning") as mock_warning:
with pytest.raises(MaxRetriesExceededError):
tasks.award_program_certificates.delay(self.student.username).get()
assert mock_warning.called
@@ -248,8 +280,8 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
Checks that the task will be aborted and not retried if the username
passed was not found, and that an exception is logged.
"""
with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
tasks.award_program_certificates.delay('nonexistent-username').get()
with mock.patch(TASKS_MODULE + ".LOGGER.exception") as mock_exception:
tasks.award_program_certificates.delay("nonexistent-username").get()
assert mock_exception.called
for mock_helper in mock_helpers:
assert not mock_helper.called
@@ -270,13 +302,13 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
assert not mock_get_certified_programs.called
assert not mock_award_program_certificate.called
@mock.patch('openedx.core.djangoapps.site_configuration.helpers.get_value')
@mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value")
def test_programs_without_certificates(
self,
mock_get_value,
mock_get_completed_programs,
mock_get_certified_programs,
mock_award_program_certificate
mock_award_program_certificate,
):
"""
Checks that the task will be aborted without further action if there exists a list
@@ -289,22 +321,22 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
assert not mock_get_certified_programs.called
assert not mock_award_program_certificate.called
@mock.patch(TASKS_MODULE + '.get_credentials_api_client')
@mock.patch(TASKS_MODULE + ".get_credentials_api_client")
def test_failure_to_create_api_client_retries(
self,
mock_get_api_client,
mock_get_completed_programs,
mock_get_certified_programs,
mock_award_program_certificate
mock_award_program_certificate,
):
"""
Checks that we log an exception and retry if the API client isn't creating.
"""
mock_get_api_client.side_effect = Exception('boom')
mock_get_api_client.side_effect = Exception("boom")
mock_get_completed_programs.return_value = {1: 1, 2: 2}
mock_get_certified_programs.return_value = [2]
with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
with mock.patch(TASKS_MODULE + ".LOGGER.exception") as mock_exception:
with pytest.raises(MaxRetriesExceededError):
tasks.award_program_certificates.delay(self.student.username).get()
@@ -347,25 +379,30 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
"""
mock_get_completed_programs.return_value = {1: 1, 2: 2}
mock_get_certified_programs.side_effect = [[], [2]]
mock_award_program_certificate.side_effect = self._make_side_effect([Exception('boom'), None])
mock_award_program_certificate.side_effect = self._make_side_effect(
[Exception("boom"), None]
)
with mock.patch(TASKS_MODULE + '.LOGGER.info') as mock_info, \
mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_warning:
with mock.patch(TASKS_MODULE + ".LOGGER.info") as mock_info, mock.patch(
TASKS_MODULE + ".LOGGER.exception"
) as mock_warning:
tasks.award_program_certificates.delay(self.student.username).get()
assert mock_award_program_certificate.call_count == 3
mock_warning.assert_called_once_with(
'Failed to award certificate for program {uuid} to user {username}.'.format(
uuid=1,
username=self.student.username)
"Failed to award certificate for program {uuid} to user {username}.".format(
uuid=1, username=self.student.username
)
)
mock_info.assert_any_call(
f"Awarded certificate for program {1} to user {self.student.username}"
)
mock_info.assert_any_call(
f"Awarded certificate for program {2} to user {self.student.username}"
)
mock_info.assert_any_call(f"Awarded certificate for program {1} to user {self.student.username}")
mock_info.assert_any_call(f"Awarded certificate for program {2} to user {self.student.username}")
def test_retry_on_programs_api_errors(
self,
mock_get_completed_programs,
*_mock_helpers
self, mock_get_completed_programs, *_mock_helpers
):
"""
Ensures that any otherwise-unhandled errors that arise while trying
@@ -373,7 +410,9 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
transient API errors) will cause the task to be failed and queued for
retry.
"""
mock_get_completed_programs.side_effect = self._make_side_effect([Exception('boom'), None])
mock_get_completed_programs.side_effect = self._make_side_effect(
[Exception("boom"), None]
)
tasks.award_program_certificates.delay(self.student.username).get()
assert mock_get_completed_programs.call_count == 3
@@ -391,7 +430,9 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
"""
mock_get_completed_programs.return_value = {1: 1, 2: 2}
mock_get_certified_programs.return_value = [1]
mock_get_certified_programs.side_effect = self._make_side_effect([Exception('boom'), None])
mock_get_certified_programs.side_effect = self._make_side_effect(
[Exception("boom"), None]
)
tasks.award_program_certificates.delay(self.student.username).get()
assert mock_get_certified_programs.call_count == 2
assert mock_award_program_certificate.call_count == 1
@@ -464,59 +505,68 @@ class PostCourseCertificateTestCase(TestCase):
"""
def setUp(self): # lint-amnesty, pylint: disable=super-method-not-called
self.student = UserFactory.create(username='test-student')
self.student = UserFactory.create(username="test-student")
self.course = CourseOverviewFactory.create(
self_paced=True # Any option to allow the certificate to be viewable for the course
)
self.certificate = GeneratedCertificateFactory(
user=self.student,
mode='verified',
mode="verified",
course_id=self.course.id,
status='downloadable'
status="downloadable",
)
@httpretty.activate
@mock.patch('openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url')
@mock.patch("openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url")
def test_post_course_certificate(self, mock_get_api_base_url):
"""
Ensure the correct API call gets made
"""
mock_get_api_base_url.return_value = 'http://test-server/'
mock_get_api_base_url.return_value = "http://test-server/"
test_client = requests.Session()
test_client.auth = SuppliedJwtAuth('test-token')
test_client.auth = SuppliedJwtAuth("test-token")
httpretty.register_uri(
httpretty.POST,
'http://test-server/credentials/',
"http://test-server/credentials/",
)
visible_date = datetime.now()
tasks.post_course_certificate(test_client, self.student.username, self.certificate, visible_date)
tasks.post_course_certificate(
test_client, self.student.username, self.certificate, visible_date
)
expected_body = {
'username': self.student.username,
'status': 'awarded',
'credential': {
'course_run_key': str(self.certificate.course_id),
'mode': self.certificate.mode,
'type': tasks.COURSE_CERTIFICATE,
"username": self.student.username,
"status": "awarded",
"credential": {
"course_run_key": str(self.certificate.course_id),
"mode": self.certificate.mode,
"type": tasks.COURSE_CERTIFICATE,
},
'date_override': None,
'attributes': [{
'name': 'visible_date',
'value': visible_date.strftime('%Y-%m-%dT%H:%M:%SZ') # text representation of date
}]
"date_override": None,
"attributes": [
{
"name": "visible_date",
"value": visible_date.strftime(
"%Y-%m-%dT%H:%M:%SZ"
), # text representation of date
}
],
}
last_request_body = httpretty.last_request().body.decode('utf-8')
last_request_body = httpretty.last_request().body.decode("utf-8")
assert json.loads(last_request_body) == expected_body
@skip_unless_lms
@ddt.ddt
@mock.patch("lms.djangoapps.certificates.api.auto_certificate_generation_enabled", mock.Mock(return_value=True))
@mock.patch(TASKS_MODULE + '.post_course_certificate')
@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username')
@mock.patch(
"lms.djangoapps.certificates.api.auto_certificate_generation_enabled",
mock.Mock(return_value=True),
)
@mock.patch(TASKS_MODULE + ".post_course_certificate")
@override_settings(CREDENTIALS_SERVICE_USERNAME="test-service-username")
class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase):
"""
Test the award_course_certificate celery task
@@ -529,21 +579,21 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase):
self.course = CourseOverviewFactory.create(
self_paced=True, # Any option to allow the certificate to be viewable for the course
certificate_available_date=self.available_date,
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE,
)
self.student = UserFactory.create(username='test-student')
self.student = UserFactory.create(username="test-student")
# Instantiate the Certificate first so that the config doesn't execute issuance
self.certificate = GeneratedCertificateFactory.create(
user=self.student,
mode='verified',
mode="verified",
course_id=self.course.id,
status='downloadable'
status="downloadable",
)
self.create_credentials_config()
self.site = SiteFactory()
ApplicationFactory.create(name='credentials')
ApplicationFactory.create(name="credentials")
UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)
def _add_certificate_date_override(self):
@@ -552,12 +602,12 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase):
"""
self.certificate.date_override = CertificateDateOverrideFactory.create(
generated_certificate=self.certificate,
overridden_by=UserFactory.create(username='test-admin'),
overridden_by=UserFactory.create(username="test-admin"),
)
@ddt.data(
'verified',
'no-id-professional',
"verified",
"no-id-professional",
)
def test_award_course_certificates(self, mode, mock_post_course_certificate):
"""
@@ -565,89 +615,119 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase):
"""
self.certificate.mode = mode
self.certificate.save()
tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get()
tasks.award_course_certificate.delay(
self.student.username, str(self.course.id)
).get()
call_args, _ = mock_post_course_certificate.call_args
assert call_args[1] == self.student.username
assert call_args[2] == self.certificate
assert call_args[3] == self.certificate.modified_date
def test_award_course_certificates_available_date(self, mock_post_course_certificate):
def test_award_course_certificates_available_date(
self, mock_post_course_certificate
):
"""
Tests the API POST method is called with available date when the course is not self paced
"""
self.course.self_paced = False
self.course.save()
tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get()
tasks.award_course_certificate.delay(
self.student.username, str(self.course.id)
).get()
call_args, _ = mock_post_course_certificate.call_args
assert call_args[1] == self.student.username
assert call_args[2] == self.certificate
assert call_args[3] == self.available_date
def test_award_course_certificates_override_date(self, mock_post_course_certificate):
def test_award_course_certificates_override_date(
self, mock_post_course_certificate
):
"""
Tests the API POST method is called with date override when present
"""
self._add_certificate_date_override()
tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get()
tasks.award_course_certificate.delay(
self.student.username, str(self.course.id)
).get()
call_args, _ = mock_post_course_certificate.call_args
assert call_args[1] == self.student.username
assert call_args[2] == self.certificate
assert call_args[3] == self.certificate.modified_date
assert call_args[4] == self.certificate.date_override.date
def test_award_course_cert_not_called_if_disabled(self, mock_post_course_certificate):
def test_award_course_cert_not_called_if_disabled(
self, mock_post_course_certificate
):
"""
Test that the post method is never called if the config is disabled
"""
self.create_credentials_config(enabled=False)
with mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning:
with mock.patch(TASKS_MODULE + ".LOGGER.warning") as mock_warning:
with pytest.raises(MaxRetriesExceededError):
tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get()
tasks.award_course_certificate.delay(
self.student.username, str(self.course.id)
).get()
assert mock_warning.called
assert not mock_post_course_certificate.called
def test_award_course_cert_not_called_if_user_not_found(self, mock_post_course_certificate):
def test_award_course_cert_not_called_if_user_not_found(
self, mock_post_course_certificate
):
"""
Test that the post method is never called if the user isn't found by username
"""
with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
with mock.patch(TASKS_MODULE + ".LOGGER.exception") as mock_exception:
# Use a random username here since this user won't be found in the DB
tasks.award_course_certificate.delay('random_username', str(self.course.id)).get()
tasks.award_course_certificate.delay(
"random_username", str(self.course.id)
).get()
assert mock_exception.called
assert not mock_post_course_certificate.called
def test_award_course_cert_not_called_if_certificate_not_found(self, mock_post_course_certificate):
def test_award_course_cert_not_called_if_certificate_not_found(
self, mock_post_course_certificate
):
"""
Test that the post method is never called if the certificate doesn't exist for the user and course
"""
self.certificate.delete()
with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
tasks.award_course_certificate.delay(self.student.username, str(self.course.id)).get()
with mock.patch(TASKS_MODULE + ".LOGGER.exception") as mock_exception:
tasks.award_course_certificate.delay(
self.student.username, str(self.course.id)
).get()
assert mock_exception.called
assert not mock_post_course_certificate.called
def test_award_course_cert_not_called_if_course_overview_not_found(self, mock_post_course_certificate):
def test_award_course_cert_not_called_if_course_overview_not_found(
self, mock_post_course_certificate
):
"""
Test that the post method is never called if the CourseOverview isn't found
"""
self.course.delete()
with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
with mock.patch(TASKS_MODULE + ".LOGGER.exception") as mock_exception:
# Use the certificate course id here since the course will be deleted
tasks.award_course_certificate.delay(self.student.username, str(self.certificate.course_id)).get()
tasks.award_course_certificate.delay(
self.student.username, str(self.certificate.course_id)
).get()
assert mock_exception.called
assert not mock_post_course_certificate.called
def test_award_course_cert_not_called_if_certificated_not_verified_mode(self, mock_post_course_certificate):
def test_award_course_cert_not_called_if_certificated_not_verified_mode(
self, mock_post_course_certificate
):
"""
Test that the post method is never called if the GeneratedCertificate is an 'audit' cert
"""
# Temporarily disable the config so the signal isn't handled from .save
self.create_credentials_config(enabled=False)
self.certificate.mode = 'audit'
self.certificate.mode = "audit"
self.certificate.save()
self.create_credentials_config()
tasks.award_course_certificate.delay(self.student.username, str(self.certificate.course_id)).get()
tasks.award_course_certificate.delay(
self.student.username, str(self.certificate.course_id)
).get()
assert not mock_post_course_certificate.called
@@ -658,42 +738,44 @@ class RevokeProgramCertificateTestCase(TestCase):
"""
@httpretty.activate
@mock.patch('openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url')
@mock.patch("openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url")
def test_revoke_program_certificate(self, mock_get_api_base_url):
"""
Ensure the correct API call gets made
"""
mock_get_api_base_url.return_value = 'http://test-server/'
test_username = 'test-username'
mock_get_api_base_url.return_value = "http://test-server/"
test_username = "test-username"
test_client = requests.Session()
test_client.auth = SuppliedJwtAuth('test-token')
test_client.auth = SuppliedJwtAuth("test-token")
httpretty.register_uri(
httpretty.POST,
'http://test-server/credentials/',
"http://test-server/credentials/",
)
tasks.revoke_program_certificate(test_client, test_username, 123)
expected_body = {
'username': test_username,
'status': 'revoked',
'credential': {
'program_uuid': 123,
'type': tasks.PROGRAM_CERTIFICATE,
}
"username": test_username,
"status": "revoked",
"credential": {
"program_uuid": 123,
"type": tasks.PROGRAM_CERTIFICATE,
},
}
last_request_body = httpretty.last_request().body.decode('utf-8')
last_request_body = httpretty.last_request().body.decode("utf-8")
assert json.loads(last_request_body) == expected_body
@skip_unless_lms
@ddt.ddt
@mock.patch(TASKS_MODULE + '.revoke_program_certificate')
@mock.patch(TASKS_MODULE + '.get_certified_programs')
@mock.patch(TASKS_MODULE + '.get_inverted_programs')
@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username')
class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase):
@mock.patch(TASKS_MODULE + ".revoke_program_certificate")
@mock.patch(TASKS_MODULE + ".get_certified_programs")
@mock.patch(TASKS_MODULE + ".get_inverted_programs")
@override_settings(CREDENTIALS_SERVICE_USERNAME="test-service-username")
class RevokeProgramCertificatesTestCase(
CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase
):
"""
Tests for the 'revoke_program_certificates' celery task.
"""
@@ -701,15 +783,15 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
def setUp(self):
super().setUp()
self.student = UserFactory.create(username='test-student')
self.course_key = 'course-v1:testX+test101+2T2020'
self.student = UserFactory.create(username="test-student")
self.course_key = "course-v1:testX+test101+2T2020"
self.site = SiteFactory()
self.site_configuration = SiteConfigurationFactory(site=self.site)
ApplicationFactory.create(name='credentials')
ApplicationFactory.create(name="credentials")
UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)
self.create_credentials_config()
self.inverted_programs = {self.course_key: [{'uuid': 1}, {'uuid': 2}]}
self.inverted_programs = {self.course_key: [{"uuid": 1}, {"uuid": 2}]}
def _make_side_effect(self, side_effects):
"""
@@ -742,7 +824,9 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
Checks that the Programs API is used correctly to determine completed
programs.
"""
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
mock_get_inverted_programs.assert_any_call(self.student)
def test_revoke_program_certificate(
@@ -757,34 +841,37 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
"""
expected_program_uuid = 1
mock_get_inverted_programs.return_value = {
self.course_key: [{'uuid': expected_program_uuid}]
self.course_key: [{"uuid": expected_program_uuid}]
}
mock_get_certified_programs.return_value = [expected_program_uuid]
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
call_args, _ = mock_revoke_program_certificate.call_args
assert call_args[1] == self.student.username
assert call_args[2] == expected_program_uuid
@ddt.data(
('credentials', 'enable_learner_issuance'),
("credentials", "enable_learner_issuance"),
)
@ddt.unpack
def test_retry_if_config_disabled(
self,
disabled_config_type,
disabled_config_attribute,
*mock_helpers
self, disabled_config_type, disabled_config_attribute, *mock_helpers
):
"""
Checks that the task is aborted if any relevant api configs are
disabled.
"""
getattr(self, f'create_{disabled_config_type}_config')(**{disabled_config_attribute: False})
with mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning:
getattr(self, f"create_{disabled_config_type}_config")(
**{disabled_config_attribute: False}
)
with mock.patch(TASKS_MODULE + ".LOGGER.warning") as mock_warning:
with pytest.raises(MaxRetriesExceededError):
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_warning.called
for mock_helper in mock_helpers:
assert not mock_helper.called
@@ -794,8 +881,10 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
Checks that the task will be aborted and not retried if the username
passed was not found, and that an exception is logged.
"""
with mock.patch(TASKS_MODULE + '.LOGGER.exception') as mock_exception:
tasks.revoke_program_certificates.delay('nonexistent-username', self.course_key).get()
with mock.patch(TASKS_MODULE + ".LOGGER.exception") as mock_exception:
tasks.revoke_program_certificates.delay(
"nonexistent-username", self.course_key
).get()
assert mock_exception.called
for mock_helper in mock_helpers:
assert not mock_helper.called
@@ -811,7 +900,9 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
not part of any program.
"""
mock_get_inverted_programs.return_value = {}
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_get_inverted_programs.called
assert not mock_get_certified_programs.called
assert not mock_revoke_program_certificate.called
@@ -830,20 +921,29 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
"""
mock_get_inverted_programs.return_value = self.inverted_programs
mock_get_certified_programs.side_effect = [[1], [1, 2]]
mock_revoke_program_certificate.side_effect = self._make_side_effect([Exception('boom'), None])
mock_revoke_program_certificate.side_effect = self._make_side_effect(
[Exception("boom"), None]
)
with mock.patch(TASKS_MODULE + '.LOGGER.info') as mock_info, \
mock.patch(TASKS_MODULE + '.LOGGER.warning') as mock_warning:
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
with mock.patch(TASKS_MODULE + ".LOGGER.info") as mock_info, mock.patch(
TASKS_MODULE + ".LOGGER.warning"
) as mock_warning:
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_revoke_program_certificate.call_count == 3
mock_warning.assert_called_once_with(
'Failed to revoke certificate for program {uuid} of user {username}.'.format(
uuid=1,
username=self.student.username)
"Failed to revoke certificate for program {uuid} of user {username}.".format(
uuid=1, username=self.student.username
)
)
mock_info.assert_any_call(
f"Revoked certificate for program {1} for user {self.student.username}"
)
mock_info.assert_any_call(
f"Revoked certificate for program {2} for user {self.student.username}"
)
mock_info.assert_any_call(f"Revoked certificate for program {1} for user {self.student.username}")
mock_info.assert_any_call(f"Revoked certificate for program {2} for user {self.student.username}")
def test_retry_on_credentials_api_errors(
self,
@@ -859,8 +959,12 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
"""
mock_get_inverted_programs.return_value = self.inverted_programs
mock_get_certified_programs.return_value = [1]
mock_get_certified_programs.side_effect = self._make_side_effect([Exception('boom'), None])
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
mock_get_certified_programs.side_effect = self._make_side_effect(
[Exception("boom"), None]
)
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_get_certified_programs.call_count == 2
assert mock_revoke_program_certificate.call_count == 1
@@ -881,7 +985,9 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
[exception, None]
)
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_revoke_program_certificate.call_count == 3
@@ -902,7 +1008,9 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
[exception, None]
)
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_revoke_program_certificate.call_count == 2
@@ -923,7 +1031,9 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
[exception, None]
)
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_revoke_program_certificate.call_count == 2
@@ -942,47 +1052,52 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC
with mock.patch(
TASKS_MODULE + ".get_credentials_api_client"
) as mock_get_api_client, mock.patch(
TASKS_MODULE + '.LOGGER.exception'
TASKS_MODULE + ".LOGGER.exception"
) as mock_exception:
mock_get_api_client.side_effect = Exception("boom")
with pytest.raises(MaxRetriesExceededError):
tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get()
tasks.revoke_program_certificates.delay(
self.student.username, self.course_key
).get()
assert mock_exception.called
assert mock_get_api_client.call_count == (tasks.MAX_RETRIES + 1)
assert not mock_revoke_program_certificate.called
@skip_unless_lms
@override_settings(CREDENTIALS_SERVICE_USERNAME='test-service-username')
@override_settings(CREDENTIALS_SERVICE_USERNAME="test-service-username")
class UpdateCredentialsCourseCertificateConfigurationAvailableDateTestCase(TestCase):
"""
Tests for the update_credentials_course_certificate_configuration_available_date function
"""
def setUp(self):
super().setUp()
self.course = CourseOverviewFactory.create(
certificate_available_date=datetime.now().strftime('%Y-%m-%dT%H:%M:%SZ')
certificate_available_date=datetime.now().strftime("%Y-%m-%dT%H:%M:%SZ")
)
CourseModeFactory.create(course_id=self.course.id, mode_slug='verified')
CourseModeFactory.create(course_id=self.course.id, mode_slug='audit')
CourseModeFactory.create(course_id=self.course.id, mode_slug="verified")
CourseModeFactory.create(course_id=self.course.id, mode_slug="audit")
self.available_date = self.course.certificate_available_date
self.course_id = self.course.id
self.credentials_worker = UserFactory(username='test-service-username')
self.credentials_worker = UserFactory(username="test-service-username")
def test_update_course_cert_available_date(self):
with mock.patch(TASKS_MODULE + '.post_course_certificate_configuration') as update_posted:
with mock.patch(
TASKS_MODULE + ".post_course_certificate_configuration"
) as update_posted:
tasks.update_credentials_course_certificate_configuration_available_date(
self.course_id,
self.available_date
self.course_id, self.available_date
)
update_posted.assert_called_once()
def test_course_with_two_paid_modes(self):
CourseModeFactory.create(course_id=self.course.id, mode_slug='professional')
with mock.patch(TASKS_MODULE + '.post_course_certificate_configuration') as update_posted:
CourseModeFactory.create(course_id=self.course.id, mode_slug="professional")
with mock.patch(
TASKS_MODULE + ".post_course_certificate_configuration"
) as update_posted:
tasks.update_credentials_course_certificate_configuration_available_date(
self.course_id,
self.available_date
self.course_id, self.available_date
)
update_posted.assert_not_called()
@@ -992,74 +1107,80 @@ class PostCourseCertificateConfigurationTestCase(TestCase):
"""
Test the post_course_certificate_configuration function
"""
def setUp(self):
super().setUp()
self.certificate = {
'mode': 'verified',
'course_id': 'testCourse',
"mode": "verified",
"course_id": "testCourse",
}
@httpretty.activate
@mock.patch('openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url')
@mock.patch("openedx.core.djangoapps.programs.tasks.get_credentials_api_base_url")
def test_post_course_certificate_configuration(self, mock_get_api_base_url):
"""
Ensure the correct API call gets made
"""
mock_get_api_base_url.return_value = 'http://test-server/'
mock_get_api_base_url.return_value = "http://test-server/"
test_client = requests.Session()
test_client.auth = SuppliedJwtAuth('test-token')
test_client.auth = SuppliedJwtAuth("test-token")
httpretty.register_uri(
httpretty.POST,
'http://test-server/course_certificates/',
"http://test-server/course_certificates/",
)
available_date = datetime.now().strftime('%Y-%m-%dT%H:%M:%SZ')
available_date = datetime.now().strftime("%Y-%m-%dT%H:%M:%SZ")
tasks.post_course_certificate_configuration(test_client, self.certificate, available_date)
tasks.post_course_certificate_configuration(
test_client, self.certificate, available_date
)
expected_body = {
"course_id": 'testCourse',
"certificate_type": 'verified',
"course_id": "testCourse",
"certificate_type": "verified",
"certificate_available_date": available_date,
"is_active": True
"is_active": True,
}
last_request_body = httpretty.last_request().body.decode('utf-8')
last_request_body = httpretty.last_request().body.decode("utf-8")
assert json.loads(last_request_body) == expected_body
@skip_unless_lms
class UpdateCertificateVisibleDatesOnCourseUpdateTestCase(CredentialsApiConfigMixin, TestCase):
class UpdateCertificateVisibleDatesOnCourseUpdateTestCase(
CredentialsApiConfigMixin, TestCase
):
"""
Tests for the `update_certificate_visible_date_on_course_update` task.
"""
def setUp(self):
super().setUp()
self.credentials_api_config = self.create_credentials_config(enabled=False)
# setup course
self.course = CourseOverviewFactory.create()
# setup users
self.student1 = UserFactory.create(username='test-student1')
self.student2 = UserFactory.create(username='test-student2')
self.student3 = UserFactory.create(username='test-student3')
self.student1 = UserFactory.create(username="test-student1")
self.student2 = UserFactory.create(username="test-student2")
self.student3 = UserFactory.create(username="test-student3")
# award certificates to users in course we created
self.certificate_student1 = GeneratedCertificateFactory.create(
user=self.student1,
mode='verified',
mode="verified",
course_id=self.course.id,
status='downloadable'
status="downloadable",
)
self.certificate_student2 = GeneratedCertificateFactory.create(
user=self.student2,
mode='verified',
mode="verified",
course_id=self.course.id,
status='downloadable'
status="downloadable",
)
self.certificate_student3 = GeneratedCertificateFactory.create(
user=self.student3,
mode='verified',
mode="verified",
course_id=self.course.id,
status='downloadable'
status="downloadable",
)
def tearDown(self):
@@ -1075,7 +1196,9 @@ class UpdateCertificateVisibleDatesOnCourseUpdateTestCase(CredentialsApiConfigMi
exception when the max number of retries has reached.
"""
with pytest.raises(MaxRetriesExceededError):
tasks.update_certificate_visible_date_on_course_update(self.course.id) # pylint: disable=no-value-for-parameter
tasks.update_certificate_visible_date_on_course_update(
self.course.id
) # pylint: disable=no-value-for-parameter
def test_update_visible_dates(self):
"""
@@ -1089,17 +1212,24 @@ class UpdateCertificateVisibleDatesOnCourseUpdateTestCase(CredentialsApiConfigMi
self.credentials_api_config.enabled = True
self.credentials_api_config.enable_learner_issuance = True
with mock.patch(f"{TASKS_MODULE}.award_course_certificate.delay") as award_course_cert:
tasks.update_certificate_visible_date_on_course_update(self.course.id) # pylint: disable=no-value-for-parameter
with mock.patch(
f"{TASKS_MODULE}.award_course_certificate.delay"
) as award_course_cert:
tasks.update_certificate_visible_date_on_course_update(
self.course.id
) # pylint: disable=no-value-for-parameter
assert award_course_cert.call_count == 3
@skip_unless_lms
class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigMixin, TestCase):
class UpdateCertificateAvailableDateOnCourseUpdateTestCase(
CredentialsApiConfigMixin, TestCase
):
"""
Tests for the `update_certificate_available_date_on_course_update` task.
"""
def setUp(self):
super().setUp()
self.credentials_api_config = self.create_credentials_config(enabled=False)
@@ -1119,7 +1249,9 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM
course = CourseOverviewFactory.create()
with pytest.raises(MaxRetriesExceededError):
tasks.update_certificate_available_date_on_course_update(course.id) # pylint: disable=no-value-for-parameter
tasks.update_certificate_available_date_on_course_update(
course.id
) # pylint: disable=no-value-for-parameter
def test_update_certificate_available_date_with_self_paced_course(self):
"""
@@ -1133,16 +1265,19 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM
self.credentials_api_config.enable_learner_issuance = True
course = CourseOverviewFactory.create(
self_paced=True,
certificate_available_date=None
self_paced=True, certificate_available_date=None
)
with mock.patch(
f"{TASKS_MODULE}.update_credentials_course_certificate_configuration_available_date.delay"
) as update_credentials_course_cert_config:
tasks.update_certificate_available_date_on_course_update(course.id) # pylint: disable=no-value-for-parameter
tasks.update_certificate_available_date_on_course_update(
course.id
) # pylint: disable=no-value-for-parameter
update_credentials_course_cert_config.assert_called_once_with(str(course.id), None)
update_credentials_course_cert_config.assert_called_once_with(
str(course.id), None
)
def test_update_certificate_available_date_with_instructor_paced_course(self):
"""
@@ -1160,15 +1295,19 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM
course = CourseOverviewFactory.create(
self_paced=False,
certificate_available_date=available_date,
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE
certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE,
)
with mock.patch(
f"{TASKS_MODULE}.update_credentials_course_certificate_configuration_available_date.delay"
) as update_credentials_course_cert_config:
tasks.update_certificate_available_date_on_course_update(course.id) # pylint: disable=no-value-for-parameter
tasks.update_certificate_available_date_on_course_update(
course.id
) # pylint: disable=no-value-for-parameter
update_credentials_course_cert_config.assert_called_once_with(str(course.id), str(available_date))
update_credentials_course_cert_config.assert_called_once_with(
str(course.id), str(available_date)
)
def test_update_certificate_available_date_with_expect_no_update(self):
"""
@@ -1183,7 +1322,7 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM
course = CourseOverviewFactory.create(
self_paced=False,
certificate_available_date=available_date,
certificates_display_behavior=CertificatesDisplayBehaviors.EARLY_NO_INFO
certificates_display_behavior=CertificatesDisplayBehaviors.EARLY_NO_INFO,
)
expected_message = (
@@ -1192,7 +1331,9 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM
)
with LogCapture(level=logging.WARNING) as log_capture:
tasks.update_certificate_available_date_on_course_update(course.id) # pylint: disable=no-value-for-parameter
tasks.update_certificate_available_date_on_course_update(
course.id
) # pylint: disable=no-value-for-parameter
assert len(log_capture.records) == 1
assert log_capture.records[0].getMessage() == expected_message

54
openedx/core/lib/edx_api_utils.py
View File

@@ -2,12 +2,16 @@
import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
from urllib.parse import urljoin
from django.core.cache import cache
from openedx.core.lib.cache_utils import zpickle, zunpickle
if TYPE_CHECKING:
from requests import session
log = logging.getLogger(__name__)
@@ -19,9 +23,20 @@ def get_fields(fields, response):
return results
def get_api_data(api_config, resource, api_client, base_api_url, resource_id=None,
querystring=None, cache_key=None, many=True,
traverse_pagination=True, fields=None, long_term_cache=False):
def get_api_data(
api_config: Any,
resource: str,
api_client: "session",
base_api_url: str,
resource_id: Optional[Union[int, str]] = None,
querystring: Optional[Dict[Any, Any]] = None,
cache_key: Optional[str] = None,
many: bool = True,
traverse_pagination: bool = True,
fields: Optional[List[Any]] = None,
long_term_cache: bool = False,
raise_on_error: bool = False,
) -> Union[List[Any], Dict[Any, Any]]:
"""
GET data from an edX REST API endpoint using the API client.
@@ -43,21 +58,26 @@ def get_api_data(api_config, resource, api_client, base_api_url, resource_id=Non
many (bool): Whether the resource requested is a collection of objects, or a single object.
If false, an empty dict will be returned in cases of failure rather than the default empty list.
traverse_pagination (bool): Whether to traverse pagination or return paginated response..
fields (list): Return only specific fields from the response
long_term_cache (bool): Whether to use the long term cache ttl or the standard cache ttl
raise_on_error (bool): Reraise errors back to the caller, instead if returning empty results.
Returns:
Data returned by the API. When hitting a list endpoint, extracts "results" (list of dict)
returned by DRF-powered APIs.
returned by DRF-powered APIs. By default, returns an empty result if the called API
returns an error.
"""
no_data = [] if many else {}
if not api_config.enabled:
log.warning('%s configuration is disabled.', api_config.API_NAME)
log.warning("%s configuration is disabled.", api_config.API_NAME)
return no_data
if cache_key:
cache_key = f'{cache_key}.{resource_id}' if resource_id is not None else cache_key
cache_key += '.zpickled'
cache_key = (
f"{cache_key}.{resource_id}" if resource_id is not None else cache_key
)
cache_key += ".zpickled"
cached = cache.get(cache_key)
if cached:
@@ -77,19 +97,23 @@ def get_api_data(api_config, resource, api_client, base_api_url, resource_id=Non
querystring = querystring if querystring else {}
api_url = urljoin(
f"{base_api_url}/",
f"{resource}/{str(resource_id) + '/' if resource_id is not None else ''}"
f"{resource}/{str(resource_id) + '/' if resource_id is not None else ''}",
)
response = api_client.get(api_url, params=querystring)
response.raise_for_status()
response = response.json()
if resource_id is None and traverse_pagination:
results = _traverse_pagination(response, api_client, api_url, querystring, no_data)
results = _traverse_pagination(
response, api_client, api_url, querystring, no_data
)
else:
results = response
except: # pylint: disable=bare-except
log.exception('Failed to retrieve data from the %s API.', api_config.API_NAME)
log.exception("Failed to retrieve data from the %s API.", api_config.API_NAME)
if raise_on_error:
raise
return no_data
if cache_key:
@@ -111,17 +135,17 @@ def _traverse_pagination(response, api_client, api_url, querystring, no_data):
Extracts and concatenates "results" (list of dict) returned by DRF-powered APIs.
"""
results = response.get('results', no_data)
results = response.get("results", no_data)
page = 1
next_page = response.get('next')
next_page = response.get("next")
while next_page:
page += 1
querystring['page'] = page
querystring["page"] = page
response = api_client.get(api_url, params=querystring)
response.raise_for_status()
response = response.json()
results += response.get('results', no_data)
next_page = response.get('next')
results += response.get("results", no_data)
next_page = response.get("next")
return results

243
openedx/core/lib/tests/test_edx_api_utils.py
View File

@@ -7,6 +7,7 @@ from urllib.parse import urljoin
import httpretty
from django.core.cache import cache
from requests.exceptions import HTTPError
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.catalog.models import CatalogIntegration
@@ -16,29 +17,36 @@ from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfi
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from openedx.core.lib.edx_api_utils import get_api_data
UTILITY_MODULE = 'openedx.core.lib.edx_api_utils'
TEST_API_URL = 'http://www-internal.example.com/api'
UTILITY_MODULE = "openedx.core.lib.edx_api_utils"
TEST_API_URL = "http://www-internal.example.com/api"
@skip_unless_lms
@httpretty.activate
class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, CacheIsolationTestCase):
class TestGetEdxApiData(
CatalogIntegrationMixin, CredentialsApiConfigMixin, CacheIsolationTestCase
):
"""
Tests for edX API data retrieval utility.
"""
ENABLED_CACHES = ['default']
ENABLED_CACHES = ["default"]
def setUp(self):
super().setUp()
self.user = UserFactory()
self.base_api_url = CatalogIntegration.current().get_internal_api_url().strip('/')
self.base_api_url = (
CatalogIntegration.current().get_internal_api_url().strip("/")
)
httpretty.httpretty.reset()
cache.clear()
def _mock_catalog_api(self, responses, url=None):
assert httpretty.is_enabled(), 'httpretty must be enabled to mock Catalog API calls.'
assert (
httpretty.is_enabled()
), "httpretty must be enabled to mock Catalog API calls."
url = url if url else urljoin(f"{self.base_api_url}/", "programs/")
@@ -57,19 +65,22 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
catalog_integration = self.create_catalog_integration()
api = get_catalog_api_client(self.user)
expected_collection = ['some', 'test', 'data']
expected_collection = ["some", "test", "data"]
data = {
'next': None,
'results': expected_collection,
"next": None,
"results": expected_collection,
}
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(data), content_type='application/json')]
[httpretty.Response(body=json.dumps(data), content_type="application/json")]
)
with mock.patch('requests.Session') as mock_init:
with mock.patch("requests.Session") as mock_init:
actual_collection = get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
)
# Verify that the helper function didn't initialize its own client.
@@ -86,25 +97,30 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
catalog_integration = self.create_catalog_integration()
api = get_catalog_api_client(self.user)
expected_collection = ['some', 'test', 'data']
expected_collection = ["some", "test", "data"]
url = urljoin(f"{self.base_api_url}/", "/programs/?page={}")
responses = []
for page, record in enumerate(expected_collection, start=1):
data = {
'next': url.format(page + 1) if page < len(expected_collection) else None,
'results': [record],
"next": url.format(page + 1)
if page < len(expected_collection)
else None,
"results": [record],
}
body = json.dumps(data)
responses.append(
httpretty.Response(body=body, content_type='application/json')
httpretty.Response(body=body, content_type="application/json")
)
self._mock_catalog_api(responses)
actual_collection = get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
)
assert actual_collection == expected_collection
@@ -120,22 +136,31 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
url = urljoin(f"{self.base_api_url}/", "/programs/?page={}")
responses = [
{
'next': url.format(2),
'results': ['some'],
"next": url.format(2),
"results": ["some"],
},
{
'next': url.format(None),
'results': ['test'],
"next": url.format(None),
"results": ["test"],
},
]
expected_response = responses[0]
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(body), content_type='application/json') for body in responses]
[
httpretty.Response(
body=json.dumps(body), content_type="application/json"
)
for body in responses
]
)
actual_collection = get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url, traverse_pagination=False
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
traverse_pagination=False,
)
assert actual_collection == expected_response
self._assert_num_requests(1)
@@ -150,15 +175,23 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
resource_id = 1
url = urljoin(f"{self.base_api_url}/", f"programs/{resource_id}/")
expected_resource = {'key': 'value'}
expected_resource = {"key": "value"}
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')],
url=url
[
httpretty.Response(
body=json.dumps(expected_resource), content_type="application/json"
)
],
url=url,
)
actual_resource = get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url, resource_id=resource_id
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=resource_id,
)
assert actual_resource == expected_resource
@@ -180,15 +213,23 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
resource_id = 0
url = urljoin(f"{self.base_api_url}/", f"programs/{resource_id}/")
expected_resource = {'key': 'value', 'results': []}
expected_resource = {"key": "value", "results": []}
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')],
url=url
[
httpretty.Response(
body=json.dumps(expected_resource), content_type="application/json"
)
],
url=url,
)
actual_resource = get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url, resource_id=resource_id
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=resource_id,
)
assert actual_resource == expected_resource
@@ -201,17 +242,21 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
catalog_integration = self.create_catalog_integration(cache_ttl=5)
api = get_catalog_api_client(self.user)
response = {'lang': 'en', 'weeks_to_complete': '5'}
response = {"lang": "en", "weeks_to_complete": "5"}
resource_id = 'course-v1:testX+testABC+1T2019'
resource_id = "course-v1:testX+testABC+1T2019"
url = urljoin(f"{self.base_api_url}/", f"course_runs/{resource_id}/")
expected_resource_for_lang = {'lang': 'en'}
expected_resource_for_weeks_to_complete = {'weeks_to_complete': '5'}
expected_resource_for_lang = {"lang": "en"}
expected_resource_for_weeks_to_complete = {"weeks_to_complete": "5"}
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(response), content_type='application/json')],
url=url
[
httpretty.Response(
body=json.dumps(response), content_type="application/json"
)
],
url=url,
)
cache_key = CatalogIntegration.current().CACHE_KEY
@@ -219,24 +264,24 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
# get response and set the cache.
actual_resource_for_lang = get_api_data(
catalog_integration,
'course_runs',
"course_runs",
resource_id=resource_id,
api_client=api,
base_api_url=self.base_api_url,
cache_key=cache_key,
fields=['lang']
fields=["lang"],
)
assert actual_resource_for_lang == expected_resource_for_lang
# Hit the cache
actual_resource = get_api_data(
catalog_integration,
'course_runs',
"course_runs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=resource_id,
cache_key=cache_key,
fields=['weeks_to_complete']
fields=["weeks_to_complete"],
)
assert actual_resource == expected_resource_for_weeks_to_complete
@@ -251,73 +296,94 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
catalog_integration = self.create_catalog_integration(cache_ttl=5)
api = get_catalog_api_client(self.user)
expected_collection = ['some', 'test', 'data']
expected_collection = ["some", "test", "data"]
data = {
'next': None,
'results': expected_collection,
"next": None,
"results": expected_collection,
}
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(data), content_type='application/json')],
[
httpretty.Response(
body=json.dumps(data), content_type="application/json"
)
],
)
resource_id = 1
url = urljoin(f"{self.base_api_url}/", f"programs/{resource_id}/")
expected_resource = {'key': 'value'}
expected_resource = {"key": "value"}
self._mock_catalog_api(
[httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')],
url=url
[
httpretty.Response(
body=json.dumps(expected_resource), content_type="application/json"
)
],
url=url,
)
cache_key = CatalogIntegration.current().CACHE_KEY
# Warm up the cache.
get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url, cache_key=cache_key
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
cache_key=cache_key,
)
get_api_data(
catalog_integration,
'programs',
"programs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=resource_id,
cache_key=cache_key
cache_key=cache_key,
)
# Hit the cache.
actual_collection = get_api_data(
catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url, cache_key=cache_key
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
cache_key=cache_key,
)
assert actual_collection == expected_collection
actual_resource = get_api_data(
catalog_integration,
'programs',
"programs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=resource_id,
cache_key=cache_key
cache_key=cache_key,
)
assert actual_resource == expected_resource
# Verify that only two requests were made, not four.
self._assert_num_requests(2)
@mock.patch(UTILITY_MODULE + '.log.warning')
@mock.patch(UTILITY_MODULE + ".log.warning")
def test_api_config_disabled(self, mock_warning):
"""
Verify that no data is retrieved if the provided config model is disabled.
"""
catalog_integration = self.create_catalog_integration(enabled=False)
actual = get_api_data(catalog_integration, 'programs', api_client=None, base_api_url=self.base_api_url)
actual = get_api_data(
catalog_integration,
"programs",
api_client=None,
base_api_url=self.base_api_url,
)
assert mock_warning.called
assert actual == []
@mock.patch(UTILITY_MODULE + '.log.exception')
@mock.patch(UTILITY_MODULE + ".log.exception")
def test_data_retrieval_failure(self, mock_exception):
"""
Verify that an exception is logged when data can't be retrieved.
@@ -326,15 +392,24 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
api = get_catalog_api_client(self.user)
self._mock_catalog_api(
[httpretty.Response(body='clunk', content_type='application/json', status_code=500)]
[
httpretty.Response(
body="clunk", content_type="application/json", status_code=500
)
]
)
actual = get_api_data(catalog_integration, 'programs', api_client=api, base_api_url=self.base_api_url)
actual = get_api_data(
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
)
assert mock_exception.called
assert actual == []
@mock.patch(UTILITY_MODULE + '.log.warning')
@mock.patch(UTILITY_MODULE + ".log.warning")
def test_api_config_disabled_with_id_and_not_collection(self, mock_warning):
"""
Verify that no data is retrieved if the provided config model is disabled.
@@ -343,17 +418,17 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
actual = get_api_data(
catalog_integration,
'programs',
"programs",
api_client=None,
base_api_url=self.base_api_url,
resource_id=100,
many=False
many=False,
)
assert mock_warning.called
assert actual == {}
@mock.patch(UTILITY_MODULE + '.log.exception')
@mock.patch(UTILITY_MODULE + ".log.exception")
def test_data_retrieval_failure_with_id(self, mock_exception):
"""
Verify that an exception is logged when data can't be retrieved.
@@ -362,16 +437,52 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
api = get_catalog_api_client(self.user)
self._mock_catalog_api(
[httpretty.Response(body='clunk', content_type='application/json', status_code=500)]
[
httpretty.Response(
body="clunk", content_type="application/json", status_code=500
)
]
)
actual = get_api_data(
catalog_integration,
'programs',
"programs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=100,
many=False
many=False,
)
assert mock_exception.called
assert actual == {}
def test_data_retrieval_raise_on_error(self):
"""
Verify that when raise_on_error is set and the API call gives an error response,
we raise HttpError.
"""
catalog_integration = self.create_catalog_integration()
api = get_catalog_api_client(self.user)
url = urljoin(f"{self.base_api_url}/", "/programs/100")
self._mock_catalog_api(
[
httpretty.Response(
body="clunk",
content_type="application/json",
status_code=500,
url=url,
)
]
)
with self.assertRaises(Exception):
get_api_data(
catalog_integration,
"programs",
api_client=api,
base_api_url=self.base_api_url,
resource_id=100,
many=False,
raise_on_error=True,
)