fix: Update logic for certificate generation status, and create cert with all available info (#28004)

MICROBA-1306 CR-3792
This commit is contained in:
Christie Rice
2021-06-28 10:05:34 -04:00
committed by GitHub
parent b23169560f
commit 24367919bb
8 changed files with 252 additions and 92 deletions

View File

@@ -0,0 +1,56 @@
Course certificate Status
=========================
Status
------
Accepted
Background
----------
Course certificates can have any one of a number of status values (for the
full list, see the `CertificateStatuses model`_). For a certificate to be
available for the user to see, it must be in the *downloadable* state, meaning
it must have a status value of *downloadable*.
Decision
--------
The course certificate code will only set the following certificate statuses:
* downloadable - The user has been granted this certificate and the certificate is ready and available
* notpassing - The user has not achieved a passing grade
* unavailable - Certificate has been invalidated
* unverified - The user does not have an approved, unexpired identity verification
Other status values are kept for historical reasons and because existing course
certificates may have been granted that status.
Consequences
------------
If a certificate has been invalidated, its status will be *unavailable*.
If all requirements have been met, a *downloadable* certificate will be
generated (created or updated).
If all requirements *except* for identity verification have been met, an
*unverified* certificate will be generated.
If all requirements *except* for passing the course have been met and a
certificate already exists, its status will be *notpassing*. Alternately, if a
*downloadable* certificate exists and a failing grade signal is received for
a user who is not on the allowlist, the certificate's status will be
*notpassing*.
Note that this does not create a hierarchy of certificate statuses. For
example, a certificate could have a *notpassing* status even if the user
has not the passed identity verification requirement.
References
----------
For a full list of requirements for a course certificate to be granted, please
see the `allowlist certificate requirements`_ and `certificate requirements`_.
For a full list of certificate statuses, see the `CertificateStatuses model`_.
.. _allowlist certificate requirements: ./001-allowlist-cert-requirements.rst
.. _certificate requirements: ./002-cert-requirements.rst
.. _CertificateStatuses model: ../../data.py

View File

@@ -28,7 +28,7 @@ from openedx.core.djangoapps.content.course_overviews.api import get_course_over
log = logging.getLogger(__name__)
def generate_course_certificate(user, course_key, generation_mode):
def generate_course_certificate(user, course_key, status, generation_mode):
"""
Generate a course certificate for this user, in this course run. If the certificate has a passing status, also emit
a certificate event.
@@ -39,10 +39,11 @@ def generate_course_certificate(user, course_key, generation_mode):
Args:
user: user for whom to generate a certificate
course_key: course run key for which to generate a certificate
status: certificate status (value from the CertificateStatuses model)
generation_mode: Used when emitting an events. Options are "self" (implying the user generated the cert
themself) and "batch" for everything else.
"""
cert = _generate_certificate(user, course_key)
cert = _generate_certificate(user, course_key, status)
if CertificateStatuses.is_passing_status(cert.status):
# Emit a certificate event
@@ -56,10 +57,13 @@ def generate_course_certificate(user, course_key, generation_mode):
emit_certificate_event(event_name='created', user=user, course_id=course_key, event_data=event_data)
emit_segment_event(user_id=user.id, course_id=course_key)
elif CertificateStatuses.unverified == cert.status:
cert.mark_unverified(source='certificate_generation')
return cert
def _generate_certificate(user, course_key):
def _generate_certificate(user, course_key, status):
"""
Generate a certificate for this user, in this course run.
"""
@@ -87,7 +91,7 @@ def _generate_certificate(user, course_key):
'course_id': course_key,
'mode': enrollment_mode,
'name': profile_name,
'status': CertificateStatuses.downloadable,
'status': status,
'grade': course_grade.percent,
'download_url': '',
'key': '',

View File

@@ -95,9 +95,9 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None):
if _can_generate_allowlist_certificate(user, course_key):
return _generate_certificate_task(user, course_key, generation_mode)
# status = _set_allowlist_cert_status(user, course_key)
# if status is not None:
# return True
status = _set_allowlist_cert_status(user, course_key)
if status is not None:
return True
return False
@@ -110,14 +110,14 @@ def generate_regular_certificate_task(user, course_key, generation_mode=None):
if _can_generate_v2_certificate(user, course_key):
return _generate_certificate_task(user, course_key, generation_mode)
# status = _set_v2_cert_status(user, course_key)
# if status is not None:
# return True
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):
def _generate_certificate_task(user, course_key, status=None, generation_mode=None):
"""
Create a task to generate a certificate
"""
@@ -128,6 +128,8 @@ def _generate_certificate_task(user, course_key, generation_mode=None):
'course_key': str(course_key),
'v2_certificate': True
}
if status is not None:
kwargs['status'] = status
if generation_mode is not None:
kwargs['generation_mode'] = generation_mode
@@ -257,11 +259,9 @@ def _set_v2_cert_status(user, course_key):
if status is not None:
return status
course_grade = _get_course_grade(user, course_key)
if not course_grade.passed:
if cert is None:
cert = GeneratedCertificate.objects.create(user=user, course_id=course_key)
if IDVerificationService.user_is_verified(user) and not _has_passing_grade(user, course_key) and cert is not None:
if cert.status != CertificateStatuses.notpassing:
course_grade = _get_course_grade(user, course_key)
cert.mark_notpassing(course_grade.percent, source='certificate_generation')
return CertificateStatuses.notpassing
@@ -275,17 +275,16 @@ def _get_cert_status_common(user, course_key, 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 CertificateInvalidation.has_certificate_invalidation(user, course_key):
if cert is None:
cert = GeneratedCertificate.objects.create(user=user, course_id=course_key)
if CertificateInvalidation.has_certificate_invalidation(user, course_key) and cert is not None:
if cert.status != CertificateStatuses.unavailable:
cert.invalidate(source='certificate_generation')
return CertificateStatuses.unavailable
if not IDVerificationService.user_is_verified(user):
if not IDVerificationService.user_is_verified(user) and _has_passing_grade_or_is_allowlisted(user, course_key):
if cert is None:
cert = GeneratedCertificate.objects.create(user=user, course_id=course_key)
if cert.status != CertificateStatuses.unverified:
_generate_certificate_task(user=user, course_key=course_key, generation_mode='batch',
status=CertificateStatuses.unverified)
elif cert.status != CertificateStatuses.unverified:
cert.mark_unverified(source='certificate_generation')
return CertificateStatuses.unverified
@@ -392,6 +391,17 @@ def _is_ccx_course(course_key):
return hasattr(course_key, 'ccx')
def _has_passing_grade_or_is_allowlisted(user, course_key):
"""
Check if the user has a passing grade in this course run, or is on the allowlist and so is exempt from needing
a passing grade.
"""
if is_on_certificate_allowlist(user, course_key):
return True
return _has_passing_grade(user, course_key)
def _has_passing_grade(user, course_key):
"""
Check if the user has a passing grade in this course run

View File

@@ -10,6 +10,7 @@ from django.contrib.auth import get_user_model
from edx_django_utils.monitoring import set_code_owner_attribute
from opaque_keys.edx.keys import CourseKey
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.generation import (
generate_course_certificate,
generate_user_certificates
@@ -45,10 +46,11 @@ def generate_certificate(self, **kwargs):
course_key = CourseKey.from_string(kwargs.pop('course_key'))
expected_verification_status = kwargs.pop('expected_verification_status', None)
v2_certificate = kwargs.pop('v2_certificate', False)
status = kwargs.pop('status', CertificateStatuses.downloadable)
generation_mode = kwargs.pop('generation_mode', 'batch')
if v2_certificate:
generate_course_certificate(user=student, course_key=course_key, generation_mode=generation_mode)
generate_course_certificate(user=student, course_key=course_key, status=status, generation_mode=generation_mode)
return
if expected_verification_status:

View File

@@ -43,7 +43,7 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
certs = GeneratedCertificate.objects.filter(user=self.u, course_id=self.key)
assert len(certs) == 0
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
certs = GeneratedCertificate.objects.filter(user=self.u, course_id=self.key)
@@ -67,7 +67,7 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
mock_tracker = analytics_patcher.start()
self.addCleanup(analytics_patcher.stop)
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
mock_tracker.track.assert_called_once_with(
@@ -96,8 +96,8 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
cert = GeneratedCertificate.objects.get(user=self.u, course_id=self.key)
assert cert.error_reason == error_reason
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.unverified, self.gen_mode)
assert generated_cert.status, CertificateStatuses.unverified
cert = GeneratedCertificate.objects.get(user=self.u, course_id=self.key)
assert cert.error_reason == ''
@@ -106,7 +106,7 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
"""
Test that the `verify_uuid` value of a certificate does not change when it is revoked and re-awarded.
"""
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
verify_uuid = generated_cert.verify_uuid
@@ -115,7 +115,7 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
assert generated_cert.status, CertificateStatuses.unavailable
assert generated_cert.verify_uuid, verify_uuid
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
assert generated_cert.verify_uuid, verify_uuid
@@ -123,7 +123,7 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
assert generated_cert.status, CertificateStatuses.notpassing
assert generated_cert.verify_uuid, verify_uuid
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
assert generated_cert.verify_uuid, verify_uuid
@@ -139,6 +139,6 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
verify_uuid=''
)
generated_cert = generate_course_certificate(self.u, self.key, self.gen_mode)
generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode)
assert generated_cert.status, CertificateStatuses.downloadable
assert generated_cert.verify_uuid != ''

View File

@@ -8,6 +8,7 @@ import ddt
from edx_toggles.toggles.testutils import override_waffle_flag
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.generation_handler import (
CERTIFICATES_USE_UPDATED,
_can_generate_allowlist_certificate,
@@ -22,7 +23,6 @@ from lms.djangoapps.certificates.generation_handler import (
_set_allowlist_cert_status,
_set_v2_cert_status
)
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.certificates.tests.factories import (
CertificateAllowlistFactory,
@@ -61,7 +61,7 @@ class AllowlistTests(ModuleStoreTestCase):
user=self.user,
course_id=self.course_run_key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
# Add user to the allowlist
@@ -82,7 +82,7 @@ class AllowlistTests(ModuleStoreTestCase):
user=u,
course_id=self.course_run_key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
CertificateAllowlistFactory.create(course_id=self.course_run_key, user=u, allowlist=False)
assert not is_on_certificate_allowlist(u, self.course_run_key)
@@ -180,7 +180,7 @@ class AllowlistTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="audit",
mode=GeneratedCertificate.MODES.audit,
)
CertificateAllowlistFactory.create(course_id=key, user=u)
@@ -198,7 +198,7 @@ class AllowlistTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
assert not _can_generate_allowlist_certificate(u, key)
assert _set_allowlist_cert_status(u, key) is None
@@ -214,7 +214,7 @@ class AllowlistTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
cert = GeneratedCertificateFactory(
user=u,
@@ -259,7 +259,7 @@ class AllowlistTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=u,
@@ -293,7 +293,7 @@ class CertificateTests(ModuleStoreTestCase):
user=self.user,
course_id=self.course_run_key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
def test_handle_valid(self):
@@ -311,7 +311,7 @@ class CertificateTests(ModuleStoreTestCase):
We test generate_certificate_task() and generate_regular_certificate_task() separately since they both
generate a cert.
"""
assert generate_regular_certificate_task(self.user, self.course_run_key)
assert generate_regular_certificate_task(self.user, self.course_run_key) is True
@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False)
def test_handle_invalid(self):
@@ -338,9 +338,9 @@ class CertificateTests(ModuleStoreTestCase):
assert _set_v2_cert_status(different_user, self.course_run_key) is None
assert not generate_regular_certificate_task(different_user, self.course_run_key)
def test_handle_verified_status(self):
def test_handle_not_passing_id_verified_no_cert(self):
"""
Test handling of a user who is not passing and is enrolled in verified mode
Test handling of a user who is not passing and is id verified and has no cert
"""
different_user = UserFactory()
CourseEnrollmentFactory(
@@ -350,8 +350,32 @@ class CertificateTests(ModuleStoreTestCase):
mode=GeneratedCertificate.MODES.verified,
)
assert _set_v2_cert_status(different_user, self.course_run_key) == 'notpassing'
assert generate_regular_certificate_task(different_user, self.course_run_key)
with mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert _set_v2_cert_status(different_user, self.course_run_key) is None
assert not generate_regular_certificate_task(different_user, self.course_run_key)
def test_handle_not_passing_id_verified_cert(self):
"""
Test handling of a user who is not passing and is id verified and has a cert
"""
different_user = UserFactory()
CourseEnrollmentFactory(
user=different_user,
course_id=self.course_run_key,
is_active=True,
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=different_user,
course_id=self.course_run_key,
mode=GeneratedCertificate.MODES.verified,
status=CertificateStatuses.generating,
)
with mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert _set_v2_cert_status(different_user, self.course_run_key) == CertificateStatuses.notpassing
assert generate_regular_certificate_task(different_user, self.course_run_key) is True
assert not _can_generate_v2_certificate(different_user, self.course_run_key)
def test_is_using_updated_true(self):
"""
@@ -397,13 +421,90 @@ class CertificateTests(ModuleStoreTestCase):
"""
assert _can_generate_certificate_for_status(None, None)
def test_can_generate_not_verified(self):
def test_can_generate_not_verified_cert(self):
"""
Test handling when the user's id is not verified
Test handling when the user's id is not verified and they have a cert
"""
u = UserFactory()
CourseEnrollmentFactory(
user=u,
course_id=self.course_run_key,
is_active=True,
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=u,
course_id=self.course_run_key,
mode=GeneratedCertificate.MODES.verified,
status=CertificateStatuses.generating
)
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
assert not _can_generate_v2_certificate(u, self.course_run_key)
assert _set_v2_cert_status(u, self.course_run_key) == CertificateStatuses.unverified
def test_can_generate_not_verified_no_cert(self):
"""
Test handling when the user's id is not verified and they don't have a cert
"""
u = UserFactory()
CourseEnrollmentFactory(
user=u,
course_id=self.course_run_key,
is_active=True,
mode=GeneratedCertificate.MODES.verified,
)
with mock.patch(ID_VERIFIED_METHOD, return_value=False):
assert not _can_generate_v2_certificate(u, self.course_run_key)
assert _set_v2_cert_status(u, self.course_run_key) == CertificateStatuses.unverified
def test_can_generate_not_verified_not_passing(self):
"""
Test handling when the user's id is not verified and the user is not passing
"""
u = UserFactory()
CourseEnrollmentFactory(
user=u,
course_id=self.course_run_key,
is_active=True,
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=u,
course_id=self.course_run_key,
mode=GeneratedCertificate.MODES.verified,
status=CertificateStatuses.generating
)
with mock.patch(ID_VERIFIED_METHOD, return_value=False):
with mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert not _can_generate_v2_certificate(u, self.course_run_key)
assert _set_v2_cert_status(u, self.course_run_key) is None
def test_can_generate_not_verified_not_passing_allowlist(self):
"""
Test handling when the user's id is not verified and the user is not passing but is on the allowlist
"""
u = UserFactory()
CourseEnrollmentFactory(
user=u,
course_id=self.course_run_key,
is_active=True,
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=u,
course_id=self.course_run_key,
mode=GeneratedCertificate.MODES.verified,
status=CertificateStatuses.generating
)
CertificateAllowlistFactory(course_id=self.course_run_key, user=u)
with mock.patch(ID_VERIFIED_METHOD, return_value=False):
with mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert not _can_generate_v2_certificate(u, self.course_run_key)
assert _set_v2_cert_status(u, self.course_run_key) == CertificateStatuses.unverified
def test_can_generate_ccx(self):
"""
@@ -421,13 +522,35 @@ class CertificateTests(ModuleStoreTestCase):
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):
def test_can_generate_not_passing_no_cert(self):
"""
Test handling when the user does not have a passing grade
Test handling when the user does not have a passing grade and no cert exists
"""
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
assert _set_v2_cert_status(self.user, self.course_run_key) is None
def test_can_generate_not_passing_cert(self):
"""
Test handling when the user does not have a passing grade and a cert exists
"""
u = UserFactory()
CourseEnrollmentFactory(
user=u,
course_id=self.course_run_key,
is_active=True,
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=u,
course_id=self.course_run_key,
mode=GeneratedCertificate.MODES.verified,
status=CertificateStatuses.generating
)
with mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert not _can_generate_v2_certificate(u, self.course_run_key)
assert _set_v2_cert_status(u, self.course_run_key) == CertificateStatuses.notpassing
def test_can_generate_not_enrolled(self):
"""
@@ -450,7 +573,7 @@ class CertificateTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="audit",
mode=GeneratedCertificate.MODES.audit,
)
assert not _can_generate_v2_certificate(u, key)
@@ -467,7 +590,7 @@ class CertificateTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
cert = GeneratedCertificateFactory(
user=u,
@@ -511,7 +634,7 @@ class CertificateTests(ModuleStoreTestCase):
user=u,
course_id=key,
is_active=True,
mode="verified",
mode=GeneratedCertificate.MODES.verified,
)
GeneratedCertificateFactory(
user=u,
@@ -521,41 +644,3 @@ class CertificateTests(ModuleStoreTestCase):
)
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

View File

@@ -11,6 +11,7 @@ from django.test import TestCase
from opaque_keys.edx.keys import CourseKey
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.tasks import generate_certificate
from lms.djangoapps.verify_student.models import IDVerificationAttempt
@@ -104,6 +105,7 @@ class GenerateUserCertificateTest(TestCase):
mock_generate_cert.assert_called_with(
user=self.user,
course_key=CourseKey.from_string(course_key),
status=CertificateStatuses.downloadable,
generation_mode='batch'
)
@@ -129,5 +131,6 @@ class GenerateUserCertificateTest(TestCase):
mock_generate_cert.assert_called_with(
user=self.user,
course_key=CourseKey.from_string(course_key),
status=CertificateStatuses.downloadable,
generation_mode=gen_mode
)

View File

@@ -2087,7 +2087,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
'failed': 0,
'skipped': 2
}
with self.assertNumQueries(55):
with self.assertNumQueries(74):
self.assertCertificatesGenerated(task_input, expected_results)
@ddt.data(