diff --git a/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst b/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst index 97c8073484626d15ff04703da7cb8eefaacb1c50..c96dc66f86b53d08a1d648177e3129f0618abbf8 100644 --- a/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst +++ b/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst @@ -8,10 +8,10 @@ Accepted Background ---------- Users can earn a course certificate in a particular course run (the certificate -is stored in the GeneratedCertificate model). If a user has not earned a certificate +is stored in the *GeneratedCertificate* model). If a user has not earned a certificate but the course staff would like them to have a certificate anyway, the user can be added to the certificate allowlist for the course run. The allowlist is currently -stored in the CertificateWhitelist model, and was previously referred to as the +stored in the *CertificateWhitelist* model, and was previously referred to as the certificate whitelist. Requirements diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 62dd2f51c94a08ef78161d4c17f3cac9acd21e56..a86545d830a1d0d1be3acd3527448aa3ac8d4e37 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -147,22 +147,25 @@ def _can_generate_allowlist_certificate(user, course_key): """ if not is_using_certificate_allowlist(course_key): # This course run is not using the allowlist feature - log.info(f'{course_key} is not using the certificate allowlist. Certificate cannot be generated.') + log.info(f'{course_key} is not using the certificate allowlist. Allowlist certificate cannot be generated' + f'for {user.id}.') return False if not auto_certificate_generation_enabled(): # Automatic certificate generation is globally disabled - log.info('Automatic certificate generation is globally disabled. Certificate cannot be generated.') + log.info(f'Automatic certificate generation is globally disabled. Allowlist certificate cannot be generated' + f'for {user.id} : {course_key}.') return False if CertificateInvalidation.has_certificate_invalidation(user, course_key): # The invalidation list overrides the allowlist - log.info(f'{user.id} : {course_key} is on the certificate invalidation list. Certificate cannot be generated.') + log.info(f'{user.id} : {course_key} is on the certificate invalidation list. Allowlist certificate cannot be ' + f'generated.') return False enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) if enrollment_mode is None: - log.info(f'{user.id} : {course_key} does not have an enrollment. Certificate cannot be generated.') + log.info(f'{user.id} : {course_key} does not have an enrollment. Allowlist certificate cannot be generated.') return False if not modes_api.is_eligible_for_certificate(enrollment_mode): @@ -171,16 +174,16 @@ def _can_generate_allowlist_certificate(user, course_key): return False if not IDVerificationService.user_is_verified(user): - log.info(f'{user.id} does not have a verified id. Certificate cannot be generated.') + log.info(f'{user.id} does not have a verified id. Allowlist certificate cannot be generated for {course_key}.') return False if not _is_on_certificate_allowlist(user, course_key): - log.info(f'{user.id} : {course_key} is not on the certificate allowlist. Certificate cannot be generated.') + log.info(f'{user.id} : {course_key} is not on the certificate allowlist. Allowlist certificate cannot be ' + f'generated.') return False log.info(f'{user.id} : {course_key} is on the certificate allowlist') - cert = GeneratedCertificate.certificate_for_student(user, course_key) - return _can_generate_allowlist_certificate_for_status(cert) + return _can_generate_allowlist_certificate_for_status(user, course_key) def _can_generate_v2_certificate(user, course_key): @@ -228,22 +231,22 @@ def _is_on_certificate_allowlist(user, course_key): return CertificateWhitelist.objects.filter(user=user, course_id=course_key, whitelist=True).exists() -def _can_generate_allowlist_certificate_for_status(cert): +def _can_generate_allowlist_certificate_for_status(user, course_key): """ - Check if the user's certificate status allows certificate generation + Check if the user's certificate status can handle allowlist certificate generation """ + cert = GeneratedCertificate.certificate_for_student(user, course_key) if cert is None: return True if cert.status == CertificateStatuses.downloadable: - log.info('Certificate with status {status} already exists for {user} : {course}, and is NOT eligible for ' - 'allowlist generation. Certificate cannot be generated.' - .format(status=cert.status, user=cert.user.id, course=cert.course_id)) + log.info(f'Certificate with status {cert.status} already exists for {user.id} : {course_key}, and is not ' + f'eligible for allowlist generation. Allowlist certificate cannot be generated as it is already in a ' + f'final state.') return False - log.info('Certificate with status {status} already exists for {user} : {course}, and is eligible for allowlist ' - 'generation' - .format(status=cert.status, user=cert.user.id, course=cert.course_id)) + log.info(f'Certificate with status {cert.status} already exists for {user.id} : {course_key}, and is eligible for ' + f'allowlist generation') return True diff --git a/lms/djangoapps/certificates/services.py b/lms/djangoapps/certificates/services.py index cd9218963aacb2067ea831a192fcc92be9bb7244..44510efefb061f48a3380a9f8fd49a52dd83840e 100644 --- a/lms/djangoapps/certificates/services.py +++ b/lms/djangoapps/certificates/services.py @@ -37,11 +37,6 @@ class CertificateService: course_id=course_key ) generated_certificate.invalidate() - log.info( - 'Certificate invalidated for user %d in course %s', - user_id, - course_key - ) except ObjectDoesNotExist: log.warning( 'Invalidation failed because a certificate for user %d in course %s does not exist.', diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index fbdfb06bc254d9e37ed38b7be6a981a51d43f049..bf28dd6a2505356b86c30ff298e14499467076cc 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -132,20 +132,20 @@ class AllowlistTests(ModuleStoreTestCase): u = UserFactory() cr = CourseFactory() key = cr.id # pylint: disable=no-member - cert = GeneratedCertificateFactory( + GeneratedCertificateFactory( user=u, course_id=key, mode=GeneratedCertificate.MODES.verified, status=status, ) - assert _can_generate_allowlist_certificate_for_status(cert) == expected_response + assert _can_generate_allowlist_certificate_for_status(u, key) == expected_response def test_generation_status_for_none(self): """ Test handling of certificate statuses for a non-existent cert """ - assert _can_generate_allowlist_certificate_for_status(None) is True + assert _can_generate_allowlist_certificate_for_status(None, None) @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=False) def test_handle_invalid(self):