From c7d06c6bfacaa01d12d38f518a2e29d03c3cb597 Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Thu, 15 Jul 2021 11:23:14 -0400 Subject: [PATCH] fix: Retrieve grade and enrollment mode only once and pass their values to the generation task, so those values can be saved in the cert. Also protect against missing course grades. MICROBA-1373 --- .../certificates/generation_handler.py | 110 +++++++----- lms/djangoapps/certificates/tests/test_api.py | 53 +++--- .../tests/test_generation_handler.py | 157 +++++++++++------- .../tests/test_tasks_helper.py | 2 +- 4 files changed, 197 insertions(+), 125 deletions(-) diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 4dc85dc1fd..c9ab850529 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -47,10 +47,13 @@ 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 _can_generate_allowlist_certificate(user, course_key): - return _generate_certificate_task(user=user, course_key=course_key, generation_mode=generation_mode) + enrollment_mode = _get_enrollment_mode(user, course_key) + course_grade = _get_course_grade(user, course_key) + if _can_generate_allowlist_certificate(user, course_key, enrollment_mode): + return _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode, + course_grade=course_grade, generation_mode=generation_mode) - status = _set_allowlist_cert_status(user, course_key) + status = _set_allowlist_cert_status(user, course_key, enrollment_mode, course_grade) if status is not None: return True @@ -62,25 +65,32 @@ 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_regular_certificate(user, course_key): - return _generate_certificate_task(user=user, course_key=course_key, generation_mode=generation_mode) + enrollment_mode = _get_enrollment_mode(user, course_key) + course_grade = _get_course_grade(user, course_key) + if _can_generate_regular_certificate(user, course_key, enrollment_mode, course_grade): + return _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode, + course_grade=course_grade, generation_mode=generation_mode) - status = _set_regular_cert_status(user, course_key) + status = _set_regular_cert_status(user, course_key, enrollment_mode, course_grade) if status is not None: return True return False -def _generate_certificate_task(user, course_key, status=None, generation_mode=None): +def _generate_certificate_task(user, course_key, enrollment_mode, course_grade, status=None, generation_mode=None): """ Create a task to generate a certificate """ log.info(f'About to create a regular certificate task for {user.id} : {course_key}') + course_grade_val = _get_grade_value(course_grade) + kwargs = { 'student': str(user.id), - 'course_key': str(course_key) + 'course_key': str(course_key), + 'enrollment_mode': str(enrollment_mode), + 'course_grade': str(course_grade_val) } if status is not None: kwargs['status'] = status @@ -91,7 +101,7 @@ def _generate_certificate_task(user, course_key, status=None, generation_mode=No return True -def _can_generate_allowlist_certificate(user, course_key): +def _can_generate_allowlist_certificate(user, course_key, enrollment_mode): """ Check if an allowlist certificate can be generated (created if it doesn't already exist, or updated if it does exist) for this user, in this course run. @@ -103,7 +113,7 @@ def _can_generate_allowlist_certificate(user, course_key): log.info(f'{user.id} : {course_key} is on the certificate allowlist') - if not _can_generate_certificate_common(user, course_key): + if not _can_generate_certificate_common(user, course_key, enrollment_mode): log.info(f'One of the common checks failed. Allowlist certificate cannot be generated for {user.id} : ' f'{course_key}.') return False @@ -112,7 +122,7 @@ def _can_generate_allowlist_certificate(user, course_key): return True -def _can_generate_regular_certificate(user, course_key): +def _can_generate_regular_certificate(user, course_key, enrollment_mode, course_grade): """ 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. @@ -125,11 +135,11 @@ def _can_generate_regular_certificate(user, course_key): log.info(f'{user.id} is a beta tester in {course_key}. Certificate cannot be generated.') return False - if not _has_passing_grade(user, course_key): + if not _is_passing_grade(course_grade): log.info(f'{user.id} does not have a passing grade in {course_key}. Certificate cannot be generated.') return False - if not _can_generate_certificate_common(user, course_key): + if not _can_generate_certificate_common(user, course_key, enrollment_mode): log.info(f'One of the common checks failed. Certificate cannot be generated for {user.id} : {course_key}.') return False @@ -137,7 +147,7 @@ def _can_generate_regular_certificate(user, course_key): return True -def _can_generate_certificate_common(user, course_key): +def _can_generate_certificate_common(user, course_key, enrollment_mode): """ 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. @@ -149,7 +159,6 @@ def _can_generate_certificate_common(user, course_key): log.info(f'{user.id} : {course_key} is on the certificate invalidation list. Certificate cannot be generated.') return False - enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) if enrollment_mode is None: log.info(f'{user.id} : {course_key} does not have an enrollment. Certificate cannot be generated.') return False @@ -179,45 +188,45 @@ def _can_generate_certificate_common(user, course_key): return True -def _set_allowlist_cert_status(user, course_key): +def _set_allowlist_cert_status(user, course_key, enrollment_mode, course_grade): """ 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): + if not _can_set_allowlist_cert_status(user, course_key, enrollment_mode): return None cert = GeneratedCertificate.certificate_for_student(user, course_key) - return _get_cert_status_common(user, course_key, cert) + return _get_cert_status_common(user, course_key, enrollment_mode, course_grade, cert) -def _set_regular_cert_status(user, course_key): +def _set_regular_cert_status(user, course_key, enrollment_mode, course_grade): """ 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_regular_cert_status(user, course_key): + if not _can_set_regular_cert_status(user, course_key, enrollment_mode): return None cert = GeneratedCertificate.certificate_for_student(user, course_key) - status = _get_cert_status_common(user, course_key, cert) + status = _get_cert_status_common(user, course_key, enrollment_mode, course_grade, cert) if status is not None: return status - if IDVerificationService.user_is_verified(user) and not _has_passing_grade(user, course_key) and cert is not None: + if IDVerificationService.user_is_verified(user) and not _is_passing_grade(course_grade) 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') + course_grade_val = _get_grade_value(course_grade) + cert.mark_notpassing(course_grade_val, source='certificate_generation') return CertificateStatuses.notpassing return None -def _get_cert_status_common(user, course_key, cert): +def _get_cert_status_common(user, course_key, enrollment_mode, course_grade, cert): """ Determine the certificate status for this user, in this course run. @@ -229,10 +238,12 @@ def _get_cert_status_common(user, course_key, cert): cert.invalidate(source='certificate_generation') return CertificateStatuses.unavailable - if not IDVerificationService.user_is_verified(user) and _has_passing_grade_or_is_allowlisted(user, course_key): + if not IDVerificationService.user_is_verified(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, generation_mode='batch', - status=CertificateStatuses.unverified) + _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode, + course_grade=course_grade, status=CertificateStatuses.unverified, + generation_mode='batch') elif cert.status != CertificateStatuses.unverified: cert.mark_unverified(source='certificate_generation') return CertificateStatuses.unverified @@ -240,17 +251,17 @@ def _get_cert_status_common(user, course_key, cert): return None -def _can_set_allowlist_cert_status(user, course_key): +def _can_set_allowlist_cert_status(user, course_key, enrollment_mode): """ Determine whether we can set a custom (non-downloadable) cert status for an allowlist certificate """ if not is_on_certificate_allowlist(user, course_key): return False - return _can_set_cert_status_common(user, course_key) + return _can_set_cert_status_common(user, course_key, enrollment_mode) -def _can_set_regular_cert_status(user, course_key): +def _can_set_regular_cert_status(user, course_key, enrollment_mode): """ Determine whether we can set a custom (non-downloadable) cert status for a regular (non-allowlist) certificate """ @@ -260,17 +271,16 @@ def _can_set_regular_cert_status(user, course_key): if _is_beta_tester(user, course_key): return False - return _can_set_cert_status_common(user, course_key) + return _can_set_cert_status_common(user, course_key, enrollment_mode) -def _can_set_cert_status_common(user, course_key): +def _can_set_cert_status_common(user, course_key, enrollment_mode): """ 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 @@ -329,7 +339,7 @@ def _is_ccx_course(course_key): return hasattr(course_key, 'ccx') -def _has_passing_grade_or_is_allowlisted(user, course_key): +def _has_passing_grade_or_is_allowlisted(user, course_key, course_grade): """ 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. @@ -337,24 +347,42 @@ def _has_passing_grade_or_is_allowlisted(user, course_key): if is_on_certificate_allowlist(user, course_key): return True - return _has_passing_grade(user, course_key) + return _is_passing_grade(course_grade) -def _has_passing_grade(user, course_key): +def _is_passing_grade(course_grade): """ - Check if the user has a passing grade in this course run + Check if the grade is a passing grade """ - course_grade = _get_course_grade(user, course_key) - return course_grade.passed + if course_grade: + return course_grade.passed + return False + + +def _get_grade_value(course_grade): + """ + Get the user's course grade as a percent, or an empty string if there is no grade + """ + if course_grade: + return course_grade.percent + return '' def _get_course_grade(user, course_key): """ - Get the user's course grade in this course run + Get the user's course grade in this course run. Note that this may be None. """ return CourseGradeFactory().read(user, course_key=course_key) +def _get_enrollment_mode(user, course_key): + """ + Get the user's enrollment mode for this course run. Note that this may be None. + """ + enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) + return enrollment_mode + + def _is_cert_downloadable(user, course_key): """ Check if cert already exists, has a downloadable status, and has not been invalidated diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 9e32365da5..2b4a3caffd 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -4,6 +4,7 @@ import uuid from contextlib import contextmanager from datetime import datetime, timedelta +from unittest import mock from unittest.mock import patch import ddt @@ -23,7 +24,6 @@ from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory -from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import ( CourseEnrollmentFactory, GlobalStaffFactory, @@ -37,6 +37,7 @@ from lms.djangoapps.certificates.api import ( create_certificate_invalidation_entry, create_or_update_certificate_allowlist_entry, example_certificates_status, + generate_certificate_task, generate_example_certificates, get_allowlist_entry, get_allowlisted_users, @@ -66,6 +67,7 @@ from lms.djangoapps.certificates.tests.factories import ( GeneratedCertificateFactory, CertificateInvalidationFactory ) +from lms.djangoapps.certificates.tests.test_generation_handler import ID_VERIFIED_METHOD, PASSING_GRADE_METHOD from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration @@ -536,39 +538,46 @@ class CertificateGetTests(SharedModuleStoreTestCase): assert get_certificate_for_user(self.student.username, self.nonexistent_course_id) is None -@override_settings(CERT_QUEUE='certificates') -class GenerateUserCertificatesTest(EventTestMixin, WebCertificateTestMixin, ModuleStoreTestCase): +class GenerateUserCertificatesTest(ModuleStoreTestCase): """Tests for generating certificates for students. """ - ERROR_REASON = "Kaboom!" - ENABLED_SIGNALS = ['course_published'] + def setUp(self): + super().setUp() - def setUp(self): # pylint: disable=arguments-differ - super().setUp('lms.djangoapps.certificates.utils.tracker') - - self.student = UserFactory.create( - email='joe_user@edx.org', - username='joeuser', - password='foo' + self.user = UserFactory() + self.course_run = CourseFactory() + self.course_run_key = self.course_run.id # pylint: disable=no-member + self.enrollment = CourseEnrollmentFactory( + user=self.user, + course_id=self.course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, ) - self.student_no_cert = UserFactory() - self.course = CourseFactory.create( - org='edx', - number='verified', - display_name='Verified Course', - grade_cutoffs={'cutoff': 0.75, 'Pass': 0.5} - ) - self.enrollment = CourseEnrollment.enroll(self.student, self.course.id, mode='honor') - self.request_factory = RequestFactory() @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': False}) def test_cert_url_empty_with_invalid_certificate(self): """ Test certificate url is empty if html view is not enabled and certificate is not yet generated """ - url = get_certificate_url(self.student.id, self.course.id) + url = get_certificate_url(self.user.id, self.course_run_key) assert url == '' + @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) + def test_generation(self): + """ + Test that a cert is successfully generated + """ + cert = get_certificate_for_user_id(self.user.id, self.course_run_key) + assert not cert + + with mock.patch(PASSING_GRADE_METHOD, return_value=True): + with mock.patch(ID_VERIFIED_METHOD, return_value=True): + generate_certificate_task(self.user, self.course_run_key) + + cert = get_certificate_for_user_id(self.user.id, self.course_run_key) + assert cert.status == CertificateStatuses.downloadable + assert cert.mode == CourseMode.VERIFIED + @ddt.ddt class CertificateGenerationEnabledTest(EventTestMixin, TestCase): diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 523ff2c810..a369279b59 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -13,12 +13,12 @@ from lms.djangoapps.certificates.generation_handler import ( _can_generate_allowlist_certificate, _can_generate_certificate_for_status, _can_generate_regular_certificate, + _generate_regular_certificate_task, + _set_allowlist_cert_status, + _set_regular_cert_status, generate_allowlist_certificate_task, generate_certificate_task, - _generate_regular_certificate_task, - is_on_certificate_allowlist, - _set_allowlist_cert_status, - _set_regular_cert_status + is_on_certificate_allowlist ) from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.certificates.tests.factories import ( @@ -26,6 +26,7 @@ from lms.djangoapps.certificates.tests.factories import ( CertificateInvalidationFactory, GeneratedCertificateFactory ) +from lms.djangoapps.grades.api import CourseGradeFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -34,8 +35,9 @@ log = logging.getLogger(__name__) BETA_TESTER_METHOD = 'lms.djangoapps.certificates.generation_handler._is_beta_tester' 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' ID_VERIFIED_METHOD = 'lms.djangoapps.verify_student.services.IDVerificationService.user_is_verified' -PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._has_passing_grade' +PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_passing_grade' WEB_CERTS_METHOD = 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled' @@ -54,11 +56,13 @@ class AllowlistTests(ModuleStoreTestCase): self.user = UserFactory() self.course_run = CourseFactory() self.course_run_key = self.course_run.id # pylint: disable=no-member + self.enrollment_mode = CourseMode.VERIFIED + self.grade = CourseGradeFactory().read(self.user, self.course_run) self.enrollment = CourseEnrollmentFactory( user=self.user, course_id=self.course_run_key, is_active=True, - mode=CourseMode.VERIFIED, + mode=self.enrollment_mode, ) # Add user to the allowlist @@ -177,16 +181,16 @@ class AllowlistTests(ModuleStoreTestCase): """ u = UserFactory() - assert not _can_generate_allowlist_certificate(u, self.course_run_key) + assert not _can_generate_allowlist_certificate(u, self.course_run_key, self.enrollment_mode) assert not generate_allowlist_certificate_task(u, self.course_run_key) assert not generate_certificate_task(u, self.course_run_key) - assert _set_allowlist_cert_status(u, self.course_run_key) is None + assert _set_allowlist_cert_status(u, self.course_run_key, self.enrollment_mode, self.grade) is None def test_handle_valid(self): """ Test handling of a valid user/course run combo """ - assert _can_generate_allowlist_certificate(self.user, self.course_run_key) + assert _can_generate_allowlist_certificate(self.user, self.course_run_key, self.enrollment_mode) assert generate_allowlist_certificate_task(self.user, self.course_run_key) def test_handle_valid_general_methods(self): @@ -200,8 +204,9 @@ class AllowlistTests(ModuleStoreTestCase): 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) - assert _set_allowlist_cert_status(self.user, self.course_run_key) == CertificateStatuses.unverified + 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 def test_can_generate_not_enrolled(self): """ @@ -210,9 +215,11 @@ class AllowlistTests(ModuleStoreTestCase): u = UserFactory() cr = CourseFactory() key = cr.id # pylint: disable=no-member + mode = None + grade = None CertificateAllowlistFactory.create(course_id=key, user=u) - assert not _can_generate_allowlist_certificate(u, key) - assert _set_allowlist_cert_status(u, key) is None + assert not _can_generate_allowlist_certificate(u, key, mode) + assert _set_allowlist_cert_status(u, key, mode, grade) is None def test_can_generate_audit(self): """ @@ -221,16 +228,17 @@ class AllowlistTests(ModuleStoreTestCase): u = UserFactory() cr = CourseFactory() key = cr.id # pylint: disable=no-member + mode = CourseMode.AUDIT CourseEnrollmentFactory( user=u, course_id=key, is_active=True, - mode=GeneratedCertificate.MODES.audit, + mode=mode, ) CertificateAllowlistFactory.create(course_id=key, user=u) - assert not _can_generate_allowlist_certificate(u, key) - assert _set_allowlist_cert_status(u, key) is None + assert not _can_generate_allowlist_certificate(u, key, mode) + assert _set_allowlist_cert_status(u, key, mode, self.grade) is None def test_can_generate_not_allowlisted(self): """ @@ -245,8 +253,8 @@ class AllowlistTests(ModuleStoreTestCase): is_active=True, mode=CourseMode.VERIFIED, ) - assert not _can_generate_allowlist_certificate(u, key) - assert _set_allowlist_cert_status(u, key) is None + assert not _can_generate_allowlist_certificate(u, key, self.enrollment_mode) + assert _set_allowlist_cert_status(u, key, self.enrollment_mode, self.grade) is None def test_can_generate_invalidated(self): """ @@ -274,24 +282,24 @@ class AllowlistTests(ModuleStoreTestCase): active=True ) - assert not _can_generate_allowlist_certificate(u, key) - assert _set_allowlist_cert_status(u, key) == CertificateStatuses.unavailable + assert not _can_generate_allowlist_certificate(u, key, self.enrollment_mode) + assert _set_allowlist_cert_status(u, key, self.enrollment_mode, self.grade) == 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_allowlist_certificate(self.user, self.course_run_key) - assert _set_allowlist_cert_status(self.user, self.course_run_key) is None + 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) 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_allowlist_certificate(self.user, self.course_run_key) - assert _set_allowlist_cert_status(self.user, self.course_run_key) is None + 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) is None def test_cert_status_downloadable(self): """ @@ -313,7 +321,7 @@ class AllowlistTests(ModuleStoreTestCase): status=CertificateStatuses.downloadable ) - assert _set_allowlist_cert_status(u, key) is None + assert _set_allowlist_cert_status(u, key, self.enrollment_mode, self.grade) is None @mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True)) @@ -333,18 +341,20 @@ class CertificateTests(ModuleStoreTestCase): self.user = UserFactory() self.course_run = CourseFactory() self.course_run_key = self.course_run.id # pylint: disable=no-member + self.enrollment_mode = CourseMode.VERIFIED + self.grade = CourseGradeFactory().read(self.user, self.course_run) self.enrollment = CourseEnrollmentFactory( user=self.user, course_id=self.course_run_key, is_active=True, - mode=CourseMode.VERIFIED, + mode=self.enrollment_mode, ) def test_handle_valid(self): """ Test handling of a valid user/course run combo. """ - assert _can_generate_regular_certificate(self.user, self.course_run_key) + assert _can_generate_regular_certificate(self.user, self.course_run_key, self.enrollment_mode, self.grade) assert generate_certificate_task(self.user, self.course_run_key) def test_handle_valid_task(self): @@ -361,23 +371,33 @@ class CertificateTests(ModuleStoreTestCase): Test handling of an invalid user/course run combo """ other_user = UserFactory() - assert not _can_generate_regular_certificate(other_user, self.course_run_key) + mode = None + grade = None + assert not _can_generate_regular_certificate(other_user, self.course_run_key, mode, grade) assert not generate_certificate_task(other_user, self.course_run_key) assert not _generate_regular_certificate_task(other_user, self.course_run_key) + def test_handle_no_grade(self): + """ + Test handling when the grade is none + """ + with mock.patch(GET_GRADE_METHOD, return_value=None): + assert generate_certificate_task(self.user, self.course_run_key) + def test_handle_audit_status(self): """ Test handling of a user who is not passing and is enrolled in audit mode """ different_user = UserFactory() + mode = CourseMode.AUDIT CourseEnrollmentFactory( user=different_user, course_id=self.course_run_key, is_active=True, - mode=GeneratedCertificate.MODES.audit, + mode=mode, ) - assert _set_regular_cert_status(different_user, self.course_run_key) is None + assert _set_regular_cert_status(different_user, self.course_run_key, mode, self.grade) 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,7 +413,8 @@ class CertificateTests(ModuleStoreTestCase): ) with mock.patch(PASSING_GRADE_METHOD, return_value=False): - assert _set_regular_cert_status(different_user, self.course_run_key) is None + assert _set_regular_cert_status(different_user, self.course_run_key, self.enrollment_mode, + self.grade) 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 +436,11 @@ class CertificateTests(ModuleStoreTestCase): ) with mock.patch(PASSING_GRADE_METHOD, return_value=False): - assert _set_regular_cert_status(different_user, self.course_run_key) == CertificateStatuses.notpassing + assert _set_regular_cert_status(different_user, self.course_run_key, self.enrollment_mode, self.grade) == \ + 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) + assert not _can_generate_regular_certificate(different_user, self.course_run_key, self.enrollment_mode, + self.grade) @ddt.data( (CertificateStatuses.deleted, True), @@ -476,8 +499,9 @@ class CertificateTests(ModuleStoreTestCase): ) with mock.patch(ID_VERIFIED_METHOD, return_value=False): - assert not _can_generate_regular_certificate(u, self.course_run_key) - assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.unverified + 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 def test_can_generate_not_verified_no_cert(self): """ @@ -492,8 +516,9 @@ class CertificateTests(ModuleStoreTestCase): ) with mock.patch(ID_VERIFIED_METHOD, return_value=False): - assert not _can_generate_regular_certificate(u, self.course_run_key) - assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.unverified + 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 def test_can_generate_not_verified_not_passing(self): """ @@ -515,8 +540,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_regular_certificate(u, self.course_run_key) - assert _set_regular_cert_status(u, self.course_run_key) is None + 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) is None def test_can_generate_not_verified_not_passing_allowlist(self): """ @@ -539,32 +564,36 @@ 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_regular_certificate(u, self.course_run_key) - assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.unverified + 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 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_regular_certificate(self.user, self.course_run_key) - assert _set_regular_cert_status(self.user, self.course_run_key) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key, self.enrollment_mode, + self.grade) + assert _set_regular_cert_status(self.user, self.course_run_key, self.enrollment_mode, self.grade) 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_regular_certificate(self.user, self.course_run_key) - assert _set_regular_cert_status(self.user, self.course_run_key) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key, self.enrollment_mode, + self.grade) + assert _set_regular_cert_status(self.user, self.course_run_key, self.enrollment_mode, self.grade) 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_regular_certificate(self.user, self.course_run_key) - assert _set_regular_cert_status(self.user, self.course_run_key) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key, self.enrollment_mode, + self.grade) + assert _set_regular_cert_status(self.user, self.course_run_key, self.enrollment_mode, self.grade) is None def test_can_generate_not_passing_cert(self): """ @@ -585,8 +614,9 @@ class CertificateTests(ModuleStoreTestCase): ) with mock.patch(PASSING_GRADE_METHOD, return_value=False): - assert not _can_generate_regular_certificate(u, self.course_run_key) - assert _set_regular_cert_status(u, self.course_run_key) == CertificateStatuses.notpassing + 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.notpassing def test_can_generate_not_enrolled(self): """ @@ -595,8 +625,10 @@ class CertificateTests(ModuleStoreTestCase): u = UserFactory() cr = CourseFactory() key = cr.id # pylint: disable=no-member - assert not _can_generate_regular_certificate(u, key) - assert _set_regular_cert_status(u, key) is None + mode = None + grade = None + assert not _can_generate_regular_certificate(u, key, mode, grade) + assert _set_regular_cert_status(u, key, mode, grade) is None def test_can_generate_audit(self): """ @@ -605,15 +637,16 @@ class CertificateTests(ModuleStoreTestCase): u = UserFactory() cr = CourseFactory() key = cr.id # pylint: disable=no-member + mode = CourseMode.AUDIT CourseEnrollmentFactory( user=u, course_id=key, is_active=True, - mode=GeneratedCertificate.MODES.audit, + mode=mode, ) - assert not _can_generate_regular_certificate(u, key) - assert _set_regular_cert_status(u, key) is None + assert not _can_generate_regular_certificate(u, key, mode, self.grade) + assert _set_regular_cert_status(u, key, mode, self.grade) is None def test_can_generate_invalidated(self): """ @@ -640,24 +673,26 @@ class CertificateTests(ModuleStoreTestCase): active=True ) - assert not _can_generate_regular_certificate(u, key) - assert _set_regular_cert_status(u, key) == CertificateStatuses.unavailable + assert not _can_generate_regular_certificate(u, key, self.enrollment_mode, self.grade) + assert _set_regular_cert_status(u, key, self.enrollment_mode, self.grade) == 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_regular_certificate(self.user, self.course_run_key) - assert _set_regular_cert_status(self.user, self.course_run_key) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key, self.enrollment_mode, + self.grade) + assert _set_regular_cert_status(self.user, self.course_run_key, self.enrollment_mode, self.grade) 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_regular_certificate(self.user, self.course_run_key) - assert _set_regular_cert_status(self.user, self.course_run_key) is None + assert not _can_generate_regular_certificate(self.user, self.course_run_key, self.enrollment_mode, + self.grade) + assert _set_regular_cert_status(self.user, self.course_run_key, self.enrollment_mode, self.grade) is None def test_cert_status_downloadable(self): """ @@ -679,4 +714,4 @@ class CertificateTests(ModuleStoreTestCase): status=CertificateStatuses.downloadable ) - assert _set_regular_cert_status(u, key) is None + assert _set_regular_cert_status(u, key, self.enrollment_mode, self.grade) is None diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 4c9a63a5d3..b152bd8106 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -2082,7 +2082,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(66): + with self.assertNumQueries(71): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data(