From ae3ce9c498017d9556edc5850e2c76b2f460866c Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Fri, 26 Jan 2024 22:20:30 +0000 Subject: [PATCH 1/6] feat: fix exception handling in program cert revocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- openedx/core/djangoapps/credentials/utils.py | 80 ++- openedx/core/djangoapps/programs/tasks.py | 346 +++++----- .../djangoapps/programs/tests/test_tasks.py | 605 +++++++++++------- openedx/core/lib/edx_api_utils.py | 54 +- openedx/core/lib/tests/test_edx_api_utils.py | 243 +++++-- 5 files changed, 840 insertions(+), 488 deletions(-) diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index b17b7ffffac3..437f7c8c95f0 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 a22bff876de4..2431f529d3e2 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 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.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 @@ # 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 _retry_with_custom_exception(username, reason, countdown): 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 _retry_with_custom_exception(username, reason, countdown): # 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 _retry_with_custom_exception(username, reason, countdown): # 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 _retry_with_custom_exception(username, reason, countdown): 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 _retry_with_custom_exception(username, reason, countdown): 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 _retry_with_custom_exception(username, reason, countdown): ) 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 _retry_with_custom_exception(username, reason, countdown): 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 _retry_with_custom_exception(username, course_run_key, reason, countdown): 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 _retry_with_custom_exception(username, course_run_key, reason, countdown): 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 _retry_with_custom_exception(username, course_run_key, reason, countdown): 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 _retry_with_custom_exception(username, course_key, reason, countdown): 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 _retry_with_custom_exception(username, course_key, reason, countdown): 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 _retry_with_custom_exception(username, course_key, reason, countdown): 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 _retry_with_custom_exception(username, course_key, reason, countdown): username, course_key, reason=error_msg, - countdown=rate_limit_countdown + countdown=rate_limit_countdown, ) from exc else: LOGGER.exception( @@ -697,12 +747,16 @@ def _retry_with_custom_exception(username, course_key, reason, countdown): ) 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 _retry_with_custom_exception(username, course_key, reason, countdown): 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( diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index 7e0db4fc6ac5..ba9c915aa1fe 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -19,23 +19,31 @@ 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 @@ def make_credential_result(self, **kwargs): 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 @@ def test_awarding_certs( 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 @@ def test_awarding_certs_with_skip_program_certificate( 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 @@ def test_awarding_certs_with_skip_program_certificate( 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 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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 @@ def test_abort_if_no_completed_programs( 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 @@ def test_programs_without_certificates( 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 @@ def test_continue_awarding_certs_if_error( """ 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 @@ def test_retry_on_programs_api_errors( 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 @@ def test_retry_on_credentials_api_errors( """ 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 @@ def setUp(self): 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 @@ def _add_certificate_date_override(self): """ 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 @@ def test_award_course_certificates(self, mode, mock_post_course_certificate): """ 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 @@ def test_inverted_programs( 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 @@ def test_revoke_program_certificate( """ 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 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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 @@ def test_abort_if_no_program( 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 @@ def test_continue_revoking_certs_if_error( """ 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 @@ def test_retry_on_credentials_api_errors( """ 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 @@ def test_retry_on_credentials_api_429_error( [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 @@ def test_no_retry_on_credentials_api_404_error( [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 @@ def test_no_retry_on_credentials_api_4XX_error( [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 @@ def test_get_api_client_failure_retries( 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 @@ def test_update_visible_dates_but_credentials_config_disabled(self): 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 @@ def test_update_visible_dates(self): 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 @@ def test_update_certificate_available_date_but_credentials_config_disabled(self) 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 @@ def test_update_certificate_available_date_with_self_paced_course(self): 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 @@ def test_update_certificate_available_date_with_instructor_paced_course(self): 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 @@ def test_update_certificate_available_date_with_expect_no_update(self): 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 @@ def test_update_certificate_available_date_with_expect_no_update(self): ) 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 diff --git a/openedx/core/lib/edx_api_utils.py b/openedx/core/lib/edx_api_utils.py index b34780f78b5e..90c45ce17eaa 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 5a71eeb1e70b..86aa17213cdd 100644 --- a/openedx/core/lib/tests/test_edx_api_utils.py +++ b/openedx/core/lib/tests/test_edx_api_utils.py @@ -7,6 +7,7 @@ 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.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 @@ def test_get_unpaginated_data(self): 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 @@ def test_get_paginated_data(self): 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 @@ def test_get_paginated_data_do_not_traverse_pagination(self): 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 @@ def test_get_specific_resource(self): 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 @@ def test_get_specific_resource_with_falsey_id(self): 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 @@ def test_get_specific_fields_from_cache_response(self): 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 @@ def test_get_specific_fields_from_cache_response(self): # 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 @@ def test_cache_utilization(self): 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 @@ def test_data_retrieval_failure(self, mock_exception): 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 @@ def test_api_config_disabled_with_id_and_not_collection(self, mock_warning): 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 @@ def test_data_retrieval_failure_with_id(self, mock_exception): 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, + ) From 918f32ab405553d8697d83f84f7fac8bf7cbc01d Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Tue, 30 Jan 2024 15:53:29 +0000 Subject: [PATCH 2/6] feat: fixed one test * fixed one test to accommodate a slightly modified workflow * allowed the automated linter to match current standards Note: as part of this process, I realized a lot of the tests in `programs/tests/test_tasks.py` are somewhat problematic. Some aren't real unit tests (allow calls to something other than the system under test), at least one for that reason it won't run on local at all, and some mock the wrong part of the system or just don't match current flow. I'm not modifying them as part of this, but they should be looked at. FIXES: APER-3146 --- openedx/core/djangoapps/programs/tasks.py | 4 +- .../djangoapps/programs/tests/test_tasks.py | 605 +++++++----------- 2 files changed, 234 insertions(+), 375 deletions(-) diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index 2431f529d3e2..6717ef3298c1 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -482,7 +482,6 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): reason=error_msg, countdown=countdown, ) - try: course_key = CourseKey.from_string(course_run_key) try: @@ -496,7 +495,8 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): # 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( diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index ba9c915aa1fe..7e0db4fc6ac5 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -19,31 +19,23 @@ 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 @@ -57,101 +49,84 @@ def make_credential_result(self, **kwargs): 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. """ @@ -159,11 +134,11 @@ class AwardProgramCertificatesTestCase( 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( @@ -202,26 +177,20 @@ def test_awarding_certs( 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 @@ -235,7 +204,9 @@ def test_awarding_certs_with_skip_program_certificate( 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 @@ -244,31 +215,28 @@ def test_awarding_certs_with_skip_program_certificate( 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 @@ -280,8 +248,8 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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 @@ -302,13 +270,13 @@ def test_abort_if_no_completed_programs( 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 @@ -321,22 +289,22 @@ def test_programs_without_certificates( 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() @@ -379,30 +347,25 @@ def test_continue_awarding_certs_if_error( """ 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 - ) - ) - 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}" + '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 + self, + mock_get_completed_programs, + *_mock_helpers ): """ Ensures that any otherwise-unhandled errors that arise while trying @@ -410,9 +373,7 @@ def test_retry_on_programs_api_errors( 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 @@ -430,9 +391,7 @@ def test_retry_on_credentials_api_errors( """ 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 @@ -505,68 +464,59 @@ 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 @@ -579,21 +529,21 @@ def setUp(self): 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): @@ -602,12 +552,12 @@ def _add_certificate_date_override(self): """ 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): """ @@ -615,119 +565,89 @@ def test_award_course_certificates(self, mode, mock_post_course_certificate): """ 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 @@ -738,44 +658,42 @@ 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. """ @@ -783,15 +701,15 @@ class RevokeProgramCertificatesTestCase( 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): """ @@ -824,9 +742,7 @@ def test_inverted_programs( 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( @@ -841,37 +757,34 @@ def test_revoke_program_certificate( """ 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 @@ -881,10 +794,8 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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 @@ -900,9 +811,7 @@ def test_abort_if_no_program( 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 @@ -921,29 +830,20 @@ def test_continue_revoking_certs_if_error( """ 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 - ) - ) - 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}" + '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}") def test_retry_on_credentials_api_errors( self, @@ -959,12 +859,8 @@ def test_retry_on_credentials_api_errors( """ 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 @@ -985,9 +881,7 @@ def test_retry_on_credentials_api_429_error( [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 @@ -1008,9 +902,7 @@ def test_no_retry_on_credentials_api_404_error( [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 @@ -1031,9 +923,7 @@ def test_no_retry_on_credentials_api_4XX_error( [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 @@ -1052,52 +942,47 @@ def test_get_api_client_failure_retries( 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() @@ -1107,80 +992,74 @@ 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): @@ -1196,9 +1075,7 @@ def test_update_visible_dates_but_credentials_config_disabled(self): 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): """ @@ -1212,24 +1089,17 @@ def test_update_visible_dates(self): 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) @@ -1249,9 +1119,7 @@ def test_update_certificate_available_date_but_credentials_config_disabled(self) 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): """ @@ -1265,19 +1133,16 @@ def test_update_certificate_available_date_with_self_paced_course(self): 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): """ @@ -1295,19 +1160,15 @@ def test_update_certificate_available_date_with_instructor_paced_course(self): 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): """ @@ -1322,7 +1183,7 @@ def test_update_certificate_available_date_with_expect_no_update(self): 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 = ( @@ -1331,9 +1192,7 @@ def test_update_certificate_available_date_with_expect_no_update(self): ) 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 From bde84117c36c38eea9df87d6ca457574c57cb1a9 Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Tue, 30 Jan 2024 16:09:12 +0000 Subject: [PATCH 3/6] feat: linting error * unused import * format FIXES: APER-3146 --- .../djangoapps/programs/tests/test_tasks.py | 606 +++++++++++------- openedx/core/lib/tests/test_edx_api_utils.py | 1 - 2 files changed, 367 insertions(+), 240 deletions(-) diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index 7e0db4fc6ac5..5a2518af78c2 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -19,23 +19,31 @@ 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,36 @@ def make_credential_result(self, **kwargs): 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,50 +95,55 @@ 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 +151,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 +194,26 @@ def test_awarding_certs( 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 +227,7 @@ def test_awarding_certs_with_skip_program_certificate( 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 +236,31 @@ def test_awarding_certs_with_skip_program_certificate( 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 +272,8 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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 +294,13 @@ def test_abort_if_no_completed_programs( 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 +313,22 @@ def test_programs_without_certificates( 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 +371,30 @@ def test_continue_awarding_certs_if_error( """ 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 +402,9 @@ def test_retry_on_programs_api_errors( 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 +422,9 @@ def test_retry_on_credentials_api_errors( """ 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 +497,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 +571,21 @@ def setUp(self): 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 +594,12 @@ def _add_certificate_date_override(self): """ 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 +607,119 @@ def test_award_course_certificates(self, mode, mock_post_course_certificate): """ 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 +730,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,29 +775,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: @@ -742,7 +811,9 @@ def test_inverted_programs( 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 +828,37 @@ def test_revoke_program_certificate( """ 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 +868,10 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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 +887,9 @@ def test_abort_if_no_program( 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 +908,29 @@ def test_continue_revoking_certs_if_error( """ 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 +946,12 @@ def test_retry_on_credentials_api_errors( """ 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 +972,9 @@ def test_retry_on_credentials_api_429_error( [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 +995,9 @@ def test_no_retry_on_credentials_api_404_error( [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 +1018,9 @@ def test_no_retry_on_credentials_api_4XX_error( [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 +1039,52 @@ def test_get_api_client_failure_retries( 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 +1094,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 +1183,9 @@ def test_update_visible_dates_but_credentials_config_disabled(self): 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 +1199,24 @@ def test_update_visible_dates(self): 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 +1236,9 @@ def test_update_certificate_available_date_but_credentials_config_disabled(self) 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 +1252,19 @@ def test_update_certificate_available_date_with_self_paced_course(self): 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 +1282,19 @@ def test_update_certificate_available_date_with_instructor_paced_course(self): 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 +1309,7 @@ def test_update_certificate_available_date_with_expect_no_update(self): 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 +1318,9 @@ def test_update_certificate_available_date_with_expect_no_update(self): ) 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 diff --git a/openedx/core/lib/tests/test_edx_api_utils.py b/openedx/core/lib/tests/test_edx_api_utils.py index 86aa17213cdd..68601caba0d6 100644 --- a/openedx/core/lib/tests/test_edx_api_utils.py +++ b/openedx/core/lib/tests/test_edx_api_utils.py @@ -7,7 +7,6 @@ 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 From adae7e3e25390deb859913694f794b2814e5fb42 Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Tue, 30 Jan 2024 17:39:57 +0000 Subject: [PATCH 4/6] feat: relinting Trying with new autoformat settings. FIXES: APER-3146 --- openedx/core/djangoapps/programs/tasks.py | 186 ++++------- .../djangoapps/programs/tests/test_tasks.py | 300 +++++------------- 2 files changed, 128 insertions(+), 358 deletions(-) diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index 6717ef3298c1..c8f1e6f97f9a 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -128,9 +128,7 @@ def award_program_certificate(client, user, program_uuid, visible_date): "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)} - ], + "attributes": [{"name": "visible_date", "value": visible_date.strftime(DATE_FORMAT)}], }, ) response.raise_for_status() @@ -138,9 +136,7 @@ def award_program_certificate(client, user, program_uuid, visible_date): @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 @@ -172,9 +168,7 @@ def _retry_with_custom_exception(username, reason, countdown): 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 @@ -187,19 +181,17 @@ def _retry_with_custom_exception(username, reason, countdown): # 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" - LOGGER.warning(error_msg) - raise _retry_with_custom_exception( - username=username, reason=error_msg, countdown=countdown + 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) 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 = {} @@ -208,9 +200,7 @@ def _retry_with_custom_exception(username, reason, countdown): 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. @@ -218,16 +208,12 @@ def _retry_with_custom_exception(username, reason, countdown): # 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. @@ -235,9 +221,7 @@ def _retry_with_custom_exception(username, reason, countdown): # 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( @@ -247,23 +231,15 @@ def _retry_with_custom_exception(username, reason, countdown): 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( @@ -290,16 +266,12 @@ def _retry_with_custom_exception(username, reason, countdown): ) 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. @@ -307,20 +279,14 @@ def _retry_with_custom_exception(username, reason, countdown): 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. @@ -333,9 +299,7 @@ def post_course_certificate_configuration( 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, @@ -360,9 +324,7 @@ def post_course_certificate_configuration( 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 """ @@ -373,20 +335,14 @@ def post_course_certificate( api_url, json={ "username": username, - "status": "awarded" - if certificate.is_valid() - else "revoked", # Only need the two options at this time + "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() @@ -417,15 +373,9 @@ 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." - ) + LOGGER.exception(f"Either course {course_key} has no certificate mode or multiple modes. Task failed.") return credentials_client = get_credentials_api_client( @@ -474,7 +424,9 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): # 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, @@ -487,9 +439,7 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): 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 @@ -546,9 +496,7 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): 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) @@ -560,9 +508,7 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): ) from exc -def get_revokable_program_uuids( - course_specific_programs: List[Dict], student: User -) -> List[str]: +def get_revokable_program_uuids(course_specific_programs: List[Dict], student: User) -> List[str]: """ Get program uuids for which certificate to be revoked. @@ -618,9 +564,7 @@ def revoke_program_certificate(client, username, program_uuid): @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. @@ -654,7 +598,9 @@ def _retry_with_custom_exception(username, course_key, reason, countdown): # 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, @@ -685,13 +631,10 @@ def _retry_with_custom_exception(username, course_key, reason, countdown): 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} " - 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( @@ -710,17 +653,13 @@ def _retry_with_custom_exception(username, course_key, reason, countdown): 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( @@ -742,21 +681,15 @@ def _retry_with_custom_exception(username, course_key, reason, 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}." - ) + 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. @@ -764,15 +697,11 @@ def _retry_with_custom_exception(username, course_key, reason, countdown): 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") - 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) @@ -810,11 +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 " @@ -862,8 +789,7 @@ def update_certificate_available_date_on_course_update(self, course_key): if ( course_overview and course_overview.self_paced is False - and course_overview.certificates_display_behavior - == CertificatesDisplayBehaviors.END_WITH_DATE + 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 " @@ -880,18 +806,12 @@ 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}" ) - 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( diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index 5a2518af78c2..ca975801328f 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -77,16 +77,12 @@ def test_get_certified_programs(self, mock_get_credentials): """ 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].get("credential_type", None) == "program" - ) + assert mock_get_credentials.call_args[1].get("credential_type", None) == "program" assert result == [1] @@ -113,9 +109,7 @@ def test_award_program_certificate(self, mock_get_api_base_url): "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, @@ -141,9 +135,7 @@ def test_award_program_certificate(self, mock_get_api_base_url): @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 -): +class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """ Tests for the 'award_program_certificates' celery task. """ @@ -194,20 +186,14 @@ def test_awarding_certs( 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, @@ -236,13 +222,9 @@ def test_awarding_certs_with_skip_program_certificate( 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 @@ -250,16 +232,12 @@ def test_awarding_certs_with_skip_program_certificate( ("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} - ) + 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() @@ -371,9 +349,7 @@ def test_continue_awarding_certs_if_error( """ 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" @@ -386,25 +362,17 @@ def test_continue_awarding_certs_if_error( 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 - ): + 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 @@ -422,9 +390,7 @@ def test_retry_on_credentials_api_errors( """ 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 @@ -441,9 +407,7 @@ def test_retry_on_credentials_api_429_error( 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() @@ -461,9 +425,7 @@ def test_no_retry_on_credentials_api_404_error( 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() @@ -481,9 +443,7 @@ def test_no_retry_on_credentials_api_4XX_error( 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() @@ -525,9 +485,7 @@ def test_post_course_certificate(self, mock_get_api_base_url): 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, @@ -541,9 +499,7 @@ def test_post_course_certificate(self, mock_get_api_base_url): "attributes": [ { "name": "visible_date", - "value": visible_date.strftime( - "%Y-%m-%dT%H:%M:%SZ" - ), # text representation of date + "value": visible_date.strftime("%Y-%m-%dT%H:%M:%SZ"), # text representation of date } ], } @@ -607,107 +563,79 @@ def test_award_course_certificates(self, mode, mock_post_course_certificate): """ 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 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: # 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() + 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: # 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 """ @@ -717,9 +645,7 @@ def test_award_course_cert_not_called_if_certificated_not_verified_mode( 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 @@ -765,9 +691,7 @@ def test_revoke_program_certificate(self, mock_get_api_base_url): @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 -): +class RevokeProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiConfigMixin, TestCase): """ Tests for the 'revoke_program_certificates' celery task. """ @@ -811,9 +735,7 @@ def test_inverted_programs( 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( @@ -827,14 +749,10 @@ def test_revoke_program_certificate( 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() + 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 @@ -844,21 +762,15 @@ def test_revoke_program_certificate( ("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} - ) + 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 @@ -869,9 +781,7 @@ def test_abort_if_invalid_username(self, *mock_helpers): 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() + 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 @@ -887,9 +797,7 @@ def test_abort_if_no_program( 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 @@ -908,16 +816,12 @@ def test_continue_revoking_certs_if_error( """ 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() + 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( @@ -925,12 +829,8 @@ def test_continue_revoking_certs_if_error( 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, @@ -946,12 +846,8 @@ def test_retry_on_credentials_api_errors( """ 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 @@ -968,13 +864,9 @@ def test_retry_on_credentials_api_429_error( 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() + tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() assert mock_revoke_program_certificate.call_count == 3 @@ -991,13 +883,9 @@ def test_no_retry_on_credentials_api_404_error( 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() + tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() assert mock_revoke_program_certificate.call_count == 2 @@ -1014,13 +902,9 @@ def test_no_retry_on_credentials_api_4XX_error( 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() + tasks.revoke_program_certificates.delay(self.student.username, self.course_key).get() assert mock_revoke_program_certificate.call_count == 2 @@ -1036,16 +920,12 @@ def test_get_api_client_failure_retries( 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( + 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): - 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 @@ -1070,9 +950,7 @@ def setUp(self): 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 ) @@ -1080,9 +958,7 @@ def test_update_course_cert_available_date(self): 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: + 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 ) @@ -1119,9 +995,7 @@ def test_post_course_certificate_configuration(self, mock_get_api_base_url): 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", @@ -1134,9 +1008,7 @@ def test_post_course_certificate_configuration(self, mock_get_api_base_url): @skip_unless_lms -class UpdateCertificateVisibleDatesOnCourseUpdateTestCase( - CredentialsApiConfigMixin, TestCase -): +class UpdateCertificateVisibleDatesOnCourseUpdateTestCase(CredentialsApiConfigMixin, TestCase): """ Tests for the `update_certificate_visible_date_on_course_update` task. """ @@ -1183,9 +1055,7 @@ def test_update_visible_dates_but_credentials_config_disabled(self): 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): """ @@ -1199,20 +1069,14 @@ def test_update_visible_dates(self): 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. """ @@ -1236,9 +1100,7 @@ def test_update_certificate_available_date_but_credentials_config_disabled(self) 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): """ @@ -1251,20 +1113,14 @@ def test_update_certificate_available_date_with_self_paced_course(self): 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" ) 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): """ @@ -1288,13 +1144,9 @@ def test_update_certificate_available_date_with_instructor_paced_course(self): 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): """ @@ -1318,9 +1170,7 @@ def test_update_certificate_available_date_with_expect_no_update(self): ) 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 From f7a9efbf6b0d1b9af52d490600da5660047a246c Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Tue, 30 Jan 2024 17:55:15 +0000 Subject: [PATCH 5/6] feat: lint autoformat errors FIXES: APER-3146 --- openedx/core/djangoapps/programs/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index c8f1e6f97f9a..db60103e4f09 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -425,7 +425,7 @@ def _retry_with_custom_exception(username, course_run_key, reason, countdown): # 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" + "Task award_course_certificate cannot be executed when credentials issuance is disabled in API config" ) LOGGER.warning(error_msg) raise _retry_with_custom_exception( From b75f8b0ed1676b493b37219971c742b7695d7abd Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Tue, 30 Jan 2024 21:34:56 +0000 Subject: [PATCH 6/6] feat: tweaks from code review wording, switching to using a constant, fixing a string concat FIXES: APER-3146 --- openedx/core/djangoapps/programs/tasks.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/programs/tasks.py b/openedx/core/djangoapps/programs/tasks.py index db60103e4f09..c8855ba4f49f 100644 --- a/openedx/core/djangoapps/programs/tasks.py +++ b/openedx/core/djangoapps/programs/tasks.py @@ -87,7 +87,7 @@ def get_certified_programs(student: User, raise_on_error: bool = False) -> List[ student: User object representing the student Keyword Arguments: - raise_on_error (bool): Reraise errors back to the caller, instead if returning empty results. + 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 @@ -96,7 +96,7 @@ def get_certified_programs(student: User, raise_on_error: bool = False) -> List[ certified_programs = [] for credential in get_credentials( student, - credential_type="program", + credential_type=PROGRAM_CERTIFICATE, raise_on_error=raise_on_error, ): certified_programs.append(credential["credential"]["program_uuid"]) @@ -244,7 +244,7 @@ def _retry_with_custom_exception(username, reason, countdown): 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"Unable to award certificate to user {username}. The program might not be configured." ) elif exc.response.status_code == 429: rate_limit_countdown = 60