Merge pull request #27321 from edx/jhynes/microba-1077_bulk-regen-update
feat: skip bulk certificate invalidation during bulk regeneration in v2 certs
This commit is contained in:
@@ -24,6 +24,7 @@ from lms.djangoapps.certificates.generation_handler import (
|
||||
generate_certificate_task as _generate_certificate_task,
|
||||
generate_user_certificates as _generate_user_certificates,
|
||||
is_using_certificate_allowlist as _is_using_certificate_allowlist,
|
||||
is_using_v2_course_certificates as _is_using_v2_course_certificates,
|
||||
regenerate_user_certificates as _regenerate_user_certificates
|
||||
)
|
||||
from lms.djangoapps.certificates.models import (
|
||||
@@ -705,3 +706,10 @@ def get_certificate_invalidation_entry(certificate):
|
||||
return None
|
||||
|
||||
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)
|
||||
|
||||
@@ -74,7 +74,7 @@ def can_generate_certificate_task(user, course_key):
|
||||
"""
|
||||
if is_using_certificate_allowlist_and_is_on_allowlist(user, course_key):
|
||||
return True
|
||||
elif _is_using_v2_course_certificates(course_key):
|
||||
elif is_using_v2_course_certificates(course_key):
|
||||
return True
|
||||
|
||||
return False
|
||||
@@ -93,7 +93,7 @@ def generate_certificate_task(user, course_key, generation_mode=None):
|
||||
f'will be made to generate an allowlist certificate.')
|
||||
return generate_allowlist_certificate_task(user, course_key, generation_mode)
|
||||
|
||||
elif _is_using_v2_course_certificates(course_key):
|
||||
elif is_using_v2_course_certificates(course_key):
|
||||
log.info(f'{course_key} is using v2 course certificates. Attempt will be made to generate a certificate for '
|
||||
f'user {user.id}.')
|
||||
return generate_regular_certificate_task(user, course_key, generation_mode)
|
||||
@@ -154,7 +154,7 @@ def _can_generate_allowlist_certificate(user, course_key):
|
||||
f'for {user.id}.')
|
||||
return False
|
||||
|
||||
if not _is_on_certificate_allowlist(user, course_key):
|
||||
if not is_on_certificate_allowlist(user, course_key):
|
||||
log.info(f'{user.id} : {course_key} is not on the certificate allowlist. Allowlist certificate cannot be '
|
||||
f'generated.')
|
||||
return False
|
||||
@@ -175,7 +175,7 @@ def _can_generate_v2_certificate(user, course_key):
|
||||
Check if a v2 course certificate can be generated (created if it doesn't already exist, or updated if it does
|
||||
exist) for this user, in this course run.
|
||||
"""
|
||||
if not _is_using_v2_course_certificates(course_key):
|
||||
if not is_using_v2_course_certificates(course_key):
|
||||
# This course run is not using the v2 course certificate feature
|
||||
log.info(f'{course_key} is not using v2 course certificates. Certificate cannot be generated.')
|
||||
return False
|
||||
@@ -245,7 +245,7 @@ def is_using_certificate_allowlist_and_is_on_allowlist(user, course_key):
|
||||
1) the course run is using the allowlist, and
|
||||
2) if the user is on the allowlist for this course run
|
||||
"""
|
||||
return is_using_certificate_allowlist(course_key) and _is_on_certificate_allowlist(user, course_key)
|
||||
return is_using_certificate_allowlist(course_key) and is_on_certificate_allowlist(user, course_key)
|
||||
|
||||
|
||||
def is_using_certificate_allowlist(course_key):
|
||||
@@ -255,14 +255,14 @@ def is_using_certificate_allowlist(course_key):
|
||||
return CERTIFICATES_USE_ALLOWLIST.is_enabled(course_key)
|
||||
|
||||
|
||||
def _is_using_v2_course_certificates(course_key):
|
||||
def is_using_v2_course_certificates(course_key):
|
||||
"""
|
||||
Return True if the course run is using v2 course certificates
|
||||
"""
|
||||
return CERTIFICATES_USE_UPDATED.is_enabled(course_key)
|
||||
|
||||
|
||||
def _is_on_certificate_allowlist(user, course_key):
|
||||
def is_on_certificate_allowlist(user, course_key):
|
||||
"""
|
||||
Check if the user is on the allowlist, and is enabled for the allowlist, for this course run
|
||||
"""
|
||||
|
||||
@@ -12,7 +12,7 @@ from lms.djangoapps.certificates.generation_handler import (
|
||||
CERTIFICATES_USE_ALLOWLIST,
|
||||
CERTIFICATES_USE_UPDATED,
|
||||
is_using_certificate_allowlist,
|
||||
_is_using_v2_course_certificates,
|
||||
is_using_v2_course_certificates,
|
||||
_can_generate_allowlist_certificate,
|
||||
_can_generate_certificate_for_status,
|
||||
_can_generate_v2_certificate,
|
||||
@@ -309,14 +309,14 @@ class CertificateTests(ModuleStoreTestCase):
|
||||
"""
|
||||
Test the updated flag
|
||||
"""
|
||||
assert _is_using_v2_course_certificates(self.course_run_key)
|
||||
assert is_using_v2_course_certificates(self.course_run_key)
|
||||
|
||||
@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False)
|
||||
def test_is_using_updated_false(self):
|
||||
"""
|
||||
Test the updated flag without the override
|
||||
"""
|
||||
assert not _is_using_v2_course_certificates(self.course_run_key)
|
||||
assert not is_using_v2_course_certificates(self.course_run_key)
|
||||
|
||||
@ddt.data(
|
||||
(CertificateStatuses.deleted, True),
|
||||
|
||||
@@ -15,7 +15,8 @@ from lms.djangoapps.certificates.api import (
|
||||
can_generate_certificate_task,
|
||||
generate_certificate_task,
|
||||
generate_user_certificates,
|
||||
get_allowlisted_users
|
||||
get_allowlisted_users,
|
||||
is_using_v2_course_certificates,
|
||||
)
|
||||
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
||||
from xmodule.modulestore.django import modulestore
|
||||
@@ -140,27 +141,33 @@ def _invalidate_generated_certificates(course_id, enrolled_students, certificate
|
||||
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 verify_uuid, download_uuid,
|
||||
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
|
||||
"""
|
||||
certificates = GeneratedCertificate.objects.filter(
|
||||
user__in=enrolled_students,
|
||||
course_id=course_id,
|
||||
status__in=certificate_statuses,
|
||||
)
|
||||
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)
|
||||
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 for '
|
||||
f'course {course_id}')
|
||||
else:
|
||||
log.info(f'About to invalidate certificate for user {c.user.id} in course {course_id}')
|
||||
c.invalidate()
|
||||
# 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()
|
||||
|
||||
@@ -30,7 +30,10 @@ 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_ALLOWLIST
|
||||
from lms.djangoapps.certificates.generation_handler import (
|
||||
CERTIFICATES_USE_ALLOWLIST,
|
||||
CERTIFICATES_USE_UPDATED
|
||||
)
|
||||
from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate
|
||||
from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory
|
||||
from lms.djangoapps.courseware.models import StudentModule
|
||||
@@ -2529,6 +2532,50 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
|
||||
invalidated_cert = certs.first()
|
||||
assert invalidated_cert.status == CertificateStatuses.unavailable
|
||||
|
||||
@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False)
|
||||
def test_invalidation_v2_certificates_disabled(self):
|
||||
"""
|
||||
Test that ensures the bulk invalidation step (as part of bulk certificate regeneration) continues to occur when
|
||||
the v2 certificates feature is disabled 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.unavailable
|
||||
|
||||
@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.
|
||||
|
||||
Reference in New Issue
Block a user