MICROBA-918 Check the allowlist when regenerating certificates, and stop incidentally modifying the certificate invalidation list (#26439)

lms/djangoapps/certificates/generation_handler.py

@@ -259,6 +259,11 @@ def regenerate_user_certificates(student, course_key, course=None,
         template_file - The template file used to render this certificate
         insecure - (Boolean)
     """
+    if is_using_certificate_allowlist_and_is_on_allowlist(student, course_key):
+        log.info(f'{course_key} is using allowlist certificates, and the user {student.id} is on its allowlist. '
+                 f'Attempt will be made to regenerate an allowlist certificate.')
+        return generate_allowlist_certificate_task(student, course_key)
+
     xqueue = XQueueCertInterface()
     if insecure:
         xqueue.use_https = False
@@ -272,7 +277,7 @@ def regenerate_user_certificates(student, course_key, course=None,
         student.username, str(course_key), generate_pdf
     )
 
-    return xqueue.regen_cert(
+    xqueue.regen_cert(
         student,
         course_key,
         course=course,
@@ -280,3 +285,4 @@ def regenerate_user_certificates(student, course_key, course=None,
         template_file=template_file,
         generate_pdf=generate_pdf
     )
+    return True

lms/djangoapps/certificates/management/commands/regenerate_user.py

@@ -101,7 +101,7 @@ class Command(BaseCommand):
                     LOGGER.info(u"Cleared badge for student %s.", student.id)
 
             # Add the certificate request to the queue
-            ret = regenerate_user_certificates(
+            regenerate_user_certificates(
                 student, course_id, course=course,
                 forced_grade=options['grade_value'],
                 template_file=options['template_file'],
@@ -111,12 +111,10 @@ class Command(BaseCommand):
             LOGGER.info(
                 (
                     u"Added a certificate regeneration task to the XQueue "
-                    u"for student %s in course '%s'. "
-                    u"The new certificate status is '%s'."
+                    u"for student %s in course '%s'."
                 ),
                 student.id,
-                text_type(course_id),
-                ret
+                text_type(course_id)
             )
 
         else:

lms/djangoapps/certificates/signals.py

@@ -156,8 +156,7 @@ def fire_ungenerated_certificate_task(user, course_key, expected_verification_st
     if is_using_certificate_allowlist_and_is_on_allowlist(user, course_key):
         log.info('{course} is using allowlist certificates, and the user {user} is on its allowlist. Attempt will be '
                  'made to generate an allowlist certificate.'.format(course=course_key, user=user.id))
-        generate_allowlist_certificate_task(user, course_key)
-        return True
+        return generate_allowlist_certificate_task(user, course_key)
 
     log.info('{course} is not using allowlist certificates (or user {user} is not on its allowlist). The normal '
              'generation logic will be followed.'.format(course=course_key, user=user.id))

lms/djangoapps/certificates/tests/test_support_views.py

@@ -372,8 +372,7 @@ class CertificateRegenerateTests(CertificateSupportTestCase):
         self.assertEqual(num_certs, 1)
 
     def test_regenerate_cert_with_invalidated_record(self):
-        """ If the certificate is marked as invalid, regenerate the certificate
-        and verify the invalidate entry is deactivated. """
+        """ If the certificate is marked as invalid, regenerate the certificate. """
 
         # mark certificate as invalid
         self._invalidate_certificate(self.cert)
@@ -389,7 +388,7 @@ class CertificateRegenerateTests(CertificateSupportTestCase):
             username=self.STUDENT_USERNAME
         )
         self.assertEqual(response.status_code, 200)
-        self.assertInvalidatedCertDoesNotExist()
+        self.assertInvalidatedCertExists()
 
         # Check that the user's certificate was updated
         # Since the student hasn't actually passed the course,

lms/djangoapps/certificates/views/support.py

@@ -201,7 +201,7 @@ def regenerate_certificate_for_user(request):
 
     # Attempt to regenerate certificates
     try:
-        certificate = regenerate_user_certificates(params["user"], params["course_key"], course=course)
+        regenerate_user_certificates(params["user"], params["course_key"], course=course)
     except:  # pylint: disable=bare-except
         # We are pessimistic about the kinds of errors that might get thrown by the
         # certificates API.  This may be overkill, but we're logging everything so we can
@@ -213,9 +213,6 @@ def regenerate_certificate_for_user(request):
         )
         return HttpResponseServerError(_("An unexpected error occurred while regenerating certificates."))
 
-    # Deactivate certificate invalidation by setting active to False.
-    _deactivate_invalidation(certificate)
-
     log.info(
         u"Started regenerating certificates for user %s in course %s from the support page.",
         params["user"].id, params["course_key"]
@@ -275,25 +272,3 @@ def generate_certificate_for_user(request):
             specific_student_id=params["user"].id
         )
         return HttpResponse(200)
-
-
-def _deactivate_invalidation(certificate):
-    """
-    Deactivate certificate invalidation by setting active to False.
-
-    Arguments:
-        certificate : The student certificate object
-
-    Return:
-        None
-    """
-    try:
-        # Fetch CertificateInvalidation object
-        certificate_invalidation = CertificateInvalidation.objects.get(
-            generated_certificate=certificate,
-            active=True
-        )
-        # Deactivate certificate invalidation if it was fetched successfully.
-        certificate_invalidation.deactivate()
-    except CertificateInvalidation.DoesNotExist:
-        pass