diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 60157b6bce025557ae97115ab86b335df828abdf..37af43b06282da5ca804a056514b17d6c6ab5f23 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -647,7 +647,7 @@ def remove_allowlist_entry(user, course_key): certificate = get_certificate_for_user(user.username, course_key, False) if certificate: log.info(f"Invalidating certificate for student {user.id} in course {course_key} before allowlist removal.") - certificate.invalidate() + certificate.invalidate(source='allowlist_removal') log.info(f"Removing student {user.id} from the allowlist in course {course_key}.") allowlist_entry.delete() diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 978762d946ae8afe4846f262516cc201670696b6..573bd6f5789012a5dd75f199c58229dc006e5a1c 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -258,7 +258,7 @@ def _set_v2_cert_status(user, course_key): if cert is None: cert = GeneratedCertificate.objects.create(user=user, course_id=course_key) if cert.status != CertificateStatuses.notpassing: - cert.mark_notpassing(course_grade.percent) + cert.mark_notpassing(course_grade.percent, source='certificate_generation') return CertificateStatuses.notpassing return None @@ -275,14 +275,14 @@ def _get_cert_status_common(user, course_key, cert): if cert is None: cert = GeneratedCertificate.objects.create(user=user, course_id=course_key) if cert.status != CertificateStatuses.unavailable: - cert.invalidate() + cert.invalidate(source='certificate_generation') return CertificateStatuses.unavailable if not IDVerificationService.user_is_verified(user): if cert is None: cert = GeneratedCertificate.objects.create(user=user, course_id=course_key) if cert.status != CertificateStatuses.unverified: - cert.mark_unverified() + cert.mark_unverified(source='certificate_generation') return CertificateStatuses.unverified return None diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 01e8decbcb43fd81e6a919a7ca293722e67b1291..609574057fdb78079d7499e7e7db221aee265fea 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -358,68 +358,66 @@ class GeneratedCertificate(models.Model): user=self.user ) - def invalidate(self): + def invalidate(self, source=None): """ - Invalidate Generated Certificate by marking it 'unavailable'. This will prevent the learner from being able to - access their certificate in the associated Course. In addition, we remove any errors and grade information - associated with the certificate record. + Invalidate Generated Certificate by marking it 'unavailable'. For additional information see the + `_revoke_certificate()` function. - We remove the `download_uuid` and the `download_url` as well, but this is only important to PDF certificates. - - Invalidating a certificate fires the `COURSE_CERT_REVOKED` signal. This kicks off a task to determine if there - are any program certificates that need to be revoked from the learner. + Args: + source (String) - source requesting invalidation of the certificate for tracking purposes """ log.info(f'Marking certificate as unavailable for {self.user.id} : {self.course_id}') + self._revoke_certificate(CertificateStatuses.unavailable, source=source) - self.error_reason = '' - self.download_uuid = '' - self.download_url = '' - self.grade = '' - self.status = CertificateStatuses.unavailable - self.save() - - COURSE_CERT_REVOKED.send_robust( - sender=self.__class__, - user=self.user, - course_key=self.course_id, - mode=self.mode, - status=self.status, - ) - - def mark_notpassing(self, grade): + def mark_notpassing(self, grade, source=None): """ - Invalidates a Generated Certificate by marking it as 'notpassing'. For additional information, please see the - comments of the `invalidate` function above as they also apply here. + Invalidates a Generated Certificate by marking it as 'notpassing'. For additional information see the + `_revoke_certificate()` function. + + Args: + grade (float) - snapshot of the learner's current grade as a decimal + source (String) - source requesting invalidation of the certificate for tracking purposes """ log.info(f'Marking certificate as notpassing for {self.user.id} : {self.course_id}') + self._revoke_certificate(CertificateStatuses.notpassing, grade=grade, source=source) - self.error_reason = '' - self.download_uuid = '' - self.download_url = '' - self.grade = grade - self.status = CertificateStatuses.notpassing - self.save() + def mark_unverified(self, source=None): + """ + Invalidates a Generated Certificate by marking it as 'unverified'. For additional information see the + `_revoke_certificate()` function. - COURSE_CERT_REVOKED.send_robust( - sender=self.__class__, - user=self.user, - course_key=self.course_id, - mode=self.mode, - status=self.status, - ) + Args: + source (String) - source requesting invalidation of the certificate for tracking purposes + """ + log.info(f'Marking certificate as unverified for {self.user.id} : {self.course_id}') + self._revoke_certificate(CertificateStatuses.unverified, source=source) - def mark_unverified(self): + def _revoke_certificate(self, status, grade=None, source=None): """ - Invalidates a Generated Certificate by marking it as 'unverified'. For additional information, please see the - comments of the `invalidate` function above as they also apply here. + Revokes a course certificate from a learner, updating the certificate's status as specified by the value of the + `status` argument. This will prevent the learner from being able to access their certiticate in the associated + course run. + + We remove the `download_uuid` and the `download_url` as well, but this is only important to PDF certificates. + + Invalidating a certificate fires the `COURSE_CERT_REVOKED` signal. This kicks off a task to determine if there + are any program certificates that also need to be revoked from the learner. + + If the certificate had a status of `downloadable` before being revoked then we will also emit an + `edx.certificate.revoked` event for tracking purposes. + + Args: + status (CertificateStatus) - certificate status to set for the `GeneratedCertificate` record + grade (float) - snapshot of the learner's current grade as a decimal + source (String) - source requesting invalidation of the certificate for tracking purposes """ - log.info(f'Marking certificate as unverified for {self.user.id} : {self.course_id}') + previous_certificate_status = self.status self.error_reason = '' self.download_uuid = '' self.download_url = '' - self.grade = '' - self.status = CertificateStatuses.unverified + self.grade = grade or '' + self.status = status self.save() COURSE_CERT_REVOKED.send_robust( @@ -430,6 +428,19 @@ class GeneratedCertificate(models.Model): status=self.status, ) + if previous_certificate_status == CertificateStatuses.downloadable: + # imported here to avoid a circular import issue + from lms.djangoapps.certificates.utils import emit_certificate_event + + event_data = { + 'user_id': self.user.id, + 'course_id': str(self.course_id), + 'certificate_id': self.verify_uuid, + 'enrollment_mode': self.mode, + 'source': source or '', + } + emit_certificate_event('revoked', self.user, str(self.course_id), event_data=event_data) + def is_valid(self): """ Return True if certificate is valid else return False. diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 4941a7b4146613dc890a6c0ac123cbd7548fd400..de1aa3cc3c0b1b971d88389799f975ebc9a65c61 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -140,7 +140,7 @@ class XQueueCertInterface: ) return None - certificate.invalidate() + certificate.invalidate(source='certificate_regeneration') LOGGER.info( f"The certificate status for student {student.id} in course '{course_id} has been changed to " diff --git a/lms/djangoapps/certificates/services.py b/lms/djangoapps/certificates/services.py index 77823e0ff3adb41776909ebb8a5e8d3e049b7dde..29ee5a05d394ca4e5d844e82ddfe339639093600 100644 --- a/lms/djangoapps/certificates/services.py +++ b/lms/djangoapps/certificates/services.py @@ -35,7 +35,7 @@ class CertificateService: user=user_id, course_id=course_key ) - generated_certificate.invalidate() + generated_certificate.invalidate(source='certificate_service') except ObjectDoesNotExist: log.warning( 'Invalidation failed because a certificate for user %d in course %s does not exist.', diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index e561d6857101e10f57899877e150fa3b391232c9..d0237eb67f20e4561e3b77d8cda740b899be94a6 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -114,7 +114,7 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli cert = GeneratedCertificate.certificate_for_student(user, course_id) if cert is not None: if CertificateStatuses.is_passing_status(cert.status): - cert.mark_notpassing(grade.percent) + cert.mark_notpassing(grade.percent, source='notpassing_signal') log.info('Certificate marked not passing for {user} : {course} via failing grade: {grade}'.format( user=user.id, course=course_id, diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 0eb075cd33a6b5e937e0c9a1c152e7a809bc1149..951206992986c0a17dc54f768b4a083deea950f1 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -67,6 +67,7 @@ from lms.djangoapps.certificates.tests.factories import ( CertificateInvalidationFactory ) from lms.djangoapps.grades.tests.utils import mock_passing_grade +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() @@ -253,6 +254,9 @@ class CertificateIsInvalid(WebCertificateTestMixin, ModuleStoreTestCase): number='verified', display_name='Verified Course' ) + self.course_overview = CourseOverviewFactory.create( + id=self.course.id + ) self.global_staff = GlobalStaffFactory() self.request_factory = RequestFactory() diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index 3a37a693a226181389e7338a257b5780448391a9..6c90f93927a4a1aa822088d1ef25188e1741ad1f 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -301,6 +301,9 @@ class CertificateInvalidationTest(SharedModuleStoreTestCase): def setUp(self): super().setUp() self.course = CourseFactory() + self.course_overview = CourseOverviewFactory.create( + id=self.course.id + ) self.user = UserFactory() self.course_id = self.course.id # pylint: disable=no-member self.certificate = GeneratedCertificateFactory.create( @@ -358,7 +361,18 @@ class GeneratedCertificateTest(SharedModuleStoreTestCase): self.course = CourseOverviewFactory() self.course_key = self.course.id - def test_invalidate(self): + def _assert_event_data(self, mocked_function_call, expected_event_data): + """Utility function that verifies the mocked function was called with the expected arguments.""" + + mocked_function_call.assert_called_with( + 'revoked', + self.user, + str(self.course_key), + event_data=expected_event_data + ) + + @patch('lms.djangoapps.certificates.utils.emit_certificate_event') + def test_invalidate(self, mock_emit_certificate_event): """ Test the invalidate method """ @@ -367,12 +381,24 @@ class GeneratedCertificateTest(SharedModuleStoreTestCase): user=self.user, course_id=self.course_key ) - cert.invalidate() + source = 'invalidated_test' + cert.invalidate(source=source) cert = GeneratedCertificate.objects.get(user=self.user, course_id=self.course_key) assert cert.status == CertificateStatuses.unavailable - def test_notpassing(self): + expected_event_data = { + 'user_id': self.user.id, + 'course_id': str(self.course_key), + 'certificate_id': cert.verify_uuid, + 'enrollment_mode': cert.mode, + 'source': source, + } + + self._assert_event_data(mock_emit_certificate_event, expected_event_data) + + @patch('lms.djangoapps.certificates.utils.emit_certificate_event') + def test_notpassing(self, mock_emit_certificate_event): """ Test the notpassing method """ @@ -382,13 +408,25 @@ class GeneratedCertificateTest(SharedModuleStoreTestCase): course_id=self.course_key ) grade = '.3' - cert.mark_notpassing(grade) + source = "notpassing_test" + cert.mark_notpassing(grade, source=source) cert = GeneratedCertificate.objects.get(user=self.user, course_id=self.course_key) assert cert.status == CertificateStatuses.notpassing assert cert.grade == grade - def test_unverified(self): + expected_event_data = { + 'user_id': self.user.id, + 'course_id': str(self.course_key), + 'certificate_id': cert.verify_uuid, + 'enrollment_mode': cert.mode, + 'source': source, + } + + self._assert_event_data(mock_emit_certificate_event, expected_event_data) + + @patch('lms.djangoapps.certificates.utils.emit_certificate_event') + def test_unverified(self, mock_emit_certificate_event): """ Test the unverified method """ @@ -397,7 +435,18 @@ class GeneratedCertificateTest(SharedModuleStoreTestCase): user=self.user, course_id=self.course_key ) - cert.mark_unverified() + source = "unverified_test" + cert.mark_unverified(source=source) cert = GeneratedCertificate.objects.get(user=self.user, course_id=self.course_key) assert cert.status == CertificateStatuses.unverified + + expected_event_data = { + 'user_id': self.user.id, + 'course_id': str(self.course_key), + 'certificate_id': cert.verify_uuid, + 'enrollment_mode': cert.mode, + 'source': source, + } + + self._assert_event_data(mock_emit_certificate_event, expected_event_data) diff --git a/lms/djangoapps/certificates/tests/test_services.py b/lms/djangoapps/certificates/tests/test_services.py index 57e1f57742d58d0748fd8d4a2ad40630254e641d..1881f558e8845e00186f870bae657632ef01d72d 100644 --- a/lms/djangoapps/certificates/tests/test_services.py +++ b/lms/djangoapps/certificates/tests/test_services.py @@ -7,6 +7,7 @@ from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.services import CertificateService from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory, GeneratedCertificateFactory +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -20,6 +21,9 @@ class CertificateServiceTests(ModuleStoreTestCase): super().setUp() self.service = CertificateService() self.course = CourseFactory() + self.course_overview = CourseOverviewFactory.create( + id=self.course.id + ) self.user = UserFactory() self.user_id = self.user.id self.course_id = self.course.id # pylint: disable=no-member diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index b0c5f687acc815868caab98471c568f09e2a6f53..0cbfa85be7d8f4ae3fa1113cb1beae0c31dae132 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3332,7 +3332,7 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat ) # Invalidate the certificate - generated_certificate.invalidate() + generated_certificate.invalidate(source='certificate_invalidation_list') return { 'id': certificate_invalidation.id, diff --git a/lms/djangoapps/instructor_task/tasks_helper/certs.py b/lms/djangoapps/instructor_task/tasks_helper/certs.py index d14997b5fc969f2e65bab1fefc8357d1309185f3..1bd686e67cd196c69aa61ff11cb5ece32dcd2da0 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/certs.py +++ b/lms/djangoapps/instructor_task/tasks_helper/certs.py @@ -157,4 +157,4 @@ def _invalidate_generated_certificates(course_id, enrolled_students, certificate f'for course {course_id}') else: log.info(f'About to invalidate certificate for user {c.user.id} in course {course_id}') - c.invalidate() + c.invalidate(source='bulk_certificate_regeneration')