From 1b0b70314444d6ca147d33f59829687b9b9dde95 Mon Sep 17 00:00:00 2001 From: Christie Rice <8483753+crice100@users.noreply.github.com> Date: Tue, 13 Jul 2021 10:30:07 -0400 Subject: [PATCH] fix: Remove disabled v1 course certificate task code, as this was previously globally disabled (#28158) MICROBA-1227 DEPR-155 --- lms/djangoapps/certificates/generation.py | 74 +------------------ lms/djangoapps/certificates/tasks.py | 42 ++--------- .../certificates/tests/test_tasks.py | 66 ++--------------- 3 files changed, 14 insertions(+), 168 deletions(-) diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index eeed571b82..d3ce2f2b25 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -15,14 +15,8 @@ from uuid import uuid4 from common.djangoapps.student.models import CourseEnrollment, UserProfile from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import GeneratedCertificate -from lms.djangoapps.certificates.queue import XQueueCertInterface -from lms.djangoapps.certificates.utils import ( - emit_certificate_event, - has_html_certificates_enabled -) +from lms.djangoapps.certificates.utils import emit_certificate_event from lms.djangoapps.grades.api import CourseGradeFactory -from lms.djangoapps.instructor.access import list_with_level -from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none log = logging.getLogger(__name__) @@ -104,69 +98,3 @@ def _generate_certificate(user, course_key, status): created_msg = 'Certificate already existed and was updated.' log.info(f'Generated certificate with status {cert.status} for {user.id} : {course_key}. {created_msg}') return cert - - -def generate_user_certificates(student, course_key, insecure=False, generation_mode='batch', forced_grade=None): - """ - It will add the add-cert request into the xqueue. - - A new record will be created to track the certificate - generation task. If an error occurs while adding the certificate - to the queue, the task will have status 'error'. It also emits - `edx.certificate.created` event for analytics. - - This method has not yet been updated (it predates the certificates revamp). If modifying this method, - see also generate_user_certificates() in generation_handler.py (which is very similar but is not called from a - celery task). In the future these methods will be unified. - - Args: - student (User) - course_key (CourseKey) - - Keyword Arguments: - insecure - (Boolean) - generation_mode - who has requested certificate generation. Its value should `batch` - in case of django command and `self` if student initiated the request. - forced_grade - a string indicating to replace grade parameter. if present grading - will be skipped. - """ - beta_testers_queryset = list_with_level(course_key, 'beta') - if beta_testers_queryset.filter(username=student.username): - log.info(f"Canceling Certificate Generation task for user {student.id} : {course_key}. User is a Beta Tester.") - return - - xqueue = XQueueCertInterface() - if insecure: - xqueue.use_https = False - - course_overview = get_course_overview_or_none(course_key) - if not course_overview: - log.info(f"Canceling Certificate Generation task for user {student.id} : {course_key} due to a missing course" - f"overview.") - return - - generate_pdf = not has_html_certificates_enabled(course_overview) - - cert = xqueue.add_cert( - student, - course_key, - generate_pdf=generate_pdf, - forced_grade=forced_grade - ) - - log.info(f"Queued Certificate Generation task for {student.id} : {course_key}") - - # If cert_status is not present in certificate valid_statuses (for example unverified) then - # add_cert returns None and raises AttributeError while accessing cert attributes. - if cert is None: - return - - if CertificateStatuses.is_passing_status(cert.status): - emit_certificate_event('created', student, course_key, course_overview, { - 'user_id': student.id, - 'course_id': str(course_key), - 'certificate_id': cert.verify_uuid, - 'enrollment_mode': cert.mode, - 'generation_mode': generation_mode - }) - return cert.status diff --git a/lms/djangoapps/certificates/tasks.py b/lms/djangoapps/certificates/tasks.py index a7f7b8347c..7e93c93024 100644 --- a/lms/djangoapps/certificates/tasks.py +++ b/lms/djangoapps/certificates/tasks.py @@ -11,11 +11,7 @@ from edx_django_utils.monitoring import set_code_owner_attribute from opaque_keys.edx.keys import CourseKey from lms.djangoapps.certificates.data import CertificateStatuses -from lms.djangoapps.certificates.generation import ( - generate_course_certificate, - generate_user_certificates -) -from lms.djangoapps.verify_student.services import IDVerificationService +from lms.djangoapps.certificates.generation import generate_course_certificate log = getLogger(__name__) User = get_user_model() @@ -24,7 +20,7 @@ CERTIFICATE_DELAY_SECONDS = 2 @shared_task(base=LoggedPersistOnFailureTask, bind=True, default_retry_delay=30, max_retries=2) @set_code_owner_attribute -def generate_certificate(self, **kwargs): +def generate_certificate(self, **kwargs): # pylint: disable=unused-argument """ Generates a certificate for a single user. @@ -32,39 +28,13 @@ def generate_certificate(self, **kwargs): - student: The student for whom to generate a certificate. - course_key: The course key for the course that the student is receiving a certificate in. - - expected_verification_status: The expected verification status - for the user. When the status has changed, we double check - 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. - - 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. + - status: Certificate status (value from the CertificateStatuses model) + - generation_mode: Used when emitting an event. 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) - v2_certificate = kwargs.pop('v2_certificate', False) status = kwargs.pop('status', CertificateStatuses.downloadable) generation_mode = kwargs.pop('generation_mode', 'batch') - if v2_certificate: - generate_course_certificate(user=student, course_key=course_key, status=status, generation_mode=generation_mode) - 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: - log.warning( - 'Expected verification status {expected} ' - 'differs from actual verification status {actual} ' - 'for user {user} in course {course}'.format( - expected=expected_verification_status, - actual=actual_verification_status, - user=student.id, - course=course_key - )) - raise self.retry(kwargs=original_kwargs) - generate_user_certificates(student=student, course_key=course_key, **kwargs) + generate_course_certificate(user=student, course_key=course_key, status=status, generation_mode=generation_mode) diff --git a/lms/djangoapps/certificates/tests/test_tasks.py b/lms/djangoapps/certificates/tests/test_tasks.py index 19972ccbf9..be4e3143e7 100644 --- a/lms/djangoapps/certificates/tests/test_tasks.py +++ b/lms/djangoapps/certificates/tests/test_tasks.py @@ -4,7 +4,7 @@ Tests for course certificate tasks. from unittest import mock -from unittest.mock import call, patch +from unittest.mock import patch import ddt from django.test import TestCase @@ -13,7 +13,6 @@ from opaque_keys.edx.keys import CourseKey from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.tasks import generate_certificate -from lms.djangoapps.verify_student.models import IDVerificationAttempt @ddt.ddt @@ -26,27 +25,6 @@ class GenerateUserCertificateTest(TestCase): self.user = UserFactory() - @patch('lms.djangoapps.certificates.tasks.generate_user_certificates') - @patch('lms.djangoapps.certificates.tasks.User.objects.get') - def test_generate_user_certs(self, user_get_mock, generate_user_certs_mock): - course_key = 'course-v1:edX+CS101+2017_T2' - kwargs = { - 'student': 'student-id', - 'course_key': course_key, - 'otherarg': 'c', - 'otherotherarg': 'd' - } - generate_certificate.apply_async(kwargs=kwargs).get() - - expected_student = user_get_mock.return_value - generate_user_certs_mock.assert_called_with( - student=expected_student, - course_key=CourseKey.from_string(course_key), - otherarg='c', - otherotherarg='d' - ) - user_get_mock.assert_called_once_with(id='student-id') - @ddt.data('student', 'course_key') def test_missing_args(self, missing_arg): kwargs = {'student': 'a', 'course_key': 'b', 'otherarg': 'c'} @@ -56,38 +34,9 @@ class GenerateUserCertificateTest(TestCase): with self.assertRaisesRegex(KeyError, missing_arg): generate_certificate.apply_async(kwargs=kwargs).get() - @patch('lms.djangoapps.certificates.tasks.generate_user_certificates') - @patch('lms.djangoapps.verify_student.services.IDVerificationService.user_status') - def test_retry_until_verification_status_updates(self, user_status_mock, generate_user_certs_mock): - course_key = 'course-v1:edX+CS101+2017_T2' - student = UserFactory() - - kwargs = { - 'student': student.id, - 'course_key': course_key, - 'expected_verification_status': IDVerificationAttempt.STATUS.approved - } - - user_status_mock.side_effect = [ - {'status': 'pending', 'error': '', 'should_display': True}, - {'status': 'approved', 'error': '', 'should_display': True} - ] - - generate_certificate.apply_async(kwargs=kwargs).get() - - user_status_mock.assert_has_calls([ - call(student), - call(student) - ]) - - generate_user_certs_mock.assert_called_once_with( - student=student, - course_key=CourseKey.from_string(course_key) - ) - def test_generation(self): """ - Verify the task handles V2 certificate generation + Verify the task handles certificate generation """ course_key = 'course-v1:edX+DemoX+Demo_Course' @@ -97,8 +46,7 @@ class GenerateUserCertificateTest(TestCase): ) as mock_generate_cert: kwargs = { 'student': self.user.id, - 'course_key': course_key, - 'v2_certificate': True + 'course_key': course_key } generate_certificate.apply_async(kwargs=kwargs) @@ -109,9 +57,9 @@ class GenerateUserCertificateTest(TestCase): generation_mode='batch' ) - def test_generation_mode(self): + def test_generation_custom(self): """ - Verify the task handles V2 certificate generation with a generation mode + Verify the task handles certificate generation custom params """ course_key = 'course-v1:edX+DemoX+Demo_Course' gen_mode = 'self' @@ -123,8 +71,8 @@ class GenerateUserCertificateTest(TestCase): kwargs = { 'student': self.user.id, 'course_key': course_key, - 'v2_certificate': True, - 'generation_mode': gen_mode + 'generation_mode': gen_mode, + 'what_about': 'dinosaurs' } generate_certificate.apply_async(kwargs=kwargs)