Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Remove V2 certificate checks, since V2 has been enabled globally for all course runs (#28077)

Browse Source 
MICROBA-1083 DEPR-155
This commit is contained in:
Christie Rice
2021-07-06 11:00:54 -04:00
committed by GitHub
parent a70578862e
commit b8114ebc59
5 changed files with 12 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
lms/djangoapps/certificates/api.py
View File

@@ -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

14
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -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,

16
lms/djangoapps/courseware/views/views.py
View File

@@ -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()

58
lms/djangoapps/instructor_task/tasks_helper/certs.py
View File

@@ -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')

31
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

@@ -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.