diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index f0d3d3d51e..a08d964054 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -27,7 +27,6 @@ from lms.djangoapps.certificates.generation_handler import ( generate_certificate_task as _generate_certificate_task, generate_user_certificates as _generate_user_certificates, is_on_certificate_allowlist as _is_on_certificate_allowlist, - is_using_v2_course_certificates as _is_using_v2_course_certificates, regenerate_user_certificates as _regenerate_user_certificates ) from lms.djangoapps.certificates.data import CertificateStatuses @@ -735,13 +734,6 @@ def get_certificate_invalidation_entry(certificate): return certificate_invalidation_entry -def is_using_v2_course_certificates(course_key): - """ - Determines if the given course run is using V2 of course certificates - """ - return _is_using_v2_course_certificates(course_key) - - def get_allowlist(course_key): """ Return the certificate allowlist for the given course run diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 12bff0384a..a8def5309a 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -38,7 +38,6 @@ from freezegun import freeze_time # lint-amnesty, pylint: disable=wrong-import- from common.djangoapps.student.tests.factories import GlobalStaffFactory from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf from lms.djangoapps.certificates import api as certs_api -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_UPDATED from lms.djangoapps.certificates.models import ( CertificateGenerationConfiguration, CertificateStatuses @@ -2355,8 +2354,7 @@ class GenerateUserCertTests(ModuleStoreTestCase): status_code=HttpResponseBadRequest.status_code, ) - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) - def test_v2_certificates_with_passing_grade(self): + def test_certificates_with_passing_grade(self): with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_read_grade: course_grade = mock_read_grade.return_value course_grade.passed = True @@ -2369,10 +2367,9 @@ class GenerateUserCertTests(ModuleStoreTestCase): mock_cert_task.assert_called_with(self.student, self.course.id, 'self') assert resp.status_code == 200 - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) - def test_v2_certificates_not_passing(self): + def test_certificates_not_passing(self): """ - Test v2 course certificates when the user is not passing the course + Test course certificates when the user is not passing the course """ with patch( 'lms.djangoapps.certificates.api.generate_certificate_task', @@ -2386,10 +2383,9 @@ class GenerateUserCertTests(ModuleStoreTestCase): status_code=HttpResponseBadRequest.status_code, ) - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) - def test_v2_certificates_with_existing_downloadable_cert(self): + def test_certificates_with_existing_downloadable_cert(self): """ - Test v2 course certificates when the user is passing the course and already has a cert + Test course certificates when the user is passing the course and already has a cert """ GeneratedCertificateFactory.create( user=self.student, diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 6856fcae7d..4e64842e49 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1610,17 +1610,13 @@ def generate_user_cert(request, course_id): student = request.user course_key = CourseKey.from_string(course_id) - use_v1_certs = True course = modulestore().get_course(course_key, depth=2) if not course: return HttpResponseBadRequest(_("Course is not valid")) - 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}.') - use_v1_certs = False - certs_api.generate_certificate_task(student, course_key, 'self') + log.info(f'Attempt will be made to generate a course certificate for {student.id} : {course_key}.') + certs_api.generate_certificate_task(student, course_key, 'self') if not is_course_passed(student, course): log.info("User %s has not passed the course: %s", student.username, course_id) @@ -1640,14 +1636,6 @@ def generate_user_cert(request, course_id): return HttpResponseBadRequest(_("Certificate has already been created.")) elif certificate_status["is_generating"]: return HttpResponseBadRequest(_("Certificate is being created.")) - elif use_v1_certs: - # If the certificate is not already in-process or completed, - # then create a new certificate generation task. - # If the certificate cannot be added to the queue, this will - # mark the certificate with "error" status, so it can be re-run - # with a management command. From the user's perspective, - # it will appear that the certificate task was submitted successfully. - certs_api.generate_user_certificates(student, course.id, generation_mode='self') return HttpResponse() diff --git a/lms/djangoapps/instructor_task/tasks_helper/certs.py b/lms/djangoapps/instructor_task/tasks_helper/certs.py index 05380f4d3c..28f51d06a1 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/certs.py +++ b/lms/djangoapps/instructor_task/tasks_helper/certs.py @@ -12,16 +12,11 @@ from django.db.models import Q from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.certificates.api import ( - can_generate_certificate_task, generate_certificate_task, - generate_user_certificates, - get_allowlisted_users, get_enrolled_allowlisted_users, - get_enrolled_allowlisted_not_passing_users, - is_using_v2_course_certificates, + get_enrolled_allowlisted_not_passing_users ) from lms.djangoapps.certificates.data import CertificateStatuses -from lms.djangoapps.certificates.models import GeneratedCertificate from .runner import TaskProgress @@ -68,11 +63,6 @@ def generate_students_certificates( log.info(f'About to attempt certificate generation for {len(students_require_certs)} users in course {course_id}. ' f'The student_set is {student_set} and statuses_to_regenerate is {statuses_to_regenerate}') - if statuses_to_regenerate: - # Mark existing generated certificates as 'unavailable' before regenerating - # We need to call this method after "students_require_certificate" otherwise "students_require_certificate" - # would return no results. - _invalidate_generated_certificates(course_id, students_to_generate_certs_for, statuses_to_regenerate) task_progress.skipped = task_progress.total - len(students_require_certs) @@ -82,13 +72,8 @@ def generate_students_certificates( # Generate certificate for each student for student in students_require_certs: task_progress.attempted += 1 - if can_generate_certificate_task(student, course_id): - log.info(f'{course_id} is using V2 certificates. Attempt will be made to generate a V2 certificate ' - f'for user {student.id}.') - generate_certificate_task(student, course_id) - else: - log.info(f'Attempt will be made to generate a certificate for user {student.id} in {course_id}.') - generate_user_certificates(student, course_id) + log.info(f'Attempt will be made to generate a course certificate for {student.id} : {course_id}.') + generate_certificate_task(student, course_id) return task_progress.update_task_state(extra_meta=current_step) @@ -122,40 +107,3 @@ def students_require_certificate(course_id, enrolled_students, statuses_to_regen # Return all the enrolled student skipping the ones whose certificates have already been generated return list(set(enrolled_students) - set(students_already_have_certs)) - - -def _invalidate_generated_certificates(course_id, enrolled_students, certificate_statuses): - """ - Invalidate generated certificates for all enrolled students in the given course having status in - 'certificate_statuses', if the student is not on the course's allowlist. - - Generated Certificates are invalidated by marking its status 'unavailable' and updating error_reason, download_uuid, - download_url and grade with empty string. - - If V2 of Course Certificates is enabled for this course-run, this step will be skipped. - - :param course_id: Course Key for the course whose generated certificates need to be removed - :param enrolled_students: (queryset or list) students enrolled in the course - :param certificate_statuses: certificates statuses for whom to remove generated certificate - """ - if is_using_v2_course_certificates(course_id): - log.info(f"Course {course_id} is using V2 certificates. Skipping certificate invalidation step of bulk " - "regeneration.") - else: - certificates = GeneratedCertificate.objects.filter( - user__in=enrolled_students, - course_id=course_id, - status__in=certificate_statuses, - ) - - allowlisted_users = get_allowlisted_users(course_id) - - # Invalidate each cert that is not allowlisted. We loop over the certs and invalidate each individually in order - # to save a history of the change. - for c in certificates: - if c.user in allowlisted_users: - log.info(f'Certificate for user {c.user.id} will not be invalidated because they are on the allowlist ' - f'for course {course_id}') - else: - log.info(f'About to invalidate certificate for user {c.user.id} in course {course_id}') - c.invalidate(source='bulk_certificate_regeneration') diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index e5a62ed18e..4c9a63a5d3 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -21,7 +21,6 @@ import unicodecsv from django.conf import settings from django.test.utils import override_settings from edx_django_utils.cache import RequestCache -from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time from pytz import UTC @@ -30,7 +29,6 @@ from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from lms.djangoapps.certificates.generation_handler import CERTIFICATES_USE_UPDATED from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.certificates.tests.factories import CertificateAllowlistFactory, GeneratedCertificateFactory @@ -40,10 +38,7 @@ from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsec from lms.djangoapps.grades.subsection_grade import CreateSubsectionGrade from lms.djangoapps.grades.transformer import GradesTransformer from lms.djangoapps.instructor_analytics.basic import UNAVAILABLE, list_problem_responses -from lms.djangoapps.instructor_task.tasks_helper.certs import ( - generate_students_certificates, - _invalidate_generated_certificates -) +from lms.djangoapps.instructor_task.tasks_helper.certs import generate_students_certificates from lms.djangoapps.instructor_task.tasks_helper.enrollments import upload_may_enroll_csv, upload_students_csv from lms.djangoapps.instructor_task.tasks_helper.grades import ( ENROLLED_IN_COURSE, @@ -2087,7 +2082,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 0, 'skipped': 2 } - with self.assertNumQueries(74): + with self.assertNumQueries(66): self.assertCertificatesGenerated(task_input, expected_results) @ddt.data( @@ -2486,28 +2481,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): self.assertCertificatesGenerated(task_input, expected_results) - @override_waffle_flag(CERTIFICATES_USE_UPDATED, active=True) - def test_invalidation_v2_certificates_enabled(self): - """ - Test that ensures the bulk invalidation step (as part of bulk certificate regeneration) is skipped when the v2 - certificates feature is enabled for a course run. - """ - students = self._create_students(2) - - for s in students: - GeneratedCertificateFactory.create( - user=s, - course_id=self.course.id, - status=CertificateStatuses.downloadable, - mode='verified' - ) - - _invalidate_generated_certificates(self.course.id, students, [CertificateStatuses.downloadable]) - - for s in students: - cert = GeneratedCertificate.objects.get(user=s, course_id=self.course.id) - assert cert.status == CertificateStatuses.downloadable - def assertCertificatesGenerated(self, task_input, expected_results): """ Generate certificates for the given task_input and compare with expected_results.