diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index b17b7ffffa..437f7c8c95 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -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 diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index a22bff876d..c8855ba4f4 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -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 of 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_CERTIFICATE, + raise_on_error=raise_on_error, + ): + certified_programs.append(credential["credential"]["program_uuid"]) return certified_programs @@ -118,19 +125,11 @@ 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() @@ -161,24 +160,21 @@ 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, @@ -247,7 +243,7 @@ def award_program_certificates(self, username): # lint-amnesty, pylint: disable except HTTPError as exc: if exc.response.status_code == 404: LOGGER.exception( - f"Certificate for program {program_uuid} could not be found. " + + 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: @@ -261,7 +257,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( @@ -308,11 +304,11 @@ def post_course_certificate_configuration(client, cert_config, certificate_avail 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 @@ -338,21 +334,16 @@ 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 +352,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 @@ -386,22 +375,20 @@ def update_credentials_course_certificate_configuration_available_date( # 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] if len(modes) != 1: - LOGGER.exception( - f'Either course {course_key} has no certificate mode or multiple modes. Task failed.' - ) + LOGGER.exception(f"Either course {course_key} has no certificate mode or multiple modes. Task failed.") return credentials_client = get_credentials_api_client( 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,19 +408,16 @@ 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 @@ -448,9 +432,8 @@ def award_course_certificate(self, username, course_run_key): username=username, course_run_key=course_run_key, reason=error_msg, - countdown=countdown + countdown=countdown, ) - try: course_key = CourseKey.from_string(course_run_key) try: @@ -463,7 +446,7 @@ def award_course_certificate(self, username, course_run_key): try: certificate = GeneratedCertificate.eligible_certificates.get( user=user.id, - course_id=course_key + course_id=course_key, ) except GeneratedCertificate.DoesNotExist: LOGGER.exception( @@ -505,7 +488,12 @@ 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}") @@ -516,11 +504,11 @@ 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 +520,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,13 +554,10 @@ 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() @@ -594,17 +584,14 @@ 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, @@ -619,13 +606,16 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py 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 @@ -644,15 +634,14 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py 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} " - f"with course {course_key}" + f"Failed to determine program certificates to be revoked for user {username} " f"with course {course_key}" ) LOGGER.exception(error_msg) raise _retry_with_custom_exception( username=username, course_key=course_key, reason=error_msg, - countdown=countdown + countdown=countdown, ) from exc if program_uuids_to_revoke: @@ -689,12 +678,10 @@ 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( - f"Unable to revoke certificate for user {username} for program {program_uuid}." - ) + LOGGER.exception(f"Unable to revoke certificate for user {username} for program {program_uuid}.") 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}.") @@ -710,12 +697,7 @@ def revoke_program_certificates(self, username, course_key): # lint-amnesty, py f"Failed to revoke certificate for user {username} " f"for programs {failed_program_certificate_revoke_attempts}" ) - raise _retry_with_custom_exception( - username, - course_key, - reason=error_msg, - countdown=countdown - ) + raise _retry_with_custom_exception(username, course_key, reason=error_msg, countdown=countdown) else: LOGGER.info(f"There is no program certificates for user {username} to revoke") @@ -738,7 +720,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 @@ -757,12 +739,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) - ) + users_with_certificates_in_course = GeneratedCertificate.eligible_available_certificates.filter( + course_id=course_key + ).values_list("user__username", flat=True) LOGGER.info( f"Resending course certificates for learners in course {course_key} to the Credentials service. Queueing " @@ -786,7 +765,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 +787,16 @@ 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 @@ -828,11 +806,7 @@ def update_certificate_available_date_on_course_update(self, course_key): # associated with a `certificate_available_date`. This ends up causing learners' certificate to be incorrectly # 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 - ): + elif 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}" diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index 7e0db4fc6a..ca97580132 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -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,32 +57,32 @@ 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].get("credential_type", None) == "program" assert result == [1] @@ -83,49 +91,50 @@ 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)) 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') +@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 +143,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( @@ -184,13 +193,13 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo 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 +213,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 @@ -222,21 +229,16 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo # 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 - ): + def test_retry_if_config_disabled(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 +250,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 +272,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 +291,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,33 +349,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}") - def test_retry_on_programs_api_errors( - self, - mock_get_completed_programs, - *_mock_helpers - ): + def test_retry_on_programs_api_errors(self, mock_get_completed_programs, *_mock_helpers): """ Ensures that any otherwise-unhandled errors that arise while trying to get completed programs (e.g. network issues or other 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 +390,7 @@ 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 @@ -408,9 +407,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo exception = HTTPError() exception.response = mock.Mock(status_code=429) mock_get_completed_programs.return_value = {1: 1, 2: 2} - mock_award_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) + mock_award_program_certificate.side_effect = self._make_side_effect([exception, None]) tasks.award_program_certificates.delay(self.student.username).get() @@ -428,9 +425,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo exception = HTTPError() exception.response = mock.Mock(status_code=404) mock_get_completed_programs.return_value = {1: 1, 2: 2} - mock_award_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) + mock_award_program_certificate.side_effect = self._make_side_effect([exception, None]) tasks.award_program_certificates.delay(self.student.username).get() @@ -448,9 +443,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo exception = HTTPError() exception.response = mock.Mock(status_code=418) mock_get_completed_programs.return_value = {1: 1, 2: 2} - mock_award_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) + mock_award_program_certificate.side_effect = self._make_side_effect([exception, None]) tasks.award_program_certificates.delay(self.student.username).get() @@ -464,30 +457,30 @@ 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() @@ -495,28 +488,33 @@ class PostCourseCertificateTestCase(TestCase): 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 +527,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 +550,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): """ @@ -600,7 +598,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): 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() assert mock_warning.called @@ -610,9 +608,9 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): """ 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 @@ -621,7 +619,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): 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: + 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 @@ -631,7 +629,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): 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() assert mock_exception.called @@ -643,7 +641,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): """ # 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() @@ -658,41 +656,41 @@ 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') +@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,29 +699,24 @@ 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): + def _make_side_effect(self, side_effects, *args, **kwargs): """ DRY helper. Returns a side effect function for use with mocks that will be called multiple times, permitting Exceptions to be raised (or not) in a specified order. - - See Also: - http://www.voidspace.org.uk/python/mock/examples.html#multiple-calls-with-different-effects - http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.side_effect - """ - def side_effect(*_a): + def side_effect(*args, **kwargs): if side_effects: exc = side_effects.pop(0) if exc: @@ -756,9 +749,7 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC the proper programs. """ expected_program_uuid = 1 - mock_get_inverted_programs.return_value = { - self.course_key: [{'uuid': expected_program_uuid}] - } + mock_get_inverted_programs.return_value = {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() @@ -768,21 +759,16 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC 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 - ): + def test_retry_if_config_disabled(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() assert mock_warning.called @@ -794,8 +780,8 @@ 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 @@ -830,17 +816,18 @@ 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: + 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}") @@ -859,7 +846,7 @@ 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]) + 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 @@ -877,9 +864,7 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC exception.response = mock.Mock(status_code=429) mock_get_inverted_programs.return_value = self.inverted_programs mock_get_certified_programs.return_value = [1, 2] - mock_revoke_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) + mock_revoke_program_certificate.side_effect = self._make_side_effect([exception, None]) tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() @@ -898,9 +883,7 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC exception.response = mock.Mock(status_code=404) mock_get_inverted_programs.return_value = self.inverted_programs mock_get_certified_programs.return_value = [1, 2] - mock_revoke_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) + mock_revoke_program_certificate.side_effect = self._make_side_effect([exception, None]) tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() @@ -919,9 +902,7 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC exception.response = mock.Mock(status_code=418) mock_get_inverted_programs.return_value = self.inverted_programs mock_get_certified_programs.return_value = [1, 2] - mock_revoke_program_certificate.side_effect = self._make_side_effect( - [exception, None] - ) + mock_revoke_program_certificate.side_effect = self._make_side_effect([exception, None]) tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() @@ -939,10 +920,8 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC mock_get_inverted_programs.return_value = self.inverted_programs mock_get_certified_programs.return_value = [1, 2] - with mock.patch( - TASKS_MODULE + ".get_credentials_api_client" - ) as mock_get_api_client, mock.patch( - TASKS_MODULE + '.LOGGER.exception' + with mock.patch(TASKS_MODULE + ".get_credentials_api_client") as mock_get_api_client, mock.patch( + TASKS_MODULE + ".LOGGER.exception" ) as mock_exception: mock_get_api_client.side_effect = Exception("boom") with pytest.raises(MaxRetriesExceededError): @@ -953,36 +932,35 @@ class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiC @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,39 +970,40 @@ 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) 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 @@ -1033,33 +1012,34 @@ class UpdateCertificateVisibleDatesOnCourseUpdateTestCase(CredentialsApiConfigMi """ 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): @@ -1100,6 +1080,7 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM """ 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) @@ -1132,10 +1113,7 @@ class UpdateCertificateAvailableDateOnCourseUpdateTestCase(CredentialsApiConfigM self.credentials_api_config.enabled = True self.credentials_api_config.enable_learner_issuance = True - course = CourseOverviewFactory.create( - self_paced=True, - certificate_available_date=None - ) + course = CourseOverviewFactory.create(self_paced=True, certificate_available_date=None) with mock.patch( f"{TASKS_MODULE}.update_credentials_course_certificate_configuration_available_date.delay" @@ -1160,7 +1138,7 @@ 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( @@ -1183,7 +1161,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 = ( diff --git a/openedx/core/lib/edx_api_utils.py b/openedx/core/lib/edx_api_utils.py index b34780f78b..90c45ce17e 100644 --- a/openedx/core/lib/edx_api_utils.py +++ b/openedx/core/lib/edx_api_utils.py @@ -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 diff --git a/openedx/core/lib/tests/test_edx_api_utils.py b/openedx/core/lib/tests/test_edx_api_utils.py index 5a71eeb1e7..68601caba0 100644 --- a/openedx/core/lib/tests/test_edx_api_utils.py +++ b/openedx/core/lib/tests/test_edx_api_utils.py @@ -16,29 +16,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 +64,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 +96,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 +135,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 +174,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 +212,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 +241,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 +263,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 +295,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 +391,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 +417,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 +436,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, + )