From 4aff07ad82ecc2d4001f57393b1c46261ca021bb Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Tue, 9 Mar 2021 11:02:19 -0500 Subject: [PATCH] MICROBA-923 Add method to generate V2 course certificates (#26898) --- .../certificates/generation_handler.py | 140 ++++++++++++++---- lms/djangoapps/certificates/models.py | 45 +----- lms/djangoapps/certificates/queue.py | 3 +- lms/djangoapps/certificates/tasks.py | 11 +- .../tests/test_generation_handler.py | 89 +++++++++-- lms/djangoapps/certificates/views/xqueue.py | 2 - 6 files changed, 200 insertions(+), 90 deletions(-) diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index 137bb97408..da047a59be 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -33,11 +33,11 @@ WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='certificates_revamp') # .. toggle_name: certificates_revamp.use_allowlist # .. toggle_implementation: CourseWaffleFlag # .. toggle_default: False -# .. toggle_description: Waffle flag enable the course certificates allowlist (aka V2 of the certificate whitelist) on -# a per-course run basis. +# .. toggle_description: Waffle flag to enable the course certificates allowlist (aka V2 of the certificate whitelist) +# on a per-course run basis. # .. toggle_use_cases: temporary # .. toggle_creation_date: 2021-01-27 -# .. toggle_target_removal_date: 2022-01-027 +# .. toggle_target_removal_date: 2022-01-27 # .. toggle_tickets: MICROBA-918 CERTIFICATES_USE_ALLOWLIST = CourseWaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, @@ -46,17 +46,69 @@ CERTIFICATES_USE_ALLOWLIST = CourseWaffleFlag( ) +# .. toggle_name: certificates_revamp.use_updated +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable the updated regular (non-allowlist) course certificate logic on a +# per-course run basis. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2021-03-05 +# .. toggle_target_removal_date: 2022-03-05 +# .. toggle_tickets: MICROBA-923 +CERTIFICATES_USE_UPDATED = CourseWaffleFlag( + waffle_namespace=WAFFLE_FLAG_NAMESPACE, + flag_name='use_updated', + module_name=__name__, +) + + +def can_generate_certificate_task(user, course_key): + """ + Determine if we can create a task to generate a certificate for this user in this course run. + + This will return True if either: + - the course run is using the allowlist and the user is on the allowlist, or + - the course run is using v2 course certificates + """ + if is_using_certificate_allowlist_and_is_on_allowlist(user, course_key): + return True + elif _is_using_v2_course_certificates(course_key): + return True + + return False + + +def generate_certificate_task(user, course_key): + """ + 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. + """ + 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) + + 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) + + 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): """ Create a task to generate an allowlist certificate for this user in this course run. """ - if not can_generate_allowlist_certificate(user, course_key): - log.info( - f'Cannot generate an allowlist certificate for {user.id} : {course_key}') + if not _can_generate_allowlist_certificate(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}') + log.info(f'About to create an allowlist certificate task for {user.id} : {course_key}') kwargs = { 'student': str(user.id), @@ -67,17 +119,34 @@ def generate_allowlist_certificate_task(user, course_key): return True -def can_generate_allowlist_certificate(user, course_key): +def generate_regular_certificate_task(user, course_key): + """ + 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 not _can_generate_v2_certificate(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}') + + kwargs = { + 'student': str(user.id), + 'course_key': str(course_key), + 'v2_certificate': True + } + generate_certificate.apply_async(countdown=CERTIFICATE_DELAY_SECONDS, kwargs=kwargs) + return True + + +def _can_generate_allowlist_certificate(user, course_key): """ 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. """ if not is_using_certificate_allowlist(course_key): # This course run is not using the allowlist feature - log.info( - '{course} is not using the certificate allowlist. Certificate cannot be generated.'.format( - course=course_key - )) + log.info(f'{course_key} is not using the certificate allowlist. Certificate cannot be generated.') return False if not auto_certificate_generation_enabled(): @@ -87,19 +156,12 @@ def can_generate_allowlist_certificate(user, course_key): if CertificateInvalidation.has_certificate_invalidation(user, course_key): # The invalidation list overrides the allowlist - log.info( - '{user} : {course} is on the certificate invalidation list. Certificate cannot be generated.'.format( - user=user.id, - course=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('{user} : {course} does not have an enrollment. Certificate cannot be generated.'.format( - user=user.id, - course=course_key - )) + log.info(f'{user.id} : {course_key} does not have an enrollment. Certificate cannot be generated.') return False if not IDVerificationService.user_is_verified(user): @@ -107,20 +169,29 @@ def can_generate_allowlist_certificate(user, course_key): return False if not _is_on_certificate_allowlist(user, course_key): - log.info('{user} : {course} is not on the certificate allowlist. Certificate cannot be generated.'.format( - user=user.id, - course=course_key - )) + log.info(f'{user.id} : {course_key} is not on the certificate allowlist. Certificate cannot be generated.') return False - log.info('{user} : {course} is on the certificate allowlist'.format( - user=user.id, - course=course_key - )) + log.info(f'{user.id} : {course_key} is on the certificate allowlist') cert = GeneratedCertificate.certificate_for_student(user, course_key) return _can_generate_allowlist_certificate_for_status(cert) +def _can_generate_v2_certificate(user, course_key): + """ + Check if a v2 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. + """ + if not _is_using_v2_course_certificates(course_key): + # This course run is not using the v2 course certificate feature + log.info(f'{course_key} is not using v2 course certificates. Certificate cannot be generated.') + return False + + # TODO: Further implementation will be added in MICROBA-923 + log.warning(f'Ignoring check on V2 course certificates for {user.id}: {course_key}') + return False + + def is_using_certificate_allowlist_and_is_on_allowlist(user, course_key): """ Return True if both: @@ -137,9 +208,16 @@ def is_using_certificate_allowlist(course_key): return CERTIFICATES_USE_ALLOWLIST.is_enabled(course_key) +def _is_using_v2_course_certificates(course_key): + """ + Return True if the course run is using v2 course certificates + """ + return CERTIFICATES_USE_UPDATED.is_enabled(course_key) + + def _is_on_certificate_allowlist(user, course_key): """ - Check if the user is on the allowlist for this course run + Check if the user is on the allowlist, and is enabled for the allowlist, for this course run """ return CertificateWhitelist.objects.filter(user=user, course_id=course_key, whitelist=True).exists() diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 2e3e655303..5822573b28 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -1,48 +1,5 @@ """ -Certificates are created for a student and an offering of a course. - -When a certificate is generated, a unique ID is generated so that -the certificate can be verified later. The ID is a UUID4, so that -it can't be easily guessed and so that it is unique. - -Certificates are generated in batches by a cron job, when a -certificate is available for download the GeneratedCertificate -table is updated with information that will be displayed -on the course overview page. - - -State diagram: - -[deleted,error,unavailable] [error,downloadable] - + + + - | | | - | | | - add_cert regen_cert del_cert - | | | - v v v - [generating] [regenerating] [deleting] - + + + - | | | - certificate certificate certificate - created removed,created deleted - +----------------+-------------+------->[error] - | | | - | | | - v v v - [downloadable] [downloadable] [deleted] - - -Eligibility: - - Students are eligible for a certificate if they pass the course - with the following exceptions: - - If the student has allow_certificate set to False in the student profile - he will never be issued a certificate. - - If the user and course is present in the certificate whitelist table - then the student will be issued a certificate regardless of his grade, - unless he has allow_certificate set to False. +Course certificates are created for a student and an offering of a course (a course run). """ diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index c1b1480198..31060569b9 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -59,8 +59,7 @@ class XQueueCertInterface: Instantiating an object will create a new connection to the queue server. - See models.py for valid state transitions, - summary of methods: + Summary of methods: add_cert: Add a new certificate. Puts a single request on the queue for the student/course. diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index d9e4def184..bbcba0b5d5 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/lms/djangoapps/certificates/tasks.py @@ -13,7 +13,7 @@ from opaque_keys.edx.keys import CourseKey from lms.djangoapps.certificates.generation import generate_allowlist_certificate, generate_user_certificates from lms.djangoapps.verify_student.services import IDVerificationService -logger = getLogger(__name__) +log = getLogger(__name__) User = get_user_model() CERTIFICATE_DELAY_SECONDS = 2 @@ -35,22 +35,29 @@ def generate_certificate(self, **kwargs): 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 """ 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 + if v2_certificate: + # TODO: will be implemented in MICROBA-923 + log.warning(f'Ignoring v2 certificate task request for {student.id}: {course_key}') + return + if expected_verification_status: actual_verification_status = IDVerificationService.user_status(student) actual_verification_status = actual_verification_status['status'] if expected_verification_status != actual_verification_status: - logger.warning( + log.warning( 'Expected verification status {expected} ' 'differs from actual verification status {actual} ' 'for user {user} in course {course}'.format( diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index a5b8cf06b1..6ff7097e4c 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -13,12 +13,17 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_ALLOWLIST +from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_ALLOWLIST, CERTIFICATES_USE_UPDATED from lms.djangoapps.certificates.generation_handler import ( _can_generate_allowlist_certificate_for_status, is_using_certificate_allowlist, - can_generate_allowlist_certificate, + _is_using_v2_course_certificates, + _can_generate_allowlist_certificate, + _can_generate_v2_certificate, + can_generate_certificate_task, generate_allowlist_certificate_task, + generate_certificate_task, + generate_regular_certificate_task, is_using_certificate_allowlist_and_is_on_allowlist ) from lms.djangoapps.certificates.models import GeneratedCertificate, CertificateStatuses @@ -147,29 +152,38 @@ class AllowlistTests(ModuleStoreTestCase): """ Test handling of an invalid user/course run combo """ - assert not can_generate_allowlist_certificate(self.user, self.course_run_key) + assert not _can_generate_allowlist_certificate(self.user, self.course_run_key) assert not generate_allowlist_certificate_task(self.user, self.course_run_key) + assert not can_generate_certificate_task(self.user, self.course_run_key) + assert not generate_certificate_task(self.user, self.course_run_key) 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) assert generate_allowlist_certificate_task(self.user, self.course_run_key) + def test_handle_valid_general_methods(self): + """ + Test handling of a valid user/course run combo for the general (non-allowlist) generation methods + """ + 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) + 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 """ with mock.patch(ID_VERIFIED_METHOD, return_value=False): - assert not can_generate_allowlist_certificate(self.user, self.course_run_key) + assert not _can_generate_allowlist_certificate(self.user, self.course_run_key) def test_can_generate_not_enrolled(self): """ @@ -179,7 +193,7 @@ class AllowlistTests(ModuleStoreTestCase): cr = CourseFactory() key = cr.id # pylint: disable=no-member CertificateWhitelistFactory.create(course_id=key, user=u) - assert not can_generate_allowlist_certificate(u, key) + assert not _can_generate_allowlist_certificate(u, key) def test_can_generate_not_whitelisted(self): """ @@ -194,7 +208,7 @@ class AllowlistTests(ModuleStoreTestCase): is_active=True, mode="verified", ) - assert not can_generate_allowlist_certificate(u, key) + assert not _can_generate_allowlist_certificate(u, key) def test_can_generate_invalidated(self): """ @@ -222,4 +236,61 @@ class AllowlistTests(ModuleStoreTestCase): active=True ) - assert not can_generate_allowlist_certificate(u, key) + assert not _can_generate_allowlist_certificate(u, 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)) +class CertificateTests(ModuleStoreTestCase): + """ + Tests for handling course certificates + """ + + def setUp(self): + super().setUp() + + # Create user, a course run, and an enrollment + 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="verified", + ) + + def test_handle_valid(self): + """ + Test handling of a valid user/course run combo. + + Note: these assertions are placeholders for now. They will be updated as the implementation is added. + """ + assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert can_generate_certificate_task(self.user, self.course_run_key) + assert not generate_certificate_task(self.user, self.course_run_key) + assert not generate_regular_certificate_task(self.user, self.course_run_key) + + @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False) + def test_handle_invalid(self): + """ + Test handling of an invalid user/course run combo + """ + assert not _can_generate_v2_certificate(self.user, self.course_run_key) + assert not can_generate_certificate_task(self.user, self.course_run_key) + assert not generate_certificate_task(self.user, self.course_run_key) + assert not generate_regular_certificate_task(self.user, self.course_run_key) + + def test_is_using_updated_true(self): + """ + Test the updated flag + """ + assert _is_using_v2_course_certificates(self.course_run_key) + + @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False) + def test_is_using_updated_false(self): + """ + Test the updated flag without the override + """ + assert not _is_using_v2_course_certificates(self.course_run_key) diff --git a/lms/djangoapps/certificates/views/xqueue.py b/lms/djangoapps/certificates/views/xqueue.py index 07ac8f30d4..4432e10214 100644 --- a/lms/djangoapps/certificates/views/xqueue.py +++ b/lms/djangoapps/certificates/views/xqueue.py @@ -69,8 +69,6 @@ def update_certificate(request): Will update GeneratedCertificate for a new certificate or modify an existing certificate entry. - See models.py for a state diagram of certificate states - This view should only ever be accessed by the xqueue server """