diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index a8b59b7cb9..d671a4ade0 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -27,7 +27,7 @@ from openedx.core.djangoapps.content.course_overviews.api import get_course_over log = logging.getLogger(__name__) -def generate_certificate_task(user, course_key, generation_mode=None): +def generate_certificate_task(user, course_key, generation_mode=None, delay_seconds=CERTIFICATE_DELAY_SECONDS): """ Create a task to generate a certificate for this user in this course run, if the user is eligible and a certificate can be generated. @@ -38,13 +38,16 @@ def generate_certificate_task(user, course_key, generation_mode=None): if is_on_certificate_allowlist(user, course_key): log.info(f'User {user.id} is on the allowlist for {course_key}. Attempt will be made to generate an allowlist ' f'certificate.') - return generate_allowlist_certificate_task(user, course_key, generation_mode) + return generate_allowlist_certificate_task(user, course_key, generation_mode=generation_mode, + delay_seconds=delay_seconds) 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=generation_mode, + delay_seconds=delay_seconds) -def generate_allowlist_certificate_task(user, course_key, generation_mode=None): +def generate_allowlist_certificate_task(user, course_key, generation_mode=None, + delay_seconds=CERTIFICATE_DELAY_SECONDS): """ Create a task to generate an allowlist certificate for this user in this course run. """ @@ -52,7 +55,8 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None): 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) + course_grade=course_grade, generation_mode=generation_mode, + delay_seconds=delay_seconds) status = _set_allowlist_cert_status(user, course_key, enrollment_mode, course_grade) if status is not None: @@ -61,7 +65,7 @@ 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, delay_seconds=CERTIFICATE_DELAY_SECONDS): """ 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. @@ -70,7 +74,8 @@ def _generate_regular_certificate_task(user, course_key, generation_mode=None): 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) + course_grade=course_grade, generation_mode=generation_mode, + delay_seconds=delay_seconds) status = _set_regular_cert_status(user, course_key, enrollment_mode, course_grade) if status is not None: @@ -79,7 +84,8 @@ def _generate_regular_certificate_task(user, course_key, generation_mode=None): return False -def _generate_certificate_task(user, course_key, enrollment_mode, course_grade, status=None, generation_mode=None): +def _generate_certificate_task(user, course_key, enrollment_mode, course_grade, status=None, generation_mode=None, + delay_seconds=CERTIFICATE_DELAY_SECONDS): """ Create a task to generate a certificate """ @@ -98,7 +104,7 @@ def _generate_certificate_task(user, course_key, enrollment_mode, course_grade, if generation_mode is not None: kwargs['generation_mode'] = generation_mode - generate_certificate.apply_async(countdown=CERTIFICATE_DELAY_SECONDS, kwargs=kwargs) + generate_certificate.apply_async(countdown=delay_seconds, kwargs=kwargs) return True diff --git a/lms/djangoapps/certificates/management/commands/regenerate_noidv_cert.py b/lms/djangoapps/certificates/management/commands/regenerate_noidv_cert.py new file mode 100644 index 0000000000..2bd4e57ae6 --- /dev/null +++ b/lms/djangoapps/certificates/management/commands/regenerate_noidv_cert.py @@ -0,0 +1,97 @@ +""" +Management command to regenerate unverified certificates when a course +transitions to honor code. +""" +import logging +import time + +from django.contrib.auth import get_user_model +from django.core.management.base import BaseCommand, CommandError +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey + +from lms.djangoapps.certificates.data import CertificateStatuses +from lms.djangoapps.certificates.generation_handler import generate_certificate_task +from lms.djangoapps.certificates.models import GeneratedCertificate +from openedx.core.djangoapps.agreements.toggles import is_integrity_signature_enabled + +log = logging.getLogger(__name__) +User = get_user_model() + + +class Command(BaseCommand): + """ + Management command to regenerate unverified certificates when a course + transitions to honor code. + """ + def add_arguments(self, parser): + parser.add_argument( + '-c', '--course-keys', + nargs='+', + dest='course_keys', + help='course run key or space separated list of course run keys' + ) + parser.add_argument( + '--batch_size', + action='store', + dest='batch_size', + type=int, + default=200, + help='Number of certs per batch' + ) + parser.add_argument( + '--sleep_seconds', + action='store', + dest='sleep_seconds', + type=int, + default=20, + help='Seconds to sleep between batches' + ) + + def handle(self, *args, **options): + courses_str = options['course_keys'] + if not courses_str: + raise CommandError('You must specify a course-key or keys') + + batch_size = options['batch_size'] + sleep_seconds = options['sleep_seconds'] + course_keys = [] + for course_id in courses_str: + # Parse the serialized course key into a CourseKey + try: + course_key = CourseKey.from_string(course_id) + except InvalidKeyError as e: + raise CommandError(f'{course_id} is not a valid course-key') from e + course_keys.append(course_key) + + count = 0 + for key in course_keys: + #rolling count to maintain batch_size across courses + count = _handle_course(key, batch_size, sleep_seconds, count) + return f'{count}' + + +def _handle_course(course_key, batch_size, sleep_seconds, count): + """ + Regenerates unverified status certificates for the designated course with delay seconds between certs. + Returns how many certs were originally unverified and regenerated. + """ + + if not is_integrity_signature_enabled(course_key): + log.warning(f'Skipping {course_key} which does not have honor code enabled') + return 0 + + certs = GeneratedCertificate.objects.filter( + course_id=course_key, + status=CertificateStatuses.unverified, + ) + + log.info(f'Regenerating {len(certs)} unverified certificates for {course_key}') + + for cert in certs: + user = User.objects.get(id=cert.user_id) + generate_certificate_task(user, course_key, generation_mode='batch', delay_seconds=0) + count += 1 + if count % batch_size == 0: + time.sleep(sleep_seconds) + return count diff --git a/lms/djangoapps/certificates/management/commands/tests/test_regenerate_noidv_cert.py b/lms/djangoapps/certificates/management/commands/tests/test_regenerate_noidv_cert.py new file mode 100644 index 0000000000..6273b8814c --- /dev/null +++ b/lms/djangoapps/certificates/management/commands/tests/test_regenerate_noidv_cert.py @@ -0,0 +1,229 @@ +""" +Tests for the cert_generation command +""" + +from unittest import mock + +import pytest +from django.core.management import CommandError, call_command + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory +from lms.djangoapps.certificates.data import CertificateStatuses +from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +COMMAND_INTEGRITY_ENABLED = \ + 'lms.djangoapps.certificates.management.commands.regenerate_noidv_cert.is_integrity_signature_enabled' +INTEGRITY_ENABLED_METHOD = 'lms.djangoapps.certificates.generation_handler.is_integrity_signature_enabled' +ID_VERIFIED_METHOD = 'lms.djangoapps.verify_student.services.IDVerificationService.user_is_verified' +PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_passing_grade' +WEB_CERTS_METHOD = 'lms.djangoapps.certificates.generation_handler.has_html_certificates_enabled' + + +# base setup is unverified users, honor code (integrity) turned on wherever imports it +# and normal passing grade certificates for convenience +@mock.patch(ID_VERIFIED_METHOD, mock.Mock(return_value=False)) +@mock.patch(INTEGRITY_ENABLED_METHOD, mock.Mock(return_value=True)) +@mock.patch(COMMAND_INTEGRITY_ENABLED, mock.Mock(return_value=True)) +@mock.patch(PASSING_GRADE_METHOD, mock.Mock(return_value=True)) +@mock.patch(WEB_CERTS_METHOD, mock.Mock(return_value=True)) +class RegenerateNoIDVCertTests(ModuleStoreTestCase): + """ + Tests for the regenerate_noidv_cert management command + """ + + def test_command_with_missing_param_course_key(self): + """ + Verify command with a missing param -- course key. + """ + with pytest.raises(CommandError, match="You must specify a course-key or keys"): + call_command("regenerate_noidv_cert") + + def test_command_with_invalid_key(self): + """ + Verify command with an invalid course run key + """ + with pytest.raises(CommandError, match=" is not a valid course-key"): + call_command("regenerate_noidv_cert", "-c", "blah") + + def test_regeneration(self): + """ + Single regeneration base case + """ + course_run = CourseFactory() + course_run_key = course_run.id + + user = UserFactory() + CourseEnrollmentFactory( + user=user, + course_id=course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user, + course_id=course_run_key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.unverified + ) + + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key) + self.assertEqual('1', regenerated) + + def test_regeneration_verified(self): + """ + Only unverified certificates should get regenerated + """ + course_run = CourseFactory() + course_run_key = course_run.id + + user = UserFactory() + CourseEnrollmentFactory( + user=user, + course_id=course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user, + course_id=course_run_key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.downloadable + ) + + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key) + self.assertEqual('0', regenerated) + + def test_regeneration_honor_off(self): + """ + If a course does not have the honor code enabled, no point regenerating + """ + course_run = CourseFactory() + course_run_key = course_run.id + + user = UserFactory() + CourseEnrollmentFactory( + user=user, + course_id=course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user, + course_id=course_run_key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.unverified + ) + + with mock.patch(COMMAND_INTEGRITY_ENABLED, mock.Mock(return_value=False)): + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key) + self.assertEqual('0', regenerated) + + def _multisetup(self): + """ + setup certs across two course runs + """ + course_run = CourseFactory() + course_run_key = course_run.id + course_run2 = CourseFactory() + course_run_key2 = course_run2.id + + user = UserFactory() + CourseEnrollmentFactory( + user=user, + course_id=course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user, + course_id=course_run_key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.unverified + ) + + user1 = UserFactory() + CourseEnrollmentFactory( + user=user1, + course_id=course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user1, + course_id=course_run_key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.unverified + ) + + user2 = UserFactory() + CourseEnrollmentFactory( + user=user2, + course_id=course_run_key2, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user2, + course_id=course_run_key2, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.unverified + ) + + user_verified = UserFactory() + CourseEnrollmentFactory( + user=user_verified, + course_id=course_run_key, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user_verified, + course_id=course_run_key, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.downloadable + ) + + user_failing = UserFactory() + CourseEnrollmentFactory( + user=user_failing, + course_id=course_run_key2, + is_active=True, + mode=CourseMode.VERIFIED, + ) + GeneratedCertificateFactory( + user=user_failing, + course_id=course_run_key2, + mode=GeneratedCertificate.MODES.verified, + status=CertificateStatuses.notpassing + ) + return (course_run_key, course_run_key2) + + def test_multiple_regeneration(self): + """ + Verify regeneration across multiple courses and users + """ + course_run_key, course_run_key2 = self._multisetup() + + #3/5 in unverified status + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key, course_run_key2) + self.assertEqual('3', regenerated) + + #nothing left unverified for another run + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key, course_run_key2) + self.assertEqual('0', regenerated) + + def test_course_at_a_time(self): + """ + Verify course regeneration separated + """ + course_run_key, course_run_key2 = self._multisetup() + + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key) + self.assertEqual('2', regenerated) + + regenerated = call_command("regenerate_noidv_cert", "-c", course_run_key2) + self.assertEqual('1', regenerated) diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index 184a353652..6d524352c3 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/lms/djangoapps/certificates/tasks.py @@ -15,6 +15,9 @@ from lms.djangoapps.certificates.generation import generate_course_certificate log = getLogger(__name__) User = get_user_model() + +# Certificate generation is delayed in case the caller is still completing their changes +# (for example a certificate regeneration reacting to a post save rather than post commit signal) CERTIFICATE_DELAY_SECONDS = 2