MICROBA-918 Update allowlist check to handle more signals (#26606)

lms/djangoapps/certificates/api.py

@@ -19,6 +19,7 @@ from organizations.api import get_course_organization_id
 from lms.djangoapps.branding import api as branding_api
 from lms.djangoapps.certificates.generation_handler import (
     is_using_certificate_allowlist_and_is_on_allowlist as _is_using_certificate_allowlist_and_is_on_allowlist,
+    generate_allowlist_certificate_task as _generate_allowlist_certificate_task,
     generate_user_certificates as _generate_user_certificates,
     regenerate_user_certificates as _regenerate_user_certificates
 )
@@ -191,6 +192,10 @@ def regenerate_user_certificates(student, course_key, course=None,
     return _regenerate_user_certificates(student, course_key, course, forced_grade, template_file, insecure)
 
 
+def generate_allowlist_certificate_task(user, course_key):
+    return _generate_allowlist_certificate_task(user, course_key)
+
+
 def certificate_downloadable_status(student, course_key):
     """
     Check the student existing certificates against a given course.

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

@@ -131,24 +131,15 @@ class Command(BaseCommand):
 
                     if not options['noop']:
                         # Add the certificate request to the queue
-                        ret = generate_user_certificates(
+                        generate_user_certificates(
                             student,
                             course_key,
                             course=course,
                             insecure=options['insecure']
                         )
 
-                        if ret == 'generating':
-                            LOGGER.info(
-                                (
-                                    u"Added a certificate generation task to the XQueue "
-                                    u"for student %s in course '%s'. "
-                                    u"The new certificate status is '%s'."
-                                ),
-                                student.id,
-                                text_type(course_key),
-                                ret
-                            )
+                        LOGGER.info(f"Added a certificate generation task to the XQueue for student {student.id} in "
+                                    f"course {course_key}.")
 
                     else:
                         LOGGER.info(

lms/djangoapps/certificates/signals.py

@@ -54,10 +54,15 @@ def _listen_for_certificate_whitelist_append(sender, instance, **kwargs):  # pyl
     """
     Listen for a user being added to or modified on the whitelist (allowlist)
     """
+    if is_using_certificate_allowlist_and_is_on_allowlist(instance.user, instance.course_id):
+        log.info(f'{instance.course_id} is using allowlist certificates, and the user {instance.user.id} is now on '
+                 f'its allowlist. Attempt will be made to generate an allowlist certificate.')
+        return generate_allowlist_certificate_task(instance.user, instance.course_id)
+
     if not auto_certificate_generation_enabled():
         return
 
-    if fire_ungenerated_certificate_task(instance.user, instance.course_id):
+    if _fire_ungenerated_certificate_task(instance.user, instance.course_id):
         log.info(u'Certificate generation task initiated for {user} : {course} via whitelist'.format(
             user=instance.user.id,
             course=instance.course_id
@@ -70,10 +75,15 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs):  # pylint: disa
     Listen for a learner passing a course, send cert generation task,
     downstream signal from COURSE_GRADE_CHANGED
     """
+    if is_using_certificate_allowlist_and_is_on_allowlist(user, course_id):
+        log.info(f'{course_id} is using allowlist certificates, and the user {user.id} is on its allowlist. Attempt '
+                 f'will be made to generate an allowlist certificate as a passing grade was received.')
+        return generate_allowlist_certificate_task(user, course_id)
+
     if not auto_certificate_generation_enabled():
         return
 
-    if fire_ungenerated_certificate_task(user, course_id):
+    if _fire_ungenerated_certificate_task(user, course_id):
         log.info(u'Certificate generation task initiated for {user} : {course} via passing grade'.format(
             user=user.id,
             course=course_id
@@ -107,7 +117,7 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs):  # pyli
 def _listen_for_id_verification_status_changed(sender, user, **kwargs):  # pylint: disable=unused-argument
     """
     Catches a track change signal, determines user status,
-    calls fire_ungenerated_certificate_task for passing grades
+    calls _fire_ungenerated_certificate_task for passing grades
     """
     if not auto_certificate_generation_enabled():
         return
@@ -118,8 +128,13 @@ def _listen_for_id_verification_status_changed(sender, user, **kwargs):  # pylin
     expected_verification_status = IDVerificationService.user_status(user)
     expected_verification_status = expected_verification_status['status']
     for enrollment in user_enrollments:
-        if grade_factory.read(user=user, course=enrollment.course_overview).passed:
-            if fire_ungenerated_certificate_task(user, enrollment.course_id, expected_verification_status):
+        if is_using_certificate_allowlist_and_is_on_allowlist(user, enrollment.course_id):
+            log.info(f'{enrollment.course_id} is using allowlist certificates, and the user {user.id} is on its '
+                     f'allowlist. Attempt will be made to generate an allowlist certificate. Id verification status '
+                     f'is {expected_verification_status}')
+            generate_allowlist_certificate_task(user, enrollment.course_id)
+        elif grade_factory.read(user=user, course=enrollment.course_overview).passed:
+            if _fire_ungenerated_certificate_task(user, enrollment.course_id, expected_verification_status):
                 message = (
                     u'Certificate generation task initiated for {user} : {course} via track change ' +
                     u'with verification status of {status}'
@@ -131,7 +146,7 @@ def _listen_for_id_verification_status_changed(sender, user, **kwargs):  # pylin
                 ))
 
 
-def fire_ungenerated_certificate_task(user, course_key, expected_verification_status=None):
+def _fire_ungenerated_certificate_task(user, course_key, expected_verification_status=None):
     """
     Helper function to fire certificate generation task.
     Auto-generation of certificates is available for following course modes:
@@ -156,14 +171,6 @@ def fire_ungenerated_certificate_task(user, course_key, expected_verification_st
     message = u'Entered into Ungenerated Certificate task for {user} : {course}'
     log.info(message.format(user=user.id, course=course_key))
 
-    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))
-        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))
-
     allowed_enrollment_modes_list = [
         CourseMode.VERIFIED,
         CourseMode.CREDIT_MODE,

lms/djangoapps/certificates/tests/test_signals.py

@@ -21,7 +21,7 @@ from lms.djangoapps.certificates.models import (
     CertificateStatuses,
     GeneratedCertificate
 )
-from lms.djangoapps.certificates.signals import fire_ungenerated_certificate_task
+from lms.djangoapps.certificates.signals import _fire_ungenerated_certificate_task
 from lms.djangoapps.certificates.tasks import CERTIFICATE_DELAY_SECONDS
 from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory
 from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
@@ -152,15 +152,14 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
                 return_value=None
             ) as mock_generate_allowlist_task:
-                CertificateWhitelistFactory(
-                    user=self.user,
-                    course_id=self.ip_course.id,
-                    whitelist=True
-                )
-
-                fire_ungenerated_certificate_task(self.user, self.ip_course.id)
-                mock_generate_certificate_apply_async.assert_not_called()
-                mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id)
+                with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
+                    CertificateWhitelistFactory(
+                        user=self.user,
+                        course_id=self.ip_course.id,
+                        whitelist=True
+                    )
+                    mock_generate_certificate_apply_async.assert_not_called()
+                    mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id)
 
     def test_fire_task_allowlist_disabled(self):
         """
@@ -174,21 +173,20 @@ class WhitelistGeneratedCertificatesTest(ModuleStoreTestCase):
                 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
                 return_value=None
             ) as mock_generate_allowlist_task:
-                CertificateWhitelistFactory(
-                    user=self.user,
-                    course_id=self.ip_course.id,
-                    whitelist=True
-                )
-
-                fire_ungenerated_certificate_task(self.user, self.ip_course.id)
-                mock_generate_certificate_apply_async.assert_called_with(
-                    countdown=CERTIFICATE_DELAY_SECONDS,
-                    kwargs={
-                        'student': six.text_type(self.user.id),
-                        'course_key': six.text_type(self.ip_course.id),
-                    }
-                )
-                mock_generate_allowlist_task.assert_not_called()
+                with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
+                    CertificateWhitelistFactory(
+                        user=self.user,
+                        course_id=self.ip_course.id,
+                        whitelist=True
+                    )
+                    mock_generate_certificate_apply_async.assert_called_with(
+                        countdown=CERTIFICATE_DELAY_SECONDS,
+                        kwargs={
+                            'student': six.text_type(self.user.id),
+                            'course_key': six.text_type(self.ip_course.id),
+                        }
+                    )
+                    mock_generate_allowlist_task.assert_not_called()
 
 
 class PassingGradeCertsTest(ModuleStoreTestCase):
@@ -280,6 +278,44 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
                 grade_factory.update(self.user, self.course)
                 mock_generate_certificate_apply_async.assert_not_called()
 
+    @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True)
+    def test_passing_grade_allowlist(self):
+        with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
+            # User who is not on the allowlist
+            GeneratedCertificateFactory(
+                user=self.user,
+                course_id=self.course.id,
+                status=CertificateStatuses.error
+            )
+            with mock_passing_grade():
+                with mock.patch(
+                    'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
+                    return_value=None
+                ) as mock_allowlist_task:
+                    CourseGradeFactory().update(self.user, self.course)
+                    mock_allowlist_task.assert_not_called()
+
+            # User who is on the allowlist
+            u = UserFactory.create()
+            c = CourseFactory()
+            course_key = c.id  # pylint: disable=no-member
+            CertificateWhitelistFactory(
+                user=u,
+                course_id=course_key
+            )
+            GeneratedCertificateFactory(
+                user=u,
+                course_id=course_key,
+                status=CertificateStatuses.error
+            )
+            with mock_passing_grade():
+                with mock.patch(
+                    'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
+                    return_value=None
+                ) as mock_allowlist_task:
+                    CourseGradeFactory().update(u, c)
+                    mock_allowlist_task.assert_called_with(u, course_key)
+
 
 @ddt.ddt
 class FailingGradeCertsTest(ModuleStoreTestCase):
@@ -437,6 +473,47 @@ class LearnerTrackChangeCertsTest(ModuleStoreTestCase):
                     }
                 )
 
+    @override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True)
+    def test_id_verification_allowlist(self):
+        # User is not on the allowlist
+        with mock.patch(
+            'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
+            return_value=None
+        ) as mock_allowlist_task:
+            with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
+                attempt = SoftwareSecurePhotoVerification.objects.create(
+                    user=self.user_two,
+                    status='submitted'
+                )
+                attempt.approve()
+                mock_allowlist_task.assert_not_called()
+
+        # User is on the allowlist
+        with mock.patch(
+            'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task',
+            return_value=None
+        ) as mock_allowlist_task:
+            with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
+                u = UserFactory.create()
+                c = CourseFactory()
+                course_key = c.id  # pylint: disable=no-member
+                CourseEnrollmentFactory(
+                    user=u,
+                    course_id=course_key,
+                    is_active=True,
+                    mode='verified'
+                )
+                CertificateWhitelistFactory(
+                    user=u,
+                    course_id=course_key
+                )
+                attempt = SoftwareSecurePhotoVerification.objects.create(
+                    user=u,
+                    status='submitted'
+                )
+                attempt.approve()
+                mock_allowlist_task.assert_called_with(u, course_key)
+
 
 @ddt.ddt
 class CertificateGenerationTaskTest(ModuleStoreTestCase):
@@ -475,6 +552,6 @@ class CertificateGenerationTaskTest(ModuleStoreTestCase):
             return_value=None
         ) as mock_generate_certificate_apply_async:
             with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True):
-                fire_ungenerated_certificate_task(self.user, self.course.id)
+                _fire_ungenerated_certificate_task(self.user, self.course.id)
                 task_created = mock_generate_certificate_apply_async.called
                 self.assertEqual(task_created, should_create)

lms/djangoapps/grades/tests/test_course_grade_factory.py

@@ -100,7 +100,7 @@ class TestCourseGradeFactory(GradeTestBase):
         with self.assertNumQueries(3), mock_get_score(1, 2):
             _assert_read(expected_pass=False, expected_percent=0)  # start off with grade of 0
 
-        num_queries = 42
+        num_queries = 44
         with self.assertNumQueries(num_queries), mock_get_score(1, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
@@ -121,7 +121,7 @@ class TestCourseGradeFactory(GradeTestBase):
         with self.assertNumQueries(3):
             _assert_read(expected_pass=True, expected_percent=1.0)  # updated to grade of 1.0
 
-        num_queries = 30
+        num_queries = 28
         with self.assertNumQueries(num_queries), mock_get_score(0, 0):  # the subsection now is worth zero
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)