From 6e1f700cdc7a49bac7b270bb9fed2563d9f56a69 Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Tue, 27 Apr 2021 09:40:06 -0400 Subject: [PATCH] feat: Generate more certificate status for V2 course certs (#27326) MICROBA-1106 --- lms/djangoapps/certificates/generation.py | 2 - .../certificates/generation_handler.py | 162 +++++++++++++++++- .../commands/tests/test_cert_generation.py | 1 - lms/djangoapps/certificates/models.py | 30 +++- .../tests/test_create_fake_cert.py | 0 .../tests/test_generation_handler.py | 112 +++++++++++- .../certificates/tests/test_models.py | 57 ++++++ 7 files changed, 348 insertions(+), 16 deletions(-) delete mode 100644 lms/djangoapps/certificates/tests/test_create_fake_cert.py diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index e24fae59f8..be898d5f7f 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -10,10 +10,8 @@ These methods should be called from tasks. """ import logging -import random # lint-amnesty, pylint: disable=unused-import from uuid import uuid4 -from capa.xqueue_interface import make_hashkey # lint-amnesty, pylint: disable=unused-import from common.djangoapps.student.models import CourseEnrollment, UserProfile from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.queue import XQueueCertInterface diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 2eab86fe58..eaad62b938 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -106,11 +106,14 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None): """ Create a task to generate an allowlist certificate for this user in this course run. """ - if not _can_generate_allowlist_certificate(user, course_key): - log.info(f'Cannot generate an allowlist certificate for {user.id} : {course_key}') - return False + if _can_generate_allowlist_certificate(user, course_key): + return _generate_certificate_task(user, course_key, generation_mode) - return _generate_certificate_task(user, course_key, generation_mode) + status = _set_allowlist_cert_status(user, course_key) + if status is not None: + return True + + return False def generate_regular_certificate_task(user, course_key, generation_mode=None): @@ -118,11 +121,14 @@ def generate_regular_certificate_task(user, course_key, generation_mode=None): Create a task to generate a regular (non-allowlist) certificate for this user in this course run, if the user is eligible and a certificate can be generated. """ - if not _can_generate_v2_certificate(user, course_key): - log.info(f'Cannot generate a v2 course certificate for {user.id} : {course_key}') - return False + if _can_generate_v2_certificate(user, course_key): + return _generate_certificate_task(user, course_key, generation_mode) - return _generate_certificate_task(user, course_key, generation_mode) + status = _set_v2_cert_status(user, course_key) + if status is not None: + return True + + return False def _generate_certificate_task(user, course_key, generation_mode=None): @@ -239,6 +245,122 @@ def _can_generate_certificate_common(user, course_key): return True +def _set_allowlist_cert_status(user, course_key): + """ + Determine the allowlist certificate status for this user, in this course run and update the cert. + + This is used when a downloadable cert cannot be generated, but we want to provide more info about why it cannot + be generated. + """ + if not _can_set_allowlist_cert_status(user, course_key): + return None + + cert = GeneratedCertificate.certificate_for_student(user, course_key) + return _get_cert_status_common(user, course_key, cert) + + +def _set_v2_cert_status(user, course_key): + """ + Determine the V2 certificate status for this user, in this course run. + + This is used when a downloadable cert cannot be generated, but we want to provide more info about why it cannot + be generated. + """ + if not _can_set_v2_cert_status(user, course_key): + return None + + cert = GeneratedCertificate.certificate_for_student(user, course_key) + status = _get_cert_status_common(user, course_key, cert) + if status is not None: + return status + + course = _get_course(course_key) + course_grade = _get_course_grade(user, course) + if not course_grade.passed: + 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) + return CertificateStatuses.notpassing + + return None + + +def _get_cert_status_common(user, course_key, cert): + """ + Determine the certificate status for this user, in this course run. + + This is used when a downloadable cert cannot be generated, but we want to provide more info about why it cannot + be generated. + """ + if CertificateInvalidation.has_certificate_invalidation(user, course_key): + if cert is None: + cert = GeneratedCertificate.objects.create(user=user, course_id=course_key) + if cert.status != CertificateStatuses.unavailable: + cert.invalidate() + 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() + return CertificateStatuses.unverified + + return None + + +def _can_set_allowlist_cert_status(user, course_key): + """ + Determine whether we can set a custom (non-downloadable) cert status for an allowlist certificate + """ + if not is_using_certificate_allowlist(course_key): + return False + + if not is_on_certificate_allowlist(user, course_key): + return False + + course = _get_course(course_key) + return _can_set_cert_status_common(user, course_key, course) + + +def _can_set_v2_cert_status(user, course_key): + """ + Determine whether we can set a custom (non-downloadable) cert status for a V2 certificate + """ + if not is_using_v2_course_certificates(course_key): + return False + + if _is_ccx_course(course_key): + return False + + course = _get_course(course_key) + if _is_beta_tester(user, course): + return False + + return _can_set_cert_status_common(user, course_key, course) + + +def _can_set_cert_status_common(user, course_key, course): + """ + Determine whether we can set a custom (non-downloadable) cert status + """ + if _is_cert_downloadable(user, course_key): + return False + + enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) + if enrollment_mode is None: + return False + + if not modes_api.is_eligible_for_certificate(enrollment_mode): + return False + + if not has_html_certificates_enabled(course): + return False + + return True + + def is_using_certificate_allowlist_and_is_on_allowlist(user, course_key): """ Return True if both: @@ -306,10 +428,32 @@ def _has_passing_grade(user, course): """ Check if the user has a passing grade in this course run """ - course_grade = CourseGradeFactory().read(user, course) + course_grade = _get_course_grade(user, course) return course_grade.passed +def _get_course_grade(user, course): + """ + Get the user's course grade in this course run + """ + return CourseGradeFactory().read(user, course) + + +def _is_cert_downloadable(user, course_key): + """ + Check if cert already exists, has a downloadable status, and has not been invalidated + """ + cert = GeneratedCertificate.certificate_for_student(user, course_key) + if cert is None: + return False + if cert.status != CertificateStatuses.downloadable: + return False + if CertificateInvalidation.has_certificate_invalidation(user, course_key): + return False + + return True + + def _get_course(course_key): """ Get the course from the course key diff --git a/lms/djangoapps/certificates/management/commands/tests/test_cert_generation.py b/lms/djangoapps/certificates/management/commands/tests/test_cert_generation.py index fdd818434c..be65ec3402 100644 --- a/lms/djangoapps/certificates/management/commands/tests/test_cert_generation.py +++ b/lms/djangoapps/certificates/management/commands/tests/test_cert_generation.py @@ -6,7 +6,6 @@ from unittest import mock import pytest from django.core.management import CommandError, call_command -from waffle.testutils import override_switch # lint-amnesty, pylint: disable=unused-import from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.tests.test_generation_handler import ID_VERIFIED_METHOD diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 2e98578cf9..827ddaf61a 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -58,7 +58,13 @@ class CertificateStatuses: restricted - The user is on the restricted list. This status was previously set if allow_certificate was set to False in the userprofile table. unavailable - Certificate has been invalidated. - unverified - The user is in verified track but does not have an approved, unexpired identity verification. + unverified - The user does not have an approved, unexpired identity verification. + + The following statuses are set by V2 of course certificates: + downloadable - See generation.py + notpassing - See GeneratedCertificate.mark_notpassing() + unavailable - See GeneratedCertificate.invalidate() + unverified - See GeneratedCertificate.mark_unverified() """ deleted = 'deleted' deleting = 'deleting' @@ -389,6 +395,28 @@ class GeneratedCertificate(models.Model): status=self.status, ) + def mark_unverified(self): + """ + 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. + """ + log.info(f'Marking certificate as unverified for {self.user.id} : {self.course_id}') + + self.error_reason = '' + self.download_uuid = '' + self.download_url = '' + self.grade = '' + self.status = CertificateStatuses.unverified + 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 is_valid(self): """ Return True if certificate is valid else return False. diff --git a/lms/djangoapps/certificates/tests/test_create_fake_cert.py b/lms/djangoapps/certificates/tests/test_create_fake_cert.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 502badd607..6bf201ac71 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -11,8 +11,6 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, U from lms.djangoapps.certificates.generation_handler import ( CERTIFICATES_USE_ALLOWLIST, CERTIFICATES_USE_UPDATED, - is_using_certificate_allowlist, - is_using_v2_course_certificates, _can_generate_allowlist_certificate, _can_generate_certificate_for_status, _can_generate_v2_certificate, @@ -20,7 +18,11 @@ from lms.djangoapps.certificates.generation_handler import ( generate_allowlist_certificate_task, generate_certificate_task, generate_regular_certificate_task, - is_using_certificate_allowlist_and_is_on_allowlist + is_using_certificate_allowlist, + is_using_certificate_allowlist_and_is_on_allowlist, + is_using_v2_course_certificates, + _set_allowlist_cert_status, + _set_v2_cert_status ) from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.tests.factories import ( @@ -153,6 +155,7 @@ class AllowlistTests(ModuleStoreTestCase): assert not generate_allowlist_certificate_task(self.user, self.course_run_key) assert not can_generate_certificate_task(self.user, self.course_run_key) assert not generate_certificate_task(self.user, self.course_run_key) + assert _set_allowlist_cert_status(self.user, self.course_run_key) is None def test_handle_valid(self): """ @@ -174,6 +177,7 @@ class AllowlistTests(ModuleStoreTestCase): """ with mock.patch(ID_VERIFIED_METHOD, return_value=False): assert not _can_generate_allowlist_certificate(self.user, self.course_run_key) + assert _set_allowlist_cert_status(self.user, self.course_run_key) == CertificateStatuses.unverified def test_can_generate_not_enrolled(self): """ @@ -184,6 +188,7 @@ class AllowlistTests(ModuleStoreTestCase): key = cr.id # pylint: disable=no-member CertificateWhitelistFactory.create(course_id=key, user=u) assert not _can_generate_allowlist_certificate(u, key) + assert _set_allowlist_cert_status(u, key) is None def test_can_generate_audit(self): """ @@ -201,6 +206,7 @@ class AllowlistTests(ModuleStoreTestCase): CertificateWhitelistFactory.create(course_id=key, user=u) assert not _can_generate_allowlist_certificate(u, key) + assert _set_allowlist_cert_status(u, key) is None def test_can_generate_not_whitelisted(self): """ @@ -216,6 +222,7 @@ class AllowlistTests(ModuleStoreTestCase): mode="verified", ) assert not _can_generate_allowlist_certificate(u, key) + assert _set_allowlist_cert_status(u, key) is None def test_can_generate_invalidated(self): """ @@ -244,6 +251,7 @@ class AllowlistTests(ModuleStoreTestCase): ) assert not _can_generate_allowlist_certificate(u, key) + assert _set_allowlist_cert_status(u, key) == CertificateStatuses.unavailable def test_can_generate_web_cert_disabled(self): """ @@ -251,6 +259,29 @@ class AllowlistTests(ModuleStoreTestCase): """ with mock.patch(WEB_CERTS_METHOD, return_value=False): assert not _can_generate_allowlist_certificate(self.user, self.course_run_key) + assert _set_allowlist_cert_status(self.user, self.course_run_key) is None + + def test_cert_status_downloadable(self): + """ + Test cert status when status is already downloadable + """ + u = UserFactory() + cr = CourseFactory() + key = cr.id # pylint: disable=no-member + CourseEnrollmentFactory( + user=u, + course_id=key, + is_active=True, + mode="verified", + ) + GeneratedCertificateFactory( + user=u, + course_id=key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.downloadable + ) + + assert _set_allowlist_cert_status(u, key) is None @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) @@ -362,6 +393,7 @@ class CertificateTests(ModuleStoreTestCase): """ with mock.patch(ID_VERIFIED_METHOD, return_value=False): assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert _set_v2_cert_status(self.user, self.course_run_key) == CertificateStatuses.unverified def test_can_generate_ccx(self): """ @@ -369,6 +401,7 @@ class CertificateTests(ModuleStoreTestCase): """ with mock.patch(CCX_COURSE_METHOD, return_value=True): assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert _set_v2_cert_status(self.user, self.course_run_key) is None def test_can_generate_beta_tester(self): """ @@ -376,6 +409,7 @@ class CertificateTests(ModuleStoreTestCase): """ with mock.patch(BETA_TESTER_METHOD, return_value=True): assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert _set_v2_cert_status(self.user, self.course_run_key) is None def test_can_generate_failing_grade(self): """ @@ -383,6 +417,7 @@ class CertificateTests(ModuleStoreTestCase): """ with mock.patch(PASSING_GRADE_METHOD, return_value=False): assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert _set_v2_cert_status(self.user, self.course_run_key) == CertificateStatuses.notpassing def test_can_generate_not_enrolled(self): """ @@ -392,6 +427,7 @@ class CertificateTests(ModuleStoreTestCase): cr = CourseFactory() key = cr.id # pylint: disable=no-member assert not _can_generate_v2_certificate(u, key) + assert _set_v2_cert_status(u, key) is None def test_can_generate_audit(self): """ @@ -408,6 +444,7 @@ class CertificateTests(ModuleStoreTestCase): ) assert not _can_generate_v2_certificate(u, key) + assert _set_v2_cert_status(u, key) is None def test_can_generate_invalidated(self): """ @@ -435,6 +472,7 @@ class CertificateTests(ModuleStoreTestCase): ) assert not _can_generate_v2_certificate(u, key) + assert _set_v2_cert_status(u, key) == CertificateStatuses.unavailable def test_can_generate_web_cert_disabled(self): """ @@ -442,3 +480,71 @@ class CertificateTests(ModuleStoreTestCase): """ with mock.patch(WEB_CERTS_METHOD, return_value=False): assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert _set_v2_cert_status(self.user, self.course_run_key) is None + + @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False) + def test_cert_status_v1(self): + """ + Test cert status with V1 of course certs + """ + assert _set_v2_cert_status(self.user, self.course_run_key) is None + + def test_cert_status_downloadable(self): + """ + Test cert status when status is already downloadable + """ + u = UserFactory() + cr = CourseFactory() + key = cr.id # pylint: disable=no-member + CourseEnrollmentFactory( + user=u, + course_id=key, + is_active=True, + mode="verified", + ) + GeneratedCertificateFactory( + user=u, + course_id=key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.downloadable + ) + + assert _set_v2_cert_status(u, key) is None + + def test_cert_status_none(self): + """ + Test cert status when the user has no cert + """ + u = UserFactory() + cr = CourseFactory() + key = cr.id # pylint: disable=no-member + CourseEnrollmentFactory( + user=u, + course_id=key, + is_active=True, + mode="verified", + ) + + assert _set_v2_cert_status(u, key) == CertificateStatuses.notpassing + + def test_cert_status_generating(self): + """ + Test cert status when status is generating + """ + u = UserFactory() + cr = CourseFactory() + key = cr.id # pylint: disable=no-member + CourseEnrollmentFactory( + user=u, + course_id=key, + is_active=True, + mode="verified", + ) + GeneratedCertificateFactory( + user=u, + course_id=key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.generating + ) + + assert _set_v2_cert_status(u, key) == CertificateStatuses.notpassing diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index f0e21e09c1..3a37a693a2 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -344,3 +344,60 @@ class CertificateInvalidationTest(SharedModuleStoreTestCase): assert mock_revoke_task.call_count == 1 assert mock_revoke_task.call_args[0] == (self.user.username, str(self.course_id)) + + +class GeneratedCertificateTest(SharedModuleStoreTestCase): + """ + Test GeneratedCertificates + """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + + self.course = CourseOverviewFactory() + self.course_key = self.course.id + + def test_invalidate(self): + """ + Test the invalidate method + """ + cert = GeneratedCertificateFactory.create( + status=CertificateStatuses.downloadable, + user=self.user, + course_id=self.course_key + ) + cert.invalidate() + + cert = GeneratedCertificate.objects.get(user=self.user, course_id=self.course_key) + assert cert.status == CertificateStatuses.unavailable + + def test_notpassing(self): + """ + Test the notpassing method + """ + cert = GeneratedCertificateFactory.create( + status=CertificateStatuses.downloadable, + user=self.user, + course_id=self.course_key + ) + grade = '.3' + cert.mark_notpassing(grade) + + 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): + """ + Test the unverified method + """ + cert = GeneratedCertificateFactory.create( + status=CertificateStatuses.downloadable, + user=self.user, + course_id=self.course_key + ) + cert.mark_unverified() + + cert = GeneratedCertificate.objects.get(user=self.user, course_id=self.course_key) + assert cert.status == CertificateStatuses.unverified