From a6d9275cde47bbcbba0a96ac3654e689a1635bb6 Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Tue, 27 Apr 2021 10:24:30 -0400 Subject: [PATCH] fix: Ignore passing signal if the cert status is already downloadable (#27403) MICROBA-1106 --- lms/djangoapps/certificates/models.py | 3 +- lms/djangoapps/certificates/signals.py | 21 +++++--- .../certificates/tests/test_signals.py | 54 ++++++++++++++++++- 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 827ddaf61a..4d9674f711 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -55,8 +55,7 @@ class CertificateStatuses: invalidated - Certificate is not valid. notpassing - The user has not achieved a passing grade. requesting - A request has been made to generate the PDF certificate. - restricted - The user is on the restricted list. This status was previously set if allow_certificate was - set to False in the userprofile table. + restricted - The user is restricted from receiving a certificate. unavailable - Certificate has been invalidated. unverified - The user does not have an approved, unexpired identity verification. diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 5ff52a8233..5db6f14c15 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -75,13 +75,19 @@ def _listen_for_certificate_whitelist_append(sender, instance, **kwargs): # pyl @receiver(COURSE_GRADE_NOW_PASSED, dispatch_uid="new_passing_learner") def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disable=unused-argument """ - Listen for a learner passing a course, send cert generation task, - downstream signal from COURSE_GRADE_CHANGED + Listen for a signal indicating that the user has passed a course run. + + If needed, generate a certificate task. """ if not auto_certificate_generation_enabled(): return if can_generate_certificate_task(user, course_id): + cert = GeneratedCertificate.certificate_for_student(user, course_id) + if cert is not None and CertificateStatuses.is_passing_status(cert.status): + log.info(f'{course_id} is using V2 certificates, and the cert status is already passing for user ' + f'{user.id}. Passing grade signal will be ignored.') + return log.info(f'{course_id} is using V2 certificates. Attempt will be made to generate a V2 certificate for ' f'{user.id} as a passing grade was received.') return generate_certificate_task(user, course_id) @@ -96,9 +102,9 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disa @receiver(COURSE_GRADE_NOW_FAILED, dispatch_uid="new_failing_learner") def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pylint: disable=unused-argument """ - Listen for a learner failing a course, mark the cert as notpassing - if it is currently passing, - downstream signal from COURSE_GRADE_CHANGED + Listen for a signal indicating that the user has failed a course run. + + If needed, mark the certificate as notpassing. """ if is_using_certificate_allowlist_and_is_on_allowlist(user, course_id): log.info('{course_id} is using allowlist certificates, and the user {user_id} is on its allowlist. The ' @@ -119,8 +125,9 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli @receiver(LEARNER_NOW_VERIFIED, dispatch_uid="learner_track_changed") 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 + Listen for a signal indicating that the user's id verification status has changed. + + If needed, generate a certificate task. """ if not auto_certificate_generation_enabled(): return diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index e278ae1dc6..3bf29aa049 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -12,7 +12,10 @@ from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.api import cert_generation_enabled -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_ALLOWLIST +from lms.djangoapps.certificates.generation_handler import ( + CERTIFICATES_USE_ALLOWLIST, + CERTIFICATES_USE_UPDATED +) from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, CertificateStatuses, @@ -217,6 +220,7 @@ class PassingGradeCertsTest(ModuleStoreTestCase): self.course = CourseFactory.create( self_paced=True, ) + self.course_key = self.course.id self.user = UserFactory.create() self.enrollment = CourseEnrollmentFactory( user=self.user, @@ -334,6 +338,54 @@ class PassingGradeCertsTest(ModuleStoreTestCase): CourseGradeFactory().update(u, c) mock_cert_task.assert_called_with(u, course_key) + @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) + def test_cert_already_generated_downloadable(self): + with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): + GeneratedCertificateFactory( + user=self.user, + course_id=self.course.id, + status=CertificateStatuses.downloadable + ) + + with mock.patch( + 'lms.djangoapps.certificates.signals.generate_certificate_task', + return_value=None + ) as mock_cert_task: + grade_factory = CourseGradeFactory() + with mock_passing_grade(): + grade_factory.update(self.user, self.course) + mock_cert_task.assert_not_called() + + @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) + def test_cert_already_generated_unverified(self): + with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): + GeneratedCertificateFactory( + user=self.user, + course_id=self.course.id, + status=CertificateStatuses.unverified + ) + + with mock.patch( + 'lms.djangoapps.certificates.signals.generate_certificate_task', + return_value=None + ) as mock_cert_task: + grade_factory = CourseGradeFactory() + with mock_passing_grade(): + grade_factory.update(self.user, self.course) + mock_cert_task.assert_called_with(self.user, self.course_key) + + @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) + def test_without_cert(self): + with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): + with mock.patch( + 'lms.djangoapps.certificates.signals.generate_certificate_task', + return_value=None + ) as mock_cert_task: + grade_factory = CourseGradeFactory() + with mock_passing_grade(): + grade_factory.update(self.user, self.course) + mock_cert_task.assert_called_with(self.user, self.course_key) + @ddt.ddt class FailingGradeCertsTest(ModuleStoreTestCase):