Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions lms/djangoapps/certificates/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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
)
Expand Down Expand Up @@ -587,9 +586,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)


Expand Down
58 changes: 6 additions & 52 deletions lms/djangoapps/certificates/generation_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,11 @@

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
Expand All @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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.')
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions lms/djangoapps/certificates/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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

Expand All @@ -26,9 +26,8 @@ def invalidate_certificate(self, user_id, course_key_or_id):
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:
Expand Down
14 changes: 7 additions & 7 deletions lms/djangoapps/certificates/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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.')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I don't think we need the leading f in the string. We aren't substituting any strings on this line.

return

cert = GeneratedCertificate.certificate_for_student(user, course_id)
Expand Down
21 changes: 7 additions & 14 deletions lms/djangoapps/certificates/tests/test_generation_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
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
Expand Down Expand Up @@ -66,21 +65,15 @@ def setUp(self):
# 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(
Expand All @@ -90,7 +83,7 @@ def test_is_using_allowlist_and_is_on_list_true(self):
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),
Expand Down