From 9a24e72643d9dc0fd7494aab22205fa2c4fdf86a Mon Sep 17 00:00:00 2001
From: Christie Rice <8483753+crice100@users.noreply.github.com>
Date: Thu, 25 Mar 2021 09:29:51 -0400
Subject: [PATCH] fix: Require web certs (#27120)
MICROBA-1039
---
.../decisions/001-allowlist-requirements.rst | 1 +
.../certificates/generation_handler.py | 24 +++++++++++++++++--
.../tests/test_generation_handler.py | 17 +++++++++++++
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst b/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst
index c96dc66f86b..f007ef7d33a 100644
--- a/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst
+++ b/lms/djangoapps/certificates/docs/decisions/001-allowlist-requirements.rst
@@ -31,6 +31,7 @@ the time the certificate is generated:
* The user must be on the allowlist for the course run (see the *CertificateWhitelist* model)
* The user must not have an invalidated certificate for the course run (see the *CertificateInvalidation* model)
* Automatic certificate generation must be globally enabled
+* HTML (web) certificates must be globally enabled, and also enabled for the course run
Note: the above requirements were written for the allowlist, which assumes the
CourseWaffleFlag *certificates_revamp.use_allowlist* has been enabled for the
diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py
index 434271574f0..7bf55c5c271 100644
--- a/lms/djangoapps/certificates/generation_handler.py
+++ b/lms/djangoapps/certificates/generation_handler.py
@@ -52,7 +52,8 @@ CERTIFICATES_USE_ALLOWLIST = CourseWaffleFlag(
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable the updated regular (non-allowlist) course certificate logic on a
-# per-course run basis.
+# per-course run basis. Note that if this flag is enabled for a course run, certificates_revamp.use_allowlist
+# should also be enabled for that course run.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2021-03-05
# .. toggle_target_removal_date: 2022-03-05
@@ -183,6 +184,12 @@ def _can_generate_allowlist_certificate(user, course_key):
log.info(f'{user.id} does not have a verified id. Allowlist certificate cannot be generated for {course_key}.')
return False
+ course = _get_course(course_key)
+ if not has_html_certificates_enabled(course):
+ log.info(f'{course_key} does not have HTML certificates enabled. Allowlist certificate cannot be generated for '
+ f'{user.id}.')
+ return False
+
log.info(f'{user.id} : {course_key} is on the certificate allowlist')
return _can_generate_allowlist_certificate_for_status(user, course_key)
@@ -226,7 +233,7 @@ def _can_generate_v2_certificate(user, course_key):
log.info(f'{course_key} is a CCX course. Certificate cannot be generated for {user.id}.')
return False
- course = modulestore().get_course(course_key, depth=0)
+ course = _get_course(course_key)
if _is_beta_tester(user, course):
log.info(f'{user.id} is a beta tester in {course_key}. Certificate cannot be generated.')
return False
@@ -238,6 +245,12 @@ def _can_generate_v2_certificate(user, course_key):
if not _can_generate_certificate_for_status(user, course_key):
return False
+ course = _get_course(course_key)
+ if not has_html_certificates_enabled(course):
+ log.info(f'{course_key} does not have HTML certificates enabled. Certificate cannot be generated for '
+ f'{user.id}.')
+ return False
+
log.info(f'V2 certificate can be generated for {user.id} : {course_key}')
return True
@@ -332,6 +345,13 @@ def _has_passing_grade(user, course):
return course_grade.passed
+def _get_course(course_key):
+ """
+ Get the course from the course key
+ """
+ return modulestore().get_course(course_key, depth=0)
+
+
def generate_user_certificates(student, course_key, course=None, insecure=False, generation_mode='batch',
forced_grade=None):
"""
diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py
index 0d92fa6507d..3a9965e0667 100644
--- a/lms/djangoapps/certificates/tests/test_generation_handler.py
+++ b/lms/djangoapps/certificates/tests/test_generation_handler.py
@@ -41,6 +41,7 @@ BETA_TESTER_METHOD = 'lms.djangoapps.certificates.generation_handler._is_beta_te
CCX_COURSE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_ccx_course'
ID_VERIFIED_METHOD = 'lms.djangoapps.verify_student.services.IDVerificationService.user_is_verified'
PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._has_passing_grade'
+WEB_CERTS_METHOD = 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled'
AUTO_GENERATION_NAMESPACE = waffle.WAFFLE_NAMESPACE
AUTO_GENERATION_NAME = waffle.AUTO_CERTIFICATE_GENERATION
AUTO_GENERATION_SWITCH_NAME = f'{AUTO_GENERATION_NAMESPACE}.{AUTO_GENERATION_NAME}'
@@ -50,6 +51,7 @@ AUTO_GENERATION_SWITCH = LegacyWaffleSwitch(AUTO_GENERATION_NAMESPACE, AUTO_GENE
@override_switch(AUTO_GENERATION_SWITCH_NAME, active=True)
@override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True)
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
+@mock.patch(WEB_CERTS_METHOD, mock.Mock(return_value=True))
@ddt.ddt
class AllowlistTests(ModuleStoreTestCase):
"""
@@ -259,12 +261,20 @@ class AllowlistTests(ModuleStoreTestCase):
assert not _can_generate_allowlist_certificate(u, key)
+ def test_can_generate_web_cert_disabled(self):
+ """
+ Test handling when web certs are not enabled
+ """
+ with mock.patch(WEB_CERTS_METHOD, return_value=False):
+ assert not _can_generate_allowlist_certificate(self.user, self.course_run_key)
+
@override_switch(AUTO_GENERATION_SWITCH_NAME, active=True)
@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
@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))
+@mock.patch(WEB_CERTS_METHOD, mock.Mock(return_value=True))
@ddt.ddt
class CertificateTests(ModuleStoreTestCase):
"""
@@ -441,3 +451,10 @@ class CertificateTests(ModuleStoreTestCase):
)
assert not _can_generate_v2_certificate(u, key)
+
+ def test_can_generate_web_cert_disabled(self):
+ """
+ Test handling when web certs are not enabled
+ """
+ with mock.patch(WEB_CERTS_METHOD, return_value=False):
+ assert not _can_generate_v2_certificate(self.user, self.course_run_key)
--
GitLab