diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 2dcd25e542bc74a5bd11adc1be34d129551a23d7..439cf32c9a415e033e90ba0604d573ca09babe72 100644 --- a/lms/djangoapps/certificates/api.py +++ b/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. diff --git a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py index 8b6c83ae7f85c3176ad8abd80988f1c2823db77a..ba88a308c1fc81a35e5eef6f520646599d81eae5 100644 --- a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py +++ b/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( diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 0b451a6d24568b6e027c8362a66e454ce330162c..967ef2e0a29d626b4a483c64a792333e5b334044 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/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, diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 6e50014a2bf86c469326f384f41199d5bdb1be16..5cfa6f68d82e6a03f8dbb78c56d9410a555b80ba 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/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) diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 21e9d33eb351c2f54a3f902ee9279709e2da4c85..c6145131c36464dc4b3e7fbbb876644a0603f17c 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/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)