From f98b8e5be0cdfc1859c2c6a04a15dce24502cf22 Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Wed, 14 Jul 2021 13:37:07 -0400 Subject: [PATCH] refactor: Replace references to V2 course certificates (#28180) MICROBA-1227 --- .../certificates/generation_handler.py | 33 ++++---- lms/djangoapps/certificates/tests/test_api.py | 2 +- .../tests/test_generation_handler.py | 84 +++++++++---------- .../certificates/tests/test_support_views.py | 2 +- 4 files changed, 60 insertions(+), 61 deletions(-) diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index d6de79ccfd..4dc85dc1fd 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -40,7 +40,7 @@ def generate_certificate_task(user, course_key, generation_mode=None): return generate_allowlist_certificate_task(user, course_key, generation_mode) log.info(f'Attempt will be made to generate course certificate for user {user.id} : {course_key}') - return generate_regular_certificate_task(user, course_key, generation_mode) + return _generate_regular_certificate_task(user, course_key, generation_mode) def generate_allowlist_certificate_task(user, course_key, generation_mode=None): @@ -57,15 +57,15 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None): return False -def generate_regular_certificate_task(user, course_key, generation_mode=None): +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 _can_generate_v2_certificate(user, course_key): + if _can_generate_regular_certificate(user, course_key): return _generate_certificate_task(user=user, course_key=course_key, generation_mode=generation_mode) - status = _set_v2_cert_status(user, course_key) + status = _set_regular_cert_status(user, course_key) if status is not None: return True @@ -76,12 +76,11 @@ def _generate_certificate_task(user, course_key, status=None, generation_mode=No """ Create a task to generate a certificate """ - log.info(f'About to create a V2 certificate task for {user.id} : {course_key}') + log.info(f'About to create a regular certificate task for {user.id} : {course_key}') kwargs = { 'student': str(user.id), - 'course_key': str(course_key), - 'v2_certificate': True + 'course_key': str(course_key) } if status is not None: kwargs['status'] = status @@ -113,10 +112,10 @@ def _can_generate_allowlist_certificate(user, course_key): return True -def _can_generate_v2_certificate(user, course_key): +def _can_generate_regular_certificate(user, course_key): """ - Check if a v2 course certificate can be generated (created if it doesn't already exist, or updated if it does - exist) for this user, in this course run. + Check if a regular (non-allowlist) course certificate can be generated (created if it doesn't already exist, or + updated if it does exist) for this user, in this course run. """ if _is_ccx_course(course_key): log.info(f'{course_key} is a CCX course. Certificate cannot be generated for {user.id}.') @@ -134,7 +133,7 @@ def _can_generate_v2_certificate(user, course_key): log.info(f'One of the common checks failed. Certificate cannot be generated for {user.id} : {course_key}.') return False - log.info(f'V2 certificate can be generated for {user.id} : {course_key}') + log.info(f'Regular certificate can be generated for {user.id} : {course_key}') return True @@ -143,7 +142,7 @@ def _can_generate_certificate_common(user, course_key): Check if a course certificate can be generated (created if it doesn't already exist, or updated if it does exist) for this user, in this course run. - This method contains checks that are common to both allowlist and V2 regular course certificates. + This method contains checks that are common to both allowlist and regular course certificates. """ if CertificateInvalidation.has_certificate_invalidation(user, course_key): # The invalidation list prevents certificate generation @@ -194,14 +193,14 @@ def _set_allowlist_cert_status(user, course_key): return _get_cert_status_common(user, course_key, cert) -def _set_v2_cert_status(user, course_key): +def _set_regular_cert_status(user, course_key): """ - Determine the V2 certificate status for this user, in this course run. + Determine the regular (non-allowlist) 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): + if not _can_set_regular_cert_status(user, course_key): return None cert = GeneratedCertificate.certificate_for_student(user, course_key) @@ -251,9 +250,9 @@ def _can_set_allowlist_cert_status(user, course_key): return _can_set_cert_status_common(user, course_key) -def _can_set_v2_cert_status(user, course_key): +def _can_set_regular_cert_status(user, course_key): """ - Determine whether we can set a custom (non-downloadable) cert status for a V2 certificate + Determine whether we can set a custom (non-downloadable) cert status for a regular (non-allowlist) certificate """ if _is_ccx_course(course_key): return False diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 60a927f9b0..9e32365da5 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -69,7 +69,7 @@ from lms.djangoapps.certificates.tests.factories import ( from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration -CAN_GENERATE_METHOD = 'lms.djangoapps.certificates.generation_handler._can_generate_v2_certificate' +CAN_GENERATE_METHOD = 'lms.djangoapps.certificates.generation_handler._can_generate_regular_certificate' FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index e9a416ed67..523ff2c810 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -12,13 +12,13 @@ from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.generation_handler import ( _can_generate_allowlist_certificate, _can_generate_certificate_for_status, - _can_generate_v2_certificate, + _can_generate_regular_certificate, generate_allowlist_certificate_task, generate_certificate_task, - generate_regular_certificate_task, + _generate_regular_certificate_task, is_on_certificate_allowlist, _set_allowlist_cert_status, - _set_v2_cert_status + _set_regular_cert_status ) from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.certificates.tests.factories import ( @@ -344,26 +344,26 @@ class CertificateTests(ModuleStoreTestCase): """ Test handling of a valid user/course run combo. """ - assert _can_generate_v2_certificate(self.user, self.course_run_key) + assert _can_generate_regular_certificate(self.user, self.course_run_key) assert generate_certificate_task(self.user, self.course_run_key) def test_handle_valid_task(self): """ Test handling of a valid user/course run combo. - We test generate_certificate_task() and generate_regular_certificate_task() separately since they both + 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) is True + assert _generate_regular_certificate_task(self.user, self.course_run_key) is True def test_handle_invalid(self): """ Test handling of an invalid user/course run combo """ other_user = UserFactory() - assert not _can_generate_v2_certificate(other_user, self.course_run_key) + assert not _can_generate_regular_certificate(other_user, self.course_run_key) assert not generate_certificate_task(other_user, self.course_run_key) - assert not generate_regular_certificate_task(other_user, self.course_run_key) + assert not _generate_regular_certificate_task(other_user, self.course_run_key) def test_handle_audit_status(self): """ @@ -377,8 +377,8 @@ class CertificateTests(ModuleStoreTestCase): mode=GeneratedCertificate.MODES.audit, ) - 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) + assert _set_regular_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_no_cert(self): """ @@ -393,8 +393,8 @@ class CertificateTests(ModuleStoreTestCase): ) 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) + assert _set_regular_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): """ @@ -415,9 +415,9 @@ class CertificateTests(ModuleStoreTestCase): ) 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) + assert _set_regular_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_regular_certificate(different_user, self.course_run_key) @ddt.data( (CertificateStatuses.deleted, True), @@ -476,8 +476,8 @@ class CertificateTests(ModuleStoreTestCase): ) 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 + assert not _can_generate_regular_certificate(u, self.course_run_key) + assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.unverified def test_can_generate_not_verified_no_cert(self): """ @@ -492,8 +492,8 @@ class CertificateTests(ModuleStoreTestCase): ) 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 + assert not _can_generate_regular_certificate(u, self.course_run_key) + assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.unverified def test_can_generate_not_verified_not_passing(self): """ @@ -515,8 +515,8 @@ class CertificateTests(ModuleStoreTestCase): 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 + assert not _can_generate_regular_certificate(u, self.course_run_key) + assert _set_regular_cert_status(u, self.course_run_key) is None def test_can_generate_not_verified_not_passing_allowlist(self): """ @@ -539,32 +539,32 @@ class CertificateTests(ModuleStoreTestCase): 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 + assert not _can_generate_regular_certificate(u, self.course_run_key) + assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.unverified def test_can_generate_ccx(self): """ Test handling when the course is a CCX (custom edX) course """ 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 + assert not _can_generate_regular_certificate(self.user, self.course_run_key) + assert _set_regular_cert_status(self.user, self.course_run_key) is None def test_can_generate_beta_tester(self): """ Test handling when the user is a beta tester """ 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 + assert not _can_generate_regular_certificate(self.user, self.course_run_key) + assert _set_regular_cert_status(self.user, self.course_run_key) is None def test_can_generate_not_passing_no_cert(self): """ 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) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key) + assert _set_regular_cert_status(self.user, self.course_run_key) is None def test_can_generate_not_passing_cert(self): """ @@ -585,8 +585,8 @@ class CertificateTests(ModuleStoreTestCase): ) 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 + assert not _can_generate_regular_certificate(u, self.course_run_key) + assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.notpassing def test_can_generate_not_enrolled(self): """ @@ -595,8 +595,8 @@ class CertificateTests(ModuleStoreTestCase): u = UserFactory() 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 + assert not _can_generate_regular_certificate(u, key) + assert _set_regular_cert_status(u, key) is None def test_can_generate_audit(self): """ @@ -612,8 +612,8 @@ class CertificateTests(ModuleStoreTestCase): mode=GeneratedCertificate.MODES.audit, ) - assert not _can_generate_v2_certificate(u, key) - assert _set_v2_cert_status(u, key) is None + assert not _can_generate_regular_certificate(u, key) + assert _set_regular_cert_status(u, key) is None def test_can_generate_invalidated(self): """ @@ -640,24 +640,24 @@ class CertificateTests(ModuleStoreTestCase): active=True ) - assert not _can_generate_v2_certificate(u, key) - assert _set_v2_cert_status(u, key) == CertificateStatuses.unavailable + assert not _can_generate_regular_certificate(u, key) + assert _set_regular_cert_status(u, key) == CertificateStatuses.unavailable 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) - assert _set_v2_cert_status(self.user, self.course_run_key) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key) + assert _set_regular_cert_status(self.user, self.course_run_key) is None def test_can_generate_no_overview(self): """ Test handling when the course overview is missing """ with mock.patch(COURSE_OVERVIEW_METHOD, return_value=None): - 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 + assert not _can_generate_regular_certificate(self.user, self.course_run_key) + assert _set_regular_cert_status(self.user, self.course_run_key) is None def test_cert_status_downloadable(self): """ @@ -679,4 +679,4 @@ class CertificateTests(ModuleStoreTestCase): status=CertificateStatuses.downloadable ) - assert _set_v2_cert_status(u, key) is None + assert _set_regular_cert_status(u, key) is None diff --git a/lms/djangoapps/certificates/tests/test_support_views.py b/lms/djangoapps/certificates/tests/test_support_views.py index 5064ac37e6..f68ad592e6 100644 --- a/lms/djangoapps/certificates/tests/test_support_views.py +++ b/lms/djangoapps/certificates/tests/test_support_views.py @@ -22,7 +22,7 @@ from openedx.core.djangoapps.content.course_overviews.tests.factories import Cou from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -CAN_GENERATE_METHOD = 'lms.djangoapps.certificates.generation_handler._can_generate_v2_certificate' +CAN_GENERATE_METHOD = 'lms.djangoapps.certificates.generation_handler._can_generate_regular_certificate' FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True