Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: make IDV optional for cert generation

Browse Source 
IDV is on its way to retirement, so it's not going to be necessary for cert
generation forever.

Introduces a function to combine the honor code flag with IDV to tell
cert generation if it should care about a missing verification.

Various tests expanded to cover the retired case. The additional calls
in test_task_helper.py are caused by one call to fetch course
overrides which finds none, and that forces one check of the
background flag per student, 71 + 1 + 5 = 77.

MST-854
This commit is contained in:
Andy Shultz
2021-10-21 15:45:20 -04:00
parent eec1d345d8
commit 7e3ef93a63
3 changed files with 63 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
lms/djangoapps/certificates/generation_handler.py
View File

@@ -21,6 +21,7 @@ from lms.djangoapps.certificates.utils import has_html_certificates_enabled
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.instructor.access import is_beta_tester
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.djangoapps.agreements.toggles import is_integrity_signature_enabled
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
log = logging.getLogger(__name__)
@@ -168,7 +169,7 @@ def _can_generate_certificate_common(user, course_key, enrollment_mode):
f'certificate. Certificate cannot be generated.')
return False
if not IDVerificationService.user_is_verified(user):
if _required_verification_missing(course_key, user):
log.info(f'{user.id} does not have a verified id. Certificate cannot be generated for {course_key}.')
return False
@@ -217,7 +218,9 @@ def _set_regular_cert_status(user, course_key, enrollment_mode, course_grade):
if status is not None:
return status
if IDVerificationService.user_is_verified(user) and not _is_passing_grade(course_grade) and cert is not None:
if not _required_verification_missing(course_key, user) \
and not _is_passing_grade(course_grade) \
and cert is not None:
if cert.status != CertificateStatuses.notpassing:
course_grade_val = _get_grade_value(course_grade)
cert.mark_notpassing(mode=enrollment_mode, grade=course_grade_val, source='certificate_generation')
@@ -238,7 +241,7 @@ def _get_cert_status_common(user, course_key, enrollment_mode, course_grade, cer
cert.invalidate(mode=enrollment_mode, source='certificate_generation')
return CertificateStatuses.unavailable
if not IDVerificationService.user_is_verified(user) and _has_passing_grade_or_is_allowlisted(user, course_key,
if _required_verification_missing(course_key, user) and _has_passing_grade_or_is_allowlisted(user, course_key,
course_grade):
if cert is None:
_generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode,
@@ -397,3 +400,10 @@ def _is_mode_now_eligible(enrollment_mode, cert):
if modes_api.is_eligible_for_certificate(enrollment_mode) and not modes_api.is_eligible_for_certificate(cert.mode):
return True
return False
def _required_verification_missing(course_key, user):
"""
Return true if IDV is required for this course and the user does not have it
"""
return not is_integrity_signature_enabled(course_key) and not IDVerificationService.user_is_verified(user)

72
lms/djangoapps/certificates/tests/test_generation_handler.py
View File

@@ -36,11 +36,13 @@ BETA_TESTER_METHOD = 'lms.djangoapps.certificates.generation_handler.is_beta_tes
COURSE_OVERVIEW_METHOD = 'lms.djangoapps.certificates.generation_handler.get_course_overview_or_none'
CCX_COURSE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_ccx_course'
GET_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._get_course_grade'
INTEGRITY_ENABLED_METHOD = 'lms.djangoapps.certificates.generation_handler.is_integrity_signature_enabled'
ID_VERIFIED_METHOD = 'lms.djangoapps.verify_student.services.IDVerificationService.user_is_verified'
PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_passing_grade'
WEB_CERTS_METHOD = 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled'
@mock.patch(INTEGRITY_ENABLED_METHOD, mock.Mock(return_value=False))
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
@mock.patch(WEB_CERTS_METHOD, mock.Mock(return_value=True))
@ddt.ddt
@@ -199,14 +201,19 @@ class AllowlistTests(ModuleStoreTestCase):
"""
assert generate_certificate_task(self.user, self.course_run_key)
def test_can_generate_not_verified(self):
@ddt.data(False, True)
def test_can_generate_not_verified(self, idv_retired):
"""
Test handling when the user's id is not verified
"""
with mock.patch(ID_VERIFIED_METHOD, return_value=False):
assert not _can_generate_allowlist_certificate(self.user, self.course_run_key, self.enrollment_mode)
assert _set_allowlist_cert_status(self.user, self.course_run_key, self.enrollment_mode, self.grade) == \
CertificateStatuses.unverified
with mock.patch(ID_VERIFIED_METHOD, return_value=False), \
mock.patch(INTEGRITY_ENABLED_METHOD, return_value=idv_retired):
self.assertEqual(idv_retired,
_can_generate_allowlist_certificate(self.user, self.course_run_key, self.enrollment_mode))
self.assertIsNot(idv_retired,
_set_allowlist_cert_status(
self.user, self.course_run_key,
self.enrollment_mode, self.grade) == CertificateStatuses.unverified)
def test_can_generate_not_enrolled(self):
"""
@@ -324,6 +331,7 @@ class AllowlistTests(ModuleStoreTestCase):
assert _set_allowlist_cert_status(u, key, self.enrollment_mode, self.grade) is None
@mock.patch(INTEGRITY_ENABLED_METHOD, mock.Mock(return_value=False))
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
@mock.patch(CCX_COURSE_METHOD, mock.Mock(return_value=False))
@mock.patch(PASSING_GRADE_METHOD, mock.Mock(return_value=True))
@@ -480,7 +488,8 @@ class CertificateTests(ModuleStoreTestCase):
"""
assert _can_generate_certificate_for_status(None, None, None)
def test_can_generate_not_verified_cert(self):
@ddt.data(False, True)
def test_can_generate_not_verified_cert(self, idv_retired):
"""
Test handling when the user's id is not verified and they have a cert
"""
@@ -498,12 +507,15 @@ class CertificateTests(ModuleStoreTestCase):
status=CertificateStatuses.generating
)
with mock.patch(ID_VERIFIED_METHOD, return_value=False):
assert not _can_generate_regular_certificate(u, self.course_run_key, self.enrollment_mode, self.grade)
assert _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode,
self.grade) == CertificateStatuses.unverified
with mock.patch(ID_VERIFIED_METHOD, return_value=False), \
mock.patch(INTEGRITY_ENABLED_METHOD, return_value=idv_retired):
self.assertEqual(idv_retired, _can_generate_regular_certificate(u, self.course_run_key,
self.enrollment_mode, self.grade))
self.assertIsNot(idv_retired, _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode,
self.grade) == CertificateStatuses.unverified)
def test_can_generate_not_verified_no_cert(self):
@ddt.data(False, True)
def test_can_generate_not_verified_no_cert(self, idv_retired):
"""
Test handling when the user's id is not verified and they don't have a cert
"""
@@ -515,12 +527,15 @@ class CertificateTests(ModuleStoreTestCase):
mode=CourseMode.VERIFIED,
)
with mock.patch(ID_VERIFIED_METHOD, return_value=False):
assert not _can_generate_regular_certificate(u, self.course_run_key, self.enrollment_mode, self.grade)
assert _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode,
self.grade) == CertificateStatuses.unverified
with mock.patch(ID_VERIFIED_METHOD, return_value=False), \
mock.patch(INTEGRITY_ENABLED_METHOD, return_value=idv_retired):
self.assertEqual(idv_retired, _can_generate_regular_certificate(u, self.course_run_key,
self.enrollment_mode, self.grade))
self.assertIsNot(idv_retired, _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode,
self.grade) == CertificateStatuses.unverified)
def test_can_generate_not_verified_not_passing(self):
@ddt.data(False, True)
def test_can_generate_not_verified_not_passing(self, idv_retired):
"""
Test handling when the user's id is not verified and the user is not passing
"""
@@ -538,12 +553,18 @@ class CertificateTests(ModuleStoreTestCase):
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_regular_certificate(u, self.course_run_key, self.enrollment_mode, self.grade)
with mock.patch(ID_VERIFIED_METHOD, return_value=False), \
mock.patch(INTEGRITY_ENABLED_METHOD, return_value=idv_retired), \
mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert not _can_generate_regular_certificate(u, self.course_run_key, self.enrollment_mode, self.grade)
if idv_retired:
assert _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode, self.grade) \
== CertificateStatuses.notpassing
else:
assert _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode, self.grade) is None
def test_can_generate_not_verified_not_passing_allowlist(self):
@ddt.data(False, True)
def test_can_generate_not_verified_not_passing_allowlist(self, idv_retired):
"""
Test handling when the user's id is not verified and the user is not passing but is on the allowlist
"""
@@ -562,9 +583,14 @@ class CertificateTests(ModuleStoreTestCase):
)
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_regular_certificate(u, self.course_run_key, self.enrollment_mode, self.grade)
with mock.patch(ID_VERIFIED_METHOD, return_value=False), \
mock.patch(INTEGRITY_ENABLED_METHOD, return_value=idv_retired), \
mock.patch(PASSING_GRADE_METHOD, return_value=False):
assert not _can_generate_regular_certificate(u, self.course_run_key, self.enrollment_mode, self.grade)
if idv_retired:
assert _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode,
self.grade) == CertificateStatuses.notpassing
else:
assert _set_regular_cert_status(u, self.course_run_key, self.enrollment_mode,
self.grade) == CertificateStatuses.unverified

2
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

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