fix: Remove V2 certificate checks from the certificates app, since V2 of course certificates has been enabled globally for all course runs (#28102)

MICROBA-1083 DEPR-155

lms/djangoapps/certificates/signals.py

@@ -8,11 +8,9 @@ from django.db.models.signals import post_save
 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 @@ from lms.djangoapps.certificates.models import (
     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)

lms/djangoapps/certificates/tests/test_generation_handler.py

@@ -5,12 +5,10 @@ import logging
 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 @@ class AllowlistTests(ModuleStoreTestCase):
         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 @@ class CertificateTests(ModuleStoreTestCase):
         """
         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

lms/djangoapps/certificates/tests/test_signals.py

@@ -12,13 +12,11 @@ from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_
 
 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 @@ class AllowlistGeneratedCertificatesTest(ModuleStoreTestCase):
         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 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
         )
         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 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
                     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 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
                     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 @@ class PassingGradeCertsTest(ModuleStoreTestCase):
                     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 @@ class LearnerIdVerificationTest(ModuleStoreTestCase):
             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 @@ class LearnerIdVerificationTest(ModuleStoreTestCase):
                 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):
     """

lms/djangoapps/certificates/views/xqueue.py

@@ -15,13 +15,8 @@ from opaque_keys.edx.keys import CourseKey
 
 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)
-                #  <class 'simples3.bucket.S3Error'>:
-                #  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