fix: Update logging (#27065)

MICROBA-923

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

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
 
 

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.',

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