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

fix: Remove unused allowlist flag (#27576)

Browse Source 
MICROBA-1073
This commit is contained in:
Christie Rice
2021-05-10 15:04:11 -04:00
committed by GitHub
parent 526982c548
commit 4dae1abd67
5 changed files with 23 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -25,7 +25,6 @@ from lms.djangoapps.certificates.generation_handler import (
can_generate_certificate_task as _can_generate_certificate_task,
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
)
@@ -612,9 +611,6 @@ def get_allowlisted_users(course_key):
"""
Return the users who are on the allowlist for this course run
"""
if not _is_using_certificate_allowlist(course_key):
return User.objects.none()
return User.objects.filter(certificatewhitelist__course_id=course_key, certificatewhitelist__whitelist=True)

58
lms/djangoapps/certificates/generation_handler.py
View File

@@ -34,28 +34,11 @@ log = logging.getLogger(__name__)
WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='certificates_revamp')
# .. toggle_name: certificates_revamp.use_allowlist
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable the course certificates allowlist (aka v2 of the certificate whitelist)
# on a per-course run basis.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2021-01-27
# .. toggle_target_removal_date: 2022-01-27
# .. toggle_tickets: MICROBA-918
CERTIFICATES_USE_ALLOWLIST = CourseWaffleFlag(
waffle_namespace=WAFFLE_FLAG_NAMESPACE,
flag_name='use_allowlist',
module_name=__name__,
)
# .. toggle_name: certificates_revamp.use_updated
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable the updated regular (non-allowlist) course certificate logic on a
# per-course run basis. Note that if this flag is enabled for a course run, certificates_revamp.use_allowlist
# should also be enabled for that course run.
# per-course run basis.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2021-03-05
# .. toggle_target_removal_date: 2022-03-05
@@ -72,10 +55,10 @@ def can_generate_certificate_task(user, course_key):
Determine if we can create a task to generate a certificate for this user in this course run.
This will return True if either:
- the course run is using the allowlist and the user is on the allowlist, or
- the user is on the allowlist for the course run, or
- the course run is using v2 course certificates
"""
if is_using_certificate_allowlist_and_is_on_allowlist(user, course_key):
if is_on_certificate_allowlist(user, course_key):
return True
elif is_using_v2_course_certificates(course_key):
return True
@@ -91,9 +74,9 @@ def generate_certificate_task(user, course_key, generation_mode=None):
If the allowlist is enabled for this course run and the user is on the allowlist, the allowlist logic will be used.
Otherwise, the regular course certificate generation logic will be used.
"""
if is_using_certificate_allowlist_and_is_on_allowlist(user, course_key):
log.info(f'{course_key} is using allowlist certificates, and the user {user.id} is on its allowlist. Attempt '
f'will be made to generate an allowlist certificate.')
if is_on_certificate_allowlist(user, course_key):
log.info(f'User {user.id} is on the allowlist for {course_key}. Attempt will be made to generate an allowlist '
f'certificate.')
return generate_allowlist_certificate_task(user, course_key, generation_mode)
elif is_using_v2_course_certificates(course_key):
@@ -157,12 +140,6 @@ def _can_generate_allowlist_certificate(user, course_key):
Check if an allowlist 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_certificate_allowlist(course_key):
# This course run is not using the allowlist feature
log.info(f'{course_key} is not using the certificate allowlist. Allowlist certificate cannot be generated'
f'for {user.id}.')
return False
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.')
@@ -315,9 +292,6 @@ def _can_set_allowlist_cert_status(user, course_key):
"""
Determine whether we can set a custom (non-downloadable) cert status for an allowlist certificate
"""
if not is_using_certificate_allowlist(course_key):
return False
if not is_on_certificate_allowlist(user, course_key):
return False
@@ -361,26 +335,6 @@ def _can_set_cert_status_common(user, course_key):
return True
def is_using_certificate_allowlist_and_is_on_allowlist(user, course_key):
"""
Return True if both:
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)
def is_using_certificate_allowlist(course_key):
"""
Check if the course run is using the allowlist, aka v2 of certificate whitelisting
Currently this returns True for all course runs, as we're enabling the allowlist for everyone.
If all goes well, we'll come back and remove this method and the flag entirely in MICROBA-1073.
"""
return True
# return CERTIFICATES_USE_ALLOWLIST.is_enabled(course_key)
def is_using_v2_course_certificates(course_key):
"""
Return True if the course run is using v2 course certificates

7
lms/djangoapps/certificates/services.py
View File

@@ -8,7 +8,7 @@ import logging
from django.core.exceptions import ObjectDoesNotExist
from opaque_keys.edx.keys import CourseKey
from lms.djangoapps.certificates.generation_handler import is_using_certificate_allowlist_and_is_on_allowlist
from lms.djangoapps.certificates.generation_handler import is_on_certificate_allowlist
from lms.djangoapps.certificates.models import GeneratedCertificate
from lms.djangoapps.utils import _get_key
@@ -26,9 +26,8 @@ class CertificateService:
course run.
"""
course_key = _get_key(course_key_or_id, CourseKey)
if is_using_certificate_allowlist_and_is_on_allowlist(user_id, course_key):
log.info('{course_key} is using allowlist certificates, and the user {user_id} is on its allowlist. The '
'certificate will not be invalidated.'.format(course_key=course_key, user_id=user_id))
if is_on_certificate_allowlist(user_id, course_key):
log.info(f'User {user_id} is on the allowlist for {course_key}. The certificate will not be invalidated.')
return False
try:

14
lms/djangoapps/certificates/signals.py
View File

@@ -15,7 +15,7 @@ from lms.djangoapps.certificates.generation_handler import (
can_generate_certificate_task,
generate_allowlist_certificate_task,
generate_certificate_task,
is_using_certificate_allowlist_and_is_on_allowlist
is_on_certificate_allowlist
)
from lms.djangoapps.certificates.models import (
CertificateGenerationCourseSetting,
@@ -60,9 +60,9 @@ def _listen_for_certificate_whitelist_append(sender, instance, **kwargs): # pyl
if not auto_certificate_generation_enabled():
return
if is_using_certificate_allowlist_and_is_on_allowlist(instance.user, instance.course_id):
log.info(f'{instance.course_id} is using allowlist certificates, and the user {instance.user.id} is now on '
f'its allowlist. Attempt will be made to generate an allowlist certificate.')
if is_on_certificate_allowlist(instance.user, instance.course_id):
log.info(f'User {instance.user.id} is now on the allowlist for course {instance.course_id}. Attempt will be '
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):
@@ -106,9 +106,9 @@ def _listen_for_failing_grade(sender, user, course_id, grade, **kwargs): # pyli
If needed, mark the certificate as notpassing.
"""
if is_using_certificate_allowlist_and_is_on_allowlist(user, course_id):
log.info('{course_id} is using allowlist certificates, and the user {user_id} is on its allowlist. The '
'failing grade will not affect the certificate.'.format(course_id=course_id, user_id=user.id))
if is_on_certificate_allowlist(user, course_id):
log.info(f'User {user.id} is on the allowlist for {course_id}. The failing grade will not affect the '
f'certificate.')
return
cert = GeneratedCertificate.certificate_for_student(user, course_id)

21
lms/djangoapps/certificates/tests/test_generation_handler.py
View File

@@ -17,8 +17,7 @@ from lms.djangoapps.certificates.generation_handler import (
generate_allowlist_certificate_task,
generate_certificate_task,
generate_regular_certificate_task,
is_using_certificate_allowlist,
is_using_certificate_allowlist_and_is_on_allowlist,
is_on_certificate_allowlist,
is_using_v2_course_certificates,
_set_allowlist_cert_status,
_set_v2_cert_status
@@ -66,21 +65,15 @@ class AllowlistTests(ModuleStoreTestCase):
# Whitelist user
CertificateAllowlistFactory.create(course_id=self.course_run_key, user=self.user)
def test_is_using_allowlist_true(self):
def test_is_on_allowlist(self):
"""
Test the allowlist flag
Test the presence of the user on the allowlist
"""
assert is_using_certificate_allowlist(self.course_run_key)
assert is_on_certificate_allowlist(self.user, self.course_run_key)
def test_is_using_allowlist_and_is_on_list(self):
def test_is_on_allowlist_false(self):
"""
Test the allowlist flag and the presence of the user on the list
"""
assert is_using_certificate_allowlist_and_is_on_allowlist(self.user, self.course_run_key)
def test_is_using_allowlist_and_is_on_list_true(self):
"""
Test the allowlist flag and the presence of the user on the list when the user is not on the list
Test the absence of the user on the allowlist
"""
u = UserFactory()
CourseEnrollmentFactory(
@@ -90,7 +83,7 @@ class AllowlistTests(ModuleStoreTestCase):
mode="verified",
)
CertificateAllowlistFactory.create(course_id=self.course_run_key, user=u, whitelist=False)
assert not is_using_certificate_allowlist_and_is_on_allowlist(u, self.course_run_key)
assert not is_on_certificate_allowlist(u, self.course_run_key)
@ddt.data(
(CertificateStatuses.deleted, True),