Merge pull request #27246 from edx/jhynes/microba-1109_update_checks

fix: update v2 certificate generation eligibility logic
This commit is contained in:
Justin Hynes
2021-04-06 10:14:40 -04:00
committed by GitHub
8 changed files with 11 additions and 44 deletions

View File

@@ -31,7 +31,6 @@ the time the certificate is generated:
* The user must have an approved, unexpired, ID verification
* The user must not have an invalidated certificate for the course run (see the *CertificateInvalidation* model)
* Automatic certificate generation must be globally enabled
* HTML (web) certificates must be globally enabled, and also enabled for the course run
* The user must be on the allowlist for the course run (see the *CertificateWhitelist* model)

View File

@@ -26,7 +26,6 @@ be true at the time the certificate is generated:
* The user must have an approved, unexpired, ID verification
* The user must not have an invalidated certificate for the course run (see the *CertificateInvalidation* model)
* Automatic certificate generation must be globally enabled
* HTML (web) certificates must be globally enabled, and also enabled for the course run
* The user must have passed the course run
* The user must not be a beta tester in the course run

View File

@@ -24,7 +24,6 @@ from lms.djangoapps.certificates.utils import emit_certificate_event, has_html_c
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.instructor.access import list_with_level
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.djangoapps.certificates.api import auto_certificate_generation_enabled
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
from xmodule.modulestore.django import modulestore
@@ -209,12 +208,6 @@ def _can_generate_certificate_common(user, course_key):
This method contains checks that are common to both allowlist and V2 regular course certificates.
"""
if not auto_certificate_generation_enabled():
# Automatic certificate generation is globally disabled
log.info(f'Automatic certificate generation is globally disabled. Certificate cannot be generated for '
f'{user.id} : {course_key}.')
return False
if CertificateInvalidation.has_certificate_invalidation(user, course_key):
# The invalidation list prevents certificate generation
log.info(f'{user.id} : {course_key} is on the certificate invalidation list. Certificate cannot be generated.')

View File

@@ -9,12 +9,11 @@ from django.core.management import CommandError, call_command
from waffle.testutils import override_switch
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.certificates.tests.test_generation_handler import AUTO_GENERATION_SWITCH_NAME, ID_VERIFIED_METHOD
from lms.djangoapps.certificates.tests.test_generation_handler import ID_VERIFIED_METHOD
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@override_switch(AUTO_GENERATION_SWITCH_NAME, active=True)
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
class CertGenerationTests(ModuleStoreTestCase):
"""

View File

@@ -57,14 +57,14 @@ def _listen_for_certificate_whitelist_append(sender, instance, **kwargs): # pyl
"""
Listen for a user being added to or modified on the whitelist (allowlist)
"""
if not auto_certificate_generation_enabled():
return
if is_using_certificate_allowlist_and_is_on_allowlist(instance.user, instance.course_id):
log.info(f'{instance.course_id} is using allowlist certificates, and the user {instance.user.id} is now on '
f'its allowlist. Attempt will be made to generate an allowlist certificate.')
return generate_allowlist_certificate_task(instance.user, instance.course_id)
if not auto_certificate_generation_enabled():
return
if _fire_ungenerated_certificate_task(instance.user, instance.course_id):
log.info('Certificate generation task initiated for {user} : {course} via whitelist'.format(
user=instance.user.id,
@@ -78,14 +78,14 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disa
Listen for a learner passing a course, send cert generation task,
downstream signal from COURSE_GRADE_CHANGED
"""
if not auto_certificate_generation_enabled():
return
if can_generate_certificate_task(user, course_id):
log.info(f'{course_id} is using V2 certificates. Attempt will be made to generate a V2 certificate for '
f'{user.id} as a passing grade was received.')
return generate_certificate_task(user, course_id)
if not auto_certificate_generation_enabled():
return
if _fire_ungenerated_certificate_task(user, course_id):
log.info('Certificate generation task initiated for {user} : {course} via passing grade'.format(
user=user.id,

View File

@@ -5,9 +5,7 @@ import logging
from unittest import mock
import ddt
from edx_toggles.toggles import LegacyWaffleSwitch
from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch
from waffle.testutils import override_switch
from edx_toggles.toggles.testutils import override_waffle_flag
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.certificates.generation_handler import (
@@ -30,7 +28,6 @@ from lms.djangoapps.certificates.tests.factories import (
CertificateWhitelistFactory,
GeneratedCertificateFactory
)
from openedx.core.djangoapps.certificates.config import waffle
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -41,13 +38,8 @@ CCX_COURSE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_ccx_cour
ID_VERIFIED_METHOD = 'lms.djangoapps.verify_student.services.IDVerificationService.user_is_verified'
PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._has_passing_grade'
WEB_CERTS_METHOD = 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled'
AUTO_GENERATION_NAMESPACE = waffle.WAFFLE_NAMESPACE
AUTO_GENERATION_NAME = waffle.AUTO_CERTIFICATE_GENERATION
AUTO_GENERATION_SWITCH_NAME = f'{AUTO_GENERATION_NAMESPACE}.{AUTO_GENERATION_NAME}'
AUTO_GENERATION_SWITCH = LegacyWaffleSwitch(AUTO_GENERATION_NAMESPACE, AUTO_GENERATION_NAME)
@override_switch(AUTO_GENERATION_SWITCH_NAME, active=True)
@override_waffle_flag(CERTIFICATES_USE_ALLOWLIST, active=True)
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
@mock.patch(WEB_CERTS_METHOD, mock.Mock(return_value=True))
@@ -176,13 +168,6 @@ class AllowlistTests(ModuleStoreTestCase):
assert can_generate_certificate_task(self.user, self.course_run_key)
assert generate_certificate_task(self.user, self.course_run_key)
def test_can_generate_auto_disabled(self):
"""
Test handling when automatic generation is disabled
"""
with override_waffle_switch(AUTO_GENERATION_SWITCH, active=False):
assert not _can_generate_allowlist_certificate(self.user, self.course_run_key)
def test_can_generate_not_verified(self):
"""
Test handling when the user's id is not verified
@@ -268,7 +253,6 @@ class AllowlistTests(ModuleStoreTestCase):
assert not _can_generate_allowlist_certificate(self.user, self.course_run_key)
@override_switch(AUTO_GENERATION_SWITCH_NAME, active=True)
@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True)
@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=True))
@mock.patch(CCX_COURSE_METHOD, mock.Mock(return_value=False))
@@ -372,13 +356,6 @@ class CertificateTests(ModuleStoreTestCase):
"""
assert _can_generate_certificate_for_status(None, None)
def test_can_generate_auto_disabled(self):
"""
Test handling when automatic generation is disabled
"""
with override_waffle_switch(AUTO_GENERATION_SWITCH, active=False):
assert not _can_generate_v2_certificate(self.user, self.course_run_key)
def test_can_generate_not_verified(self):
"""
Test handling when the user's id is not verified

View File

@@ -98,7 +98,7 @@ class TestCourseGradeFactory(GradeTestBase):
with self.assertNumQueries(3), mock_get_score(1, 2):
_assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0
num_queries = 46
num_queries = 42
with self.assertNumQueries(num_queries), mock_get_score(1, 2):
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
@@ -119,7 +119,7 @@ class TestCourseGradeFactory(GradeTestBase):
with self.assertNumQueries(3):
_assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0
num_queries = 28
num_queries = 30
with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero
grade_factory.update(self.request.user, self.course, force_update_subsections=True)

View File

@@ -2021,7 +2021,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
'failed': 0,
'skipped': 2
}
with self.assertNumQueries(169):
with self.assertNumQueries(170):
self.assertCertificatesGenerated(task_input, expected_results)
expected_results = {