From df71aebdcf5f843fa404d43bf204d1ee8162e149 Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Mon, 22 Nov 2021 14:52:17 -0500 Subject: [PATCH] fix: [MICROBA-1594] fix cert generation logic for eligible modes that don't require IDV [MICROBA-1594] * Update course certificate generation logic when ID verification fails to check if the enrollment mode requires IDV. This change fixes an issue where Honor certificates could not be generated in an Open edX installation unless a manual IDV override was added for the student. --- .../certificates/generation_handler.py | 14 ++++- .../tests/test_generation_handler.py | 55 +++++++++++++++++++ .../tests/test_tasks_helper.py | 2 +- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index d671a4ade0..4bf08fbdda 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -9,6 +9,7 @@ cannot be generated, a message is logged and no further action is taken. import logging from common.djangoapps.course_modes import api as modes_api +from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import ( @@ -170,14 +171,21 @@ def _can_generate_certificate_common(user, course_key, enrollment_mode): log.info(f'{user.id} : {course_key} does not have an enrollment. Certificate cannot be generated.') return False - if not modes_api.is_eligible_for_certificate(enrollment_mode): + is_eligible_for_cert = modes_api.is_eligible_for_certificate(enrollment_mode) + if not is_eligible_for_cert: log.info(f'{user.id} : {course_key} has an enrollment mode of {enrollment_mode}, which is not eligible for a ' f'certificate. Certificate cannot be generated.') return False + # If the IDV check fails we then check if the course-run requires ID verification. Honor and Professional-No-ID + # modes do not require IDV for certificate generation. if _required_verification_missing(course_key, user): - log.info(f'{user.id} does not have a verified id. Certificate cannot be generated for {course_key}.') - return False + if enrollment_mode not in CourseMode.NON_VERIFIED_MODES: + log.info(f'{user.id} does not have a verified id. Certificate cannot be generated for {course_key}.') + return False + + log.info(f'{user.id} : {course_key} is eligible for a certificate without requiring a verified ID. ' + 'Skipping results of the ID verification check.') if not _can_generate_certificate_for_status(user, course_key, enrollment_mode): return False diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index c590c192cf..6b12a0a6c9 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -5,6 +5,8 @@ import logging from unittest import mock import ddt +from django.conf import settings +from django.test import override_settings from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -330,6 +332,31 @@ class AllowlistTests(ModuleStoreTestCase): assert _set_allowlist_cert_status(u, key, self.enrollment_mode, self.grade) is None + def test_generate_allowlist_honor_cert(self): + """ + Test that verifies we can generate an Honor cert for an Open edX installation configured to support Honor + certificates. + """ + course_run = CourseFactory() + course_run_key = course_run.id # pylint: disable=no-member + enrollment_mode = CourseMode.HONOR + CourseEnrollmentFactory( + user=self.user, + course_id=course_run_key, + is_active=True, + mode=enrollment_mode, + ) + + CertificateAllowlistFactory.create(course_id=course_run_key, user=self.user) + + # Enable Honor Certificates and verify we can generate an AllowList certificate + with override_settings(FEATURES={**settings.FEATURES, 'DISABLE_HONOR_CERTIFICATES': False}): + assert _can_generate_allowlist_certificate(self.user, course_run_key, enrollment_mode) + + # Disable Honor Certificates and verify we cannot generate an AllowList certificate + with override_settings(FEATURES={**settings.FEATURES, 'DISABLE_HONOR_CERTIFICATES': True}): + assert not _can_generate_allowlist_certificate(self.user, course_run_key, enrollment_mode) + @mock.patch(INTEGRITY_ENABLED_METHOD, mock.Mock(return_value=False)) @mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True)) @@ -741,3 +768,31 @@ class CertificateTests(ModuleStoreTestCase): ) assert _set_regular_cert_status(u, key, self.enrollment_mode, self.grade) is None + + def test_can_generate_honor_cert(self): + """ + Test that verifies we can generate an Honor cert for an Open edX installation configured to support Honor + certificates. + """ + course_run = CourseFactory() + course_run_key = course_run.id # pylint: disable=no-member + enrollment_mode = CourseMode.HONOR + grade = CourseGradeFactory().read(self.user, course_run) + CourseEnrollmentFactory( + user=self.user, + course_id=course_run_key, + is_active=True, + mode=enrollment_mode, + ) + + # Enable Honor Certificates and verify we can generate a certificate + with mock.patch(ID_VERIFIED_METHOD, return_value=False), \ + mock.patch(PASSING_GRADE_METHOD, return_value=True), \ + override_settings(FEATURES={**settings.FEATURES, 'DISABLE_HONOR_CERTIFICATES': False}): + assert _can_generate_regular_certificate(self.user, course_run_key, enrollment_mode, grade) + + # Disable Honor Certificates and verify we cannot generate a certificate + with mock.patch(ID_VERIFIED_METHOD, return_value=False), \ + mock.patch(PASSING_GRADE_METHOD, return_value=True), \ + override_settings(FEATURES={**settings.FEATURES, 'DISABLE_HONOR_CERTIFICATES': True}): + assert not _can_generate_regular_certificate(self.user, course_run_key, enrollment_mode, grade) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index fb25f0e5b9..462bc0d82b 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -2081,7 +2081,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(77): + with self.assertNumQueries(82): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data(