From 8f50edea6f75e2d989f4150839d9175061eb4e6c Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Wed, 31 Mar 2021 14:12:19 -0400 Subject: [PATCH] feat: Implement generation of regular V2 course certificates (#27189) MICROBA-1039 --- lms/djangoapps/certificates/api.py | 10 +++- lms/djangoapps/certificates/generation.py | 45 +++++++-------- .../certificates/generation_handler.py | 32 ++++++----- lms/djangoapps/certificates/tasks.py | 13 ++--- .../certificates/tests/test_generation.py | 15 ++--- .../tests/test_generation_handler.py | 8 +++ .../certificates/tests/test_tasks.py | 57 ++++++++++++++++++- lms/djangoapps/courseware/views/views.py | 2 +- 8 files changed, 121 insertions(+), 61 deletions(-) diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index c962e7f5d2..9b8ae7d0f1 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -222,15 +222,21 @@ def can_generate_certificate_task(user, course_key): return _can_generate_certificate_task(user, course_key) -def generate_certificate_task(user, course_key): +def generate_certificate_task(user, course_key, generation_mode=None): """ 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. If the allowlist is enabled for this course run and the user is on the allowlist, the allowlist logic will be used. Otherwise, the regular course certificate generation logic will be used. + + Args: + user: user for whom to generate a certificate + course_key: course run key for which to generate a certificate + generation_mode: Used when emitting an events. Options are "self" (implying the user generated the cert + themself) and "batch" for everything else. """ - return _generate_certificate_task(user, course_key) + return _generate_certificate_task(user, course_key, generation_mode) def certificate_downloadable_status(student, course_key): diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index 18d46de42d..4f90cdc74d 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -25,54 +25,55 @@ from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) -def generate_allowlist_certificate(user, course_key): +def generate_course_certificate(user, course_key, generation_mode): """ - Generate an allowlist certificate for this user, in this course run. This method should be called from a task. + Generate a course certificate for this user, in this course run. If the certificate has a passing status, also emit + a certificate event. + + Note that the certificate could be either an allowlist certificate or a "regular" course certificate; the content + will be the same either way. + + Args: + user: user for whom to generate a certificate + course_key: course run key for which to generate a certificate + generation_mode: Used when emitting an events. Options are "self" (implying the user generated the cert + themself) and "batch" for everything else. """ cert = _generate_certificate(user, course_key) if CertificateStatuses.is_passing_status(cert.status): - # Emit a certificate event. Note that the two options for generation_mode are "self" (implying the user - # generated the cert themself) and "batch" for everything else. + # Emit a certificate event event_data = { 'user_id': user.id, 'course_id': str(course_key), 'certificate_id': cert.verify_uuid, 'enrollment_mode': cert.mode, - 'generation_mode': 'batch' + 'generation_mode': generation_mode } emit_certificate_event(event_name='created', user=user, course_id=course_key, event_data=event_data) return cert -def generate_course_certificate(user, course_key): - """ - Generate a regular certificate for this user, in this course run. This method should be called from a task. - """ - # TODO: Implementation will be added in MICROBA-1039 - log.warning(f'Ignoring course certificate generation for {user.id}: {course_key}') - - -def _generate_certificate(user, course_id): +def _generate_certificate(user, course_key): """ Generate a certificate for this user, in this course run. """ profile = UserProfile.objects.get(user=user) profile_name = profile.name - course = modulestore().get_course(course_id, depth=0) + course = modulestore().get_course(course_key, depth=0) course_grade = CourseGradeFactory().read(user, course) - enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_id) + enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) key = make_hashkey(random.random()) uuid = uuid4().hex cert, created = GeneratedCertificate.objects.update_or_create( user=user, - course_id=course_id, + course_id=course_key, defaults={ 'user': user, - 'course_id': course_id, + 'course_id': course_key, 'mode': enrollment_mode, 'name': profile_name, 'status': CertificateStatuses.downloadable, @@ -87,13 +88,7 @@ def _generate_certificate(user, course_id): created_msg = 'Certificate was created.' else: created_msg = 'Certificate already existed and was updated.' - log.info( - 'Generated certificate with status {status} for {user} : {course}. {created_msg}'.format( - status=cert.status, - user=cert.user.id, - course=cert.course_id, - created_msg=created_msg - )) + log.info(f'Generated certificate with status {cert.status} for {user.id} : {course_key}. {created_msg}') return cert diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 224df8d192..61a988d1b0 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -81,7 +81,7 @@ def can_generate_certificate_task(user, course_key): return False -def generate_certificate_task(user, course_key): +def generate_certificate_task(user, course_key, generation_mode=None): """ 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. @@ -92,18 +92,18 @@ def generate_certificate_task(user, course_key): if is_using_certificate_allowlist_and_is_on_allowlist(user, course_key): log.info(f'{course_key} is using allowlist certificates, and the user {user.id} is on its allowlist. Attempt ' f'will be made to generate an allowlist certificate.') - return generate_allowlist_certificate_task(user, course_key) + return generate_allowlist_certificate_task(user, course_key, generation_mode) elif _is_using_v2_course_certificates(course_key): log.info(f'{course_key} is using v2 course certificates. Attempt will be made to generate a certificate for ' f'user {user.id}.') - return generate_regular_certificate_task(user, course_key) + return generate_regular_certificate_task(user, course_key, generation_mode) log.info(f'Neither an allowlist nor a v2 course certificate can be generated for {user.id} : {course_key}.') return False -def generate_allowlist_certificate_task(user, course_key): +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. """ @@ -111,18 +111,10 @@ def generate_allowlist_certificate_task(user, course_key): log.info(f'Cannot generate an allowlist certificate for {user.id} : {course_key}') return False - log.info(f'About to create an allowlist certificate task for {user.id} : {course_key}') - - kwargs = { - 'student': str(user.id), - 'course_key': str(course_key), - 'allowlist_certificate': True - } - generate_certificate.apply_async(countdown=CERTIFICATE_DELAY_SECONDS, kwargs=kwargs) - return True + return _generate_certificate_task(user, course_key, generation_mode) -def generate_regular_certificate_task(user, course_key): +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. @@ -131,13 +123,23 @@ def generate_regular_certificate_task(user, course_key): log.info(f'Cannot generate a v2 course certificate for {user.id} : {course_key}') return False - log.info(f'About to create a v2 course certificate task for {user.id} : {course_key}') + return _generate_certificate_task(user, course_key, generation_mode) + + +def _generate_certificate_task(user, course_key, generation_mode=None): + """ + Create a task to generate a certificate + """ + log.info(f'About to create a V2 certificate task for {user.id} : {course_key}') kwargs = { 'student': str(user.id), 'course_key': str(course_key), 'v2_certificate': True } + if generation_mode is not None: + kwargs['generation_mode'] = generation_mode + generate_certificate.apply_async(countdown=CERTIFICATE_DELAY_SECONDS, kwargs=kwargs) return True diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index 2691c87b7e..abba57a2c1 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/lms/djangoapps/certificates/tasks.py @@ -11,7 +11,6 @@ from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey from lms.djangoapps.certificates.generation import ( - generate_allowlist_certificate, generate_course_certificate, generate_user_certificates ) @@ -37,23 +36,19 @@ def generate_certificate(self, **kwargs): that the actual verification status is as expected before generating a certificate, in the off chance that the database has not yet updated with the user's new verification status. - - allowlist_certificate: A flag indicating whether to generate an allowlist certificate (which is V2 of - whitelisted certificates) - v2_certificate: A flag indicating whether to generate a v2 course certificate + - generation_mode: Only used when emitting an event for V2 certificates. Options are "self" (implying the user + generated the cert themself) and "batch" for everything else. """ original_kwargs = kwargs.copy() student = User.objects.get(id=kwargs.pop('student')) course_key = CourseKey.from_string(kwargs.pop('course_key')) expected_verification_status = kwargs.pop('expected_verification_status', None) - allowlist_certificate = kwargs.pop('allowlist_certificate', False) v2_certificate = kwargs.pop('v2_certificate', False) - - if allowlist_certificate: - generate_allowlist_certificate(user=student, course_key=course_key) - return + generation_mode = kwargs.pop('generation_mode', 'batch') if v2_certificate: - generate_course_certificate(user=student, course_key=course_key) + generate_course_certificate(user=student, course_key=course_key, generation_mode=generation_mode) return if expected_verification_status: diff --git a/lms/djangoapps/certificates/tests/test_generation.py b/lms/djangoapps/certificates/tests/test_generation.py index d0e76c3ba3..e34ea95f22 100644 --- a/lms/djangoapps/certificates/tests/test_generation.py +++ b/lms/djangoapps/certificates/tests/test_generation.py @@ -7,7 +7,7 @@ from edx_toggles.toggles import LegacyWaffleSwitch from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from common.djangoapps.util.testing import EventTestMixin -from lms.djangoapps.certificates.generation import generate_allowlist_certificate +from lms.djangoapps.certificates.generation import generate_course_certificate from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from openedx.core.djangoapps.certificates.config import waffle from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -22,17 +22,17 @@ AUTO_GENERATION_SWITCH_NAME = f'{AUTO_GENERATION_NAMESPACE}.{AUTO_GENERATION_NAM AUTO_GENERATION_SWITCH = LegacyWaffleSwitch(AUTO_GENERATION_NAMESPACE, AUTO_GENERATION_NAME) -class AllowlistTests(EventTestMixin, ModuleStoreTestCase): +class CertificateTests(EventTestMixin, ModuleStoreTestCase): """ - Tests for generating allowlist certificates + Tests for certificate generation """ def setUp(self): # pylint: disable=arguments-differ super().setUp('lms.djangoapps.certificates.utils.tracker') - def test_allowlist_generation(self): + def test_generation(self): """ - Test allowlist certificate generation + Test certificate generation """ # Create user, a course run, and an enrollment u = UserFactory() @@ -44,11 +44,12 @@ class AllowlistTests(EventTestMixin, ModuleStoreTestCase): is_active=True, mode="verified", ) + gen_mode = 'batch' certs = GeneratedCertificate.objects.filter(user=u, course_id=key) assert len(certs) == 0 - generated_cert = generate_allowlist_certificate(u, key) + generated_cert = generate_course_certificate(u, key, gen_mode) assert generated_cert.status, CertificateStatuses.downloadable certs = GeneratedCertificate.objects.filter(user=u, course_id=key) @@ -61,5 +62,5 @@ class AllowlistTests(EventTestMixin, ModuleStoreTestCase): certificate_id=generated_cert.verify_uuid, enrollment_mode=generated_cert.mode, certificate_url='', - generation_mode='batch' + generation_mode=gen_mode ) diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 3a9965e066..6886b691c8 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -302,6 +302,14 @@ class CertificateTests(ModuleStoreTestCase): assert _can_generate_v2_certificate(self.user, self.course_run_key) assert can_generate_certificate_task(self.user, self.course_run_key) assert generate_certificate_task(self.user, self.course_run_key) + + def test_handle_valid_task(self): + """ + Test handling of a valid user/course run combo. + + We test generate_certificate_task() and generate_regular_certificate_task() separately since they both + generate a cert. + """ assert generate_regular_certificate_task(self.user, self.course_run_key) @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False) diff --git a/lms/djangoapps/certificates/tests/test_tasks.py b/lms/djangoapps/certificates/tests/test_tasks.py index 4cb3c56cf5..12d1468735 100644 --- a/lms/djangoapps/certificates/tests/test_tasks.py +++ b/lms/djangoapps/certificates/tests/test_tasks.py @@ -1,8 +1,9 @@ """ -Test module for user certificate generation. +Tests for course certificate tasks. """ +from unittest import mock from unittest.mock import call, patch import ddt @@ -17,8 +18,12 @@ from lms.djangoapps.verify_student.models import IDVerificationAttempt @ddt.ddt class GenerateUserCertificateTest(TestCase): """ - Tests for course certificates + Tests for course certificate tasks """ + def setUp(self): + super().setUp() + + self.user = UserFactory() @patch('lms.djangoapps.certificates.tasks.generate_user_certificates') @patch('lms.djangoapps.certificates.tasks.User.objects.get') @@ -78,3 +83,51 @@ class GenerateUserCertificateTest(TestCase): student=student, course_key=CourseKey.from_string(course_key) ) + + def test_generation(self): + """ + Verify the task handles V2 certificate generation + """ + course_key = 'course-v1:edX+DemoX+Demo_Course' + + with mock.patch( + 'lms.djangoapps.certificates.tasks.generate_course_certificate', + return_value=None + ) as mock_generate_cert: + kwargs = { + 'student': self.user.id, + 'course_key': course_key, + 'v2_certificate': True + } + + generate_certificate.apply_async(kwargs=kwargs) + mock_generate_cert.assert_called_with( + user=self.user, + course_key=CourseKey.from_string(course_key), + generation_mode='batch' + ) + + def test_generation_mode(self): + """ + Verify the task handles V2 certificate generation with a generation mode + """ + course_key = 'course-v1:edX+DemoX+Demo_Course' + gen_mode = 'self' + + with mock.patch( + 'lms.djangoapps.certificates.tasks.generate_course_certificate', + return_value=None + ) as mock_generate_cert: + kwargs = { + 'student': self.user.id, + 'course_key': course_key, + 'v2_certificate': True, + 'generation_mode': gen_mode + } + + generate_certificate.apply_async(kwargs=kwargs) + mock_generate_cert.assert_called_with( + user=self.user, + course_key=CourseKey.from_string(course_key), + generation_mode=gen_mode + ) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index b8d0ed0005..f164660228 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1583,7 +1583,7 @@ def generate_user_cert(request, course_id): if certs_api.can_generate_certificate_task(student, course_key): log.info(f'{course_key} is using V2 certificates. Attempt will be made to generate a V2 certificate for ' f'user {student.id}.') - certs_api.generate_certificate_task(student, course_key) + certs_api.generate_certificate_task(student, course_key, 'self') return HttpResponse() if not is_course_passed(student, course):