diff --git a/lms/djangoapps/certificates/signals.py b/lms/djangoapps/certificates/signals.py index 5196fe39a20a..2039c4e47032 100644 --- a/lms/djangoapps/certificates/signals.py +++ b/lms/djangoapps/certificates/signals.py @@ -8,11 +8,9 @@ from django.dispatch import receiver 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 common.djangoapps.student.signals import ENROLLMENT_TRACK_UPDATED from lms.djangoapps.certificates.generation_handler import ( - can_generate_certificate_task, generate_allowlist_certificate_task, generate_certificate_task, is_on_certificate_allowlist @@ -23,8 +21,6 @@ CertificateStatuses, GeneratedCertificate ) -from lms.djangoapps.certificates.tasks import CERTIFICATE_DELAY_SECONDS, generate_certificate -from lms.djangoapps.grades.api import CourseGradeFactory from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.certificates.api import auto_certificate_generation_enabled from openedx.core.djangoapps.content.course_overviews.signals import COURSE_PACING_CHANGED @@ -65,12 +61,6 @@ def _listen_for_certificate_allowlist_append(sender, instance, **kwargs): # pyl f'made to generate an allowlist certificate.') return generate_allowlist_certificate_task(instance.user, instance.course_id) - if _fire_ungenerated_certificate_task(instance.user, instance.course_id): - log.info('Certificate generation task initiated for {user} : {course} via allowlist'.format( - user=instance.user.id, - course=instance.course_id - )) - @receiver(COURSE_GRADE_NOW_PASSED, dispatch_uid="new_passing_learner") def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disable=unused-argument @@ -82,21 +72,14 @@ def listen_for_passing_grade(sender, user, course_id, **kwargs): # pylint: disa if not auto_certificate_generation_enabled(): return - if can_generate_certificate_task(user, course_id): - cert = GeneratedCertificate.certificate_for_student(user, course_id) - if cert is not None and CertificateStatuses.is_passing_status(cert.status): - log.info(f'{course_id} is using V2 certificates, and the cert status is already passing for user ' - f'{user.id}. Passing grade signal will be ignored.') - return - 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 _fire_ungenerated_certificate_task(user, course_id): - log.info('Certificate generation task initiated for {user} : {course} via passing grade'.format( - user=user.id, - course=course_id - )) + cert = GeneratedCertificate.certificate_for_student(user, course_id) + if cert is not None and CertificateStatuses.is_passing_status(cert.status): + log.info(f'The cert status is already passing for user {user.id} : {course_id}. Passing grade signal will be ' + f'ignored.') + return + log.info(f'Attempt will be made to generate a course certificate for {user.id} : {course_id} as a passing grade ' + f'was received.') + return generate_certificate_task(user, course_id) @receiver(COURSE_GRADE_NOW_FAILED, dispatch_uid="new_failing_learner") @@ -126,33 +109,18 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli def _listen_for_id_verification_status_changed(sender, user, **kwargs): # pylint: disable=unused-argument """ Listen for a signal indicating that the user's id verification status has changed. - - If needed, generate a certificate task. """ if not auto_certificate_generation_enabled(): return user_enrollments = CourseEnrollment.enrollments_for_user(user=user) - - grade_factory = CourseGradeFactory() expected_verification_status = IDVerificationService.user_status(user) expected_verification_status = expected_verification_status['status'] + for enrollment in user_enrollments: - if can_generate_certificate_task(user, enrollment.course_id): - log.info(f'{enrollment.course_id} is using V2 certificates. Attempt will be made to generate a V2 ' - f'certificate for {user.id}. Id verification status is {expected_verification_status}') - generate_certificate_task(user, enrollment.course_id) - elif grade_factory.read(user=user, course=enrollment.course_overview).passed: - if _fire_ungenerated_certificate_task(user, enrollment.course_id, expected_verification_status): - message = ( - 'Certificate generation task initiated for {user} : {course} via track change ' + - 'with verification status of {status}' - ) - log.info(message.format( - user=user.id, - course=enrollment.course_id, - status=expected_verification_status - )) + log.info(f'Attempt will be made to generate a course certificate for {user.id} : {enrollment.course_id}. Id ' + f'verification status is {expected_verification_status}') + generate_certificate_task(user, enrollment.course_id) @receiver(ENROLLMENT_TRACK_UPDATED) @@ -164,63 +132,6 @@ def _listen_for_enrollment_mode_change(sender, user, course_key, mode, **kwargs) if the user has moved to the audit track. """ if modes_api.is_eligible_for_certificate(mode): - if can_generate_certificate_task(user, course_key): - log.info(f'{course_key} is using V2 certificates. Attempt will be made to generate a V2 certificate for ' - f'{user.id} since the enrollment mode is now {mode}.') - generate_certificate_task(user, course_key) - - -def _fire_ungenerated_certificate_task(user, course_key, expected_verification_status=None): - """ - Helper function to fire certificate generation task. - Auto-generation of certificates is available for following course modes: - 1- VERIFIED - 2- CREDIT_MODE - 3- PROFESSIONAL - 4- NO_ID_PROFESSIONAL_MODE - - Certificate generation task is fired to either generate a certificate - when there is no generated certificate for user in a particular course or - update a certificate if it has 'unverified' status. - - Task is fired to attempt an update to a certificate - with 'unverified' status as this method is called when a user is - successfully verified, any certificate associated - with such user can now be verified. - - NOTE: Purpose of restricting other course modes (HONOR and AUDIT) from auto-generation is to reduce - traffic to workers. - """ - - message = 'Entered into Ungenerated Certificate task for {user} : {course}' - log.info(message.format(user=user.id, course=course_key)) - - allowed_enrollment_modes_list = [ - CourseMode.VERIFIED, - CourseMode.CREDIT_MODE, - CourseMode.PROFESSIONAL, - CourseMode.NO_ID_PROFESSIONAL_MODE, - CourseMode.MASTERS, - CourseMode.EXECUTIVE_EDUCATION, - ] - enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(user, course_key) - cert = GeneratedCertificate.certificate_for_student(user, course_key) - - generate_learner_certificate = ( - enrollment_mode in allowed_enrollment_modes_list and ( - cert is None or cert.status == CertificateStatuses.unverified) - ) - - if generate_learner_certificate: - kwargs = { - 'student': str(user.id), - 'course_key': str(course_key) - } - if expected_verification_status: - kwargs['expected_verification_status'] = str(expected_verification_status) - generate_certificate.apply_async(countdown=CERTIFICATE_DELAY_SECONDS, kwargs=kwargs) - return True - - message = 'Certificate Generation task failed for {user} : {course}' - log.info(message.format(user=user.id, course=course_key)) - return False + log.info(f'Attempt will be made to generate a course certificate for {user.id} : {course_key} since the ' + f'enrollment mode is now {mode}.') + generate_certificate_task(user, course_key) diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 491cb1725066..acd676d1e4f4 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -5,12 +5,10 @@ from unittest import mock import ddt -from edx_toggles.toggles.testutils import override_waffle_flag from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.generation_handler import ( - CERTIFICATES_USE_UPDATED, _can_generate_allowlist_certificate, _can_generate_certificate_for_status, _can_generate_v2_certificate, @@ -271,7 +269,6 @@ def test_cert_status_downloadable(self): assert _set_allowlist_cert_status(u, key) is None -@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)) @mock.patch(PASSING_GRADE_METHOD, mock.Mock(return_value=True)) @@ -313,7 +310,6 @@ def test_handle_valid_task(self): """ assert generate_regular_certificate_task(self.user, self.course_run_key) is True - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False) def test_handle_invalid(self): """ Test handling of an invalid user/course run combo diff --git a/lms/djangoapps/certificates/tests/test_signals.py b/lms/djangoapps/certificates/tests/test_signals.py index 8cd1c8bdac2b..1699d57a927a 100644 --- a/lms/djangoapps/certificates/tests/test_signals.py +++ b/lms/djangoapps/certificates/tests/test_signals.py @@ -12,13 +12,11 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from lms.djangoapps.certificates.api import cert_generation_enabled -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_UPDATED from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, GeneratedCertificate ) -from lms.djangoapps.certificates.signals import _fire_ungenerated_certificate_task from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory, GeneratedCertificateFactory from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.tests.utils import mock_passing_grade @@ -79,40 +77,30 @@ def test_fire_task_allowlist_auto_enabled(self): Test that the allowlist generation is invoked if automatic generation is enabled """ with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', + 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task', return_value=None - ) as mock_generate_certificate_apply_async: - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task', - return_value=None - ) as mock_generate_allowlist_task: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): - CertificateAllowlistFactory( - user=self.user, - course_id=self.ip_course.id - ) - mock_generate_certificate_apply_async.assert_not_called() - mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id) + ) as mock_generate_allowlist_task: + with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): + CertificateAllowlistFactory( + user=self.user, + course_id=self.ip_course.id + ) + mock_generate_allowlist_task.assert_called_with(self.user, self.ip_course.id) def test_fire_task_allowlist_auto_disabled(self): """ Test that the allowlist generation is not invoked if automatic generation is disabled """ with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', + 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task', return_value=None - ) as mock_generate_certificate_apply_async: - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_allowlist_certificate_task', - return_value=None - ) as mock_generate_allowlist_task: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False): - CertificateAllowlistFactory( - user=self.user, - course_id=self.ip_course.id - ) - mock_generate_certificate_apply_async.assert_not_called() - mock_generate_allowlist_task.assert_not_called() + ) as mock_generate_allowlist_task: + with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=False): + CertificateAllowlistFactory( + user=self.user, + course_id=self.ip_course.id + ) + mock_generate_allowlist_task.assert_not_called() class PassingGradeCertsTest(ModuleStoreTestCase): @@ -146,23 +134,6 @@ def setUp(self): ) attempt.approve() - def test_cert_already_generated(self): - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - grade_factory = CourseGradeFactory() - # Create the certificate - GeneratedCertificateFactory( - user=self.user, - course_id=self.course.id, - status=CertificateStatuses.downloadable - ) - # Certs are not re-fired after passing - with mock_passing_grade(): - grade_factory.update(self.user, self.course) - mock_generate_certificate_apply_async.assert_not_called() - def test_passing_grade_allowlist(self): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): # User who is not on the allowlist @@ -200,7 +171,6 @@ def test_passing_grade_allowlist(self): CourseGradeFactory().update(u, c) mock_cert_task.assert_called_with(u, course_key) - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) def test_cert_already_generated_downloadable(self): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): GeneratedCertificateFactory( @@ -218,7 +188,6 @@ def test_cert_already_generated_downloadable(self): grade_factory.update(self.user, self.course) mock_cert_task.assert_not_called() - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) def test_cert_already_generated_unverified(self): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): GeneratedCertificateFactory( @@ -236,7 +205,6 @@ def test_cert_already_generated_unverified(self): grade_factory.update(self.user, self.course) mock_cert_task.assert_called_with(self.user, self.course_key) - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) def test_without_cert(self): with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): with mock.patch( @@ -362,7 +330,6 @@ def setUp(self): grade_factory.update(self.user_one, self.course_one) grade_factory.update(self.user_two, self.course_two) - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) def test_cert_generation_on_photo_verification(self): with mock.patch( 'lms.djangoapps.certificates.signals.generate_certificate_task', @@ -417,48 +384,6 @@ def test_id_verification_allowlist(self): mock_allowlist_task.assert_called_with(u, course_key) -@ddt.ddt -class CertificateGenerationTaskTest(ModuleStoreTestCase): - """ - Tests for certificate generation task. - """ - - def setUp(self): - super().setUp() - self.course = CourseFactory.create() - - @ddt.data( - ('professional', True), - ('verified', True), - ('no-id-professional', True), - ('credit', True), - ('masters', True), - ('audit', False), - ('honor', False), - ) - @ddt.unpack - def test_fire_ungenerated_certificate_task_allowed_modes(self, enrollment_mode, should_create): - """ - Test that certificate generation task is fired for only modes that are - allowed to generate certificates automatically. - """ - self.user = UserFactory.create() - CourseEnrollmentFactory( - user=self.user, - course_id=self.course.id, - is_active=True, - mode=enrollment_mode - ) - with mock.patch( - 'lms.djangoapps.certificates.signals.generate_certificate.apply_async', - return_value=None - ) as mock_generate_certificate_apply_async: - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True): - _fire_ungenerated_certificate_task(self.user, self.course.id) - task_created = mock_generate_certificate_apply_async.called - assert task_created == should_create - - @override_waffle_flag(AUTO_CERTIFICATE_GENERATION_SWITCH, active=True) class EnrollmentModeChangeCertsTest(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/certificates/views/xqueue.py b/lms/djangoapps/certificates/views/xqueue.py index ff99759ab1cb..78b6eed276c0 100644 --- a/lms/djangoapps/certificates/views/xqueue.py +++ b/lms/djangoapps/certificates/views/xqueue.py @@ -15,13 +15,8 @@ from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest from common.djangoapps.util.request_rate_limiter import BadRequestRateLimiter -from lms.djangoapps.certificates.api import ( - can_generate_certificate_task, - generate_certificate_task, - generate_user_certificates -) +from lms.djangoapps.certificates.api import generate_certificate_task from lms.djangoapps.certificates.models import ( - CertificateStatuses, ExampleCertificate, GeneratedCertificate, certificate_status_for_student @@ -49,14 +44,9 @@ def request_certificate(request): course_key = CourseKey.from_string(request.POST.get('course_id')) status = certificate_status_for_student(student, course_key)['status'] - if can_generate_certificate_task(student, course_key): - log.info(f'{course_key} is using V2 course certificates. Attempt will be made to generate a V2 ' - f'certificate for user {student.id}.') - generate_certificate_task(student, course_key) - elif status in [CertificateStatuses.unavailable, CertificateStatuses.notpassing, CertificateStatuses.error]: - log_msg = 'Grading and certification requested for user %s in course %s via /request_certificate call' - log.info(log_msg, username, course_key) - status = generate_user_certificates(student, course_key) + log.info(f'{course_key} is using V2 course certificates. Attempt will be made to generate a V2 certificate ' + f'for user {student.id}.') + generate_certificate_task(student, course_key) return HttpResponse(json.dumps({'add_status': status}), content_type='application/json') # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps return HttpResponse(json.dumps({'add_status': 'ERRORANONYMOUSUSER'}), content_type='application/json') # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps @@ -70,7 +60,6 @@ def update_certificate(request): This view should only ever be accessed by the xqueue server """ - status = CertificateStatuses if request.method == "POST": xqueue_body = json.loads(request.POST.get('xqueue_body')) @@ -99,55 +88,15 @@ def update_certificate(request): }), content_type='application/json') user = cert.user - if can_generate_certificate_task(user, course_key): - log.warning(f'{course_key} is using V2 certificates. Request to update the certificate for user {user.id} ' - f'will be ignored.') - return HttpResponse( # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps - json.dumps({ - 'return_code': 1, - 'content': 'allowlist certificate' - }), - content_type='application/json' - ) - - if 'error' in xqueue_body: - cert.status = status.error - if 'error_reason' in xqueue_body: - - # Hopefully we will record a meaningful error - # here if something bad happened during the - # certificate generation process - # - # example: - # (aamorm BerkeleyX/CS169.1x/2012_Fall) - # : - # HTTP error (reason=error(32, 'Broken pipe'), filename=None) : - # certificate_agent.py:175 - - cert.error_reason = xqueue_body['error_reason'] - else: - if cert.status == status.generating: - cert.download_uuid = xqueue_body['download_uuid'] - cert.verify_uuid = xqueue_body['verify_uuid'] - cert.download_url = xqueue_body['url'] - cert.status = status.downloadable - elif cert.status in [status.deleting]: - cert.status = status.deleted - else: - log.critical( - 'Invalid state for cert update: %s', cert.status - ) - return HttpResponse( # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps - json.dumps({ - 'return_code': 1, - 'content': 'invalid cert status' - }), - content_type='application/json' - ) - - cert.save() - return HttpResponse(json.dumps({'return_code': 0}), # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps - content_type='application/json') + log.warning(f'{course_key} is using V2 certificates. Request to update the certificate for user {user.id} will ' + f'be ignored.') + return HttpResponse( # pylint: disable=http-response-with-content-type-json, http-response-with-json-dumps + json.dumps({ + 'return_code': 1, + 'content': 'allowlist certificate' + }), + content_type='application/json' + ) @csrf_exempt