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 4dc85dc1fdc6..c9ab850529d8 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 9e32365da505..2b4a3caffd64 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 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 @@ 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 @@ 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 @@ def test_get_certificate_with_deleted_course(self): 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): # pylint: disable=arguments-differ - super().setUp('lms.djangoapps.certificates.utils.tracker') + def setUp(self): + super().setUp() - self.student = UserFactory.create( - email='joe_user@edx.org', - username='joeuser', - password='foo' - ) - 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.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.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 523ff2c8101c..a369279b5980 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -13,12 +13,12 @@ _can_generate_allowlist_certificate, _can_generate_certificate_for_status, _can_generate_regular_certificate, - generate_allowlist_certificate_task, - generate_certificate_task, _generate_regular_certificate_task, - is_on_certificate_allowlist, _set_allowlist_cert_status, - _set_regular_cert_status + _set_regular_cert_status, + generate_allowlist_certificate_task, + generate_certificate_task, + is_on_certificate_allowlist ) from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.certificates.tests.factories import ( @@ -26,6 +26,7 @@ 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 @@ 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 @@ def setUp(self): 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 @@ def test_handle_invalid(self): """ 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 @@ def test_can_generate_not_verified(self): 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 @@ def test_can_generate_not_enrolled(self): 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 @@ def test_can_generate_audit(self): 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 @@ def test_can_generate_not_allowlisted(self): 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 @@ def test_can_generate_invalidated(self): 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 @@ def test_cert_status_downloadable(self): 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 @@ def setUp(self): 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 @@ def test_handle_invalid(self): 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 @@ def test_handle_not_passing_id_verified_no_cert(self): ) 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 @@ def test_handle_not_passing_id_verified_cert(self): ) 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 @@ def test_can_generate_not_verified_cert(self): ) 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 @@ def test_can_generate_not_verified_no_cert(self): ) 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 @@ def test_can_generate_not_verified_not_passing(self): 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 @@ def test_can_generate_not_verified_not_passing_allowlist(self): 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 @@ def test_can_generate_not_passing_cert(self): ) 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 @@ def test_can_generate_not_enrolled(self): 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 @@ def test_can_generate_audit(self): 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 @@ def test_can_generate_invalidated(self): 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 @@ def test_cert_status_downloadable(self): 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 4c9a63a5d31a..b152bd8106b6 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 @@ def test_certificate_generation_for_students(self): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(66): + with self.assertNumQueries(71): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data(