Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Remove V2 certificate checks from the certificates app #28108

Merged
merged 1 commit into from
Jul 8, 2021
Merged

fix: Remove V2 certificate checks from the certificates app #28108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 0 additions & 12 deletions lms/djangoapps/certificates/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from common.djangoapps.student.models import CourseEnrollment
from lms.djangoapps.branding import api as branding_api
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_on_certificate_allowlist as _is_on_certificate_allowlist,
Expand Down Expand Up @@ -209,17 +208,6 @@ def regenerate_user_certificates(student, course_key, forced_grade=None, templat
return _regenerate_user_certificates(student, course_key, forced_grade, template_file, insecure)


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 course run is using v2 course certificates
"""
return _can_generate_certificate_task(user, course_key)


def generate_certificate_task(user, course_key, generation_mode=None):
"""
Create a task to generate a certificate for this user in this course run, if the user is eligible and a certificate
Expand Down
155 changes: 13 additions & 142 deletions lms/djangoapps/certificates/generation_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

import logging

from edx_toggles.toggles import LegacyWaffleFlagNamespace

from common.djangoapps.course_modes import api as modes_api
from common.djangoapps.student.models import CourseEnrollment
from lms.djangoapps.certificates.data import CertificateStatuses
Expand All @@ -18,53 +16,15 @@
CertificateInvalidation,
GeneratedCertificate
)
from lms.djangoapps.certificates.queue import XQueueCertInterface
from lms.djangoapps.certificates.tasks import CERTIFICATE_DELAY_SECONDS, generate_certificate
from lms.djangoapps.certificates.utils import (
emit_certificate_event,
has_html_certificates_enabled
)
from lms.djangoapps.certificates.utils import has_html_certificates_enabled
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.instructor.access import list_with_level
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag

log = logging.getLogger(__name__)

WAFFLE_FLAG_NAMESPACE = LegacyWaffleFlagNamespace(name='certificates_revamp')

# .. 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.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2021-03-05
# .. toggle_target_removal_date: 2022-03-05
# .. toggle_tickets: MICROBA-923
CERTIFICATES_USE_UPDATED = CourseWaffleFlag(
waffle_namespace=WAFFLE_FLAG_NAMESPACE,
flag_name='use_updated',
module_name=__name__,
)


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 user is on the allowlist for the course run, or
- the course run is using v2 course certificates
"""
if is_on_certificate_allowlist(user, course_key):
return True
elif is_using_v2_course_certificates(course_key):
return True

return False


def generate_certificate_task(user, course_key, generation_mode=None):
"""
Expand All @@ -79,13 +39,8 @@ def generate_certificate_task(user, course_key, generation_mode=None):
f'certificate.')
return generate_allowlist_certificate_task(user, course_key, generation_mode)

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)

log.info(f'Neither an allowlist nor a v2 course certificate can be generated for {user.id} : {course_key}.')
return False
log.info(f'Attempt will be made to generate course certificate for user {user.id} : {course_key}')
return generate_regular_certificate_task(user, course_key, generation_mode)


def generate_allowlist_certificate_task(user, course_key, generation_mode=None):
Expand Down Expand Up @@ -163,11 +118,6 @@ 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):
# 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

if _is_ccx_course(course_key):
log.info(f'{course_key} is a CCX course. Certificate cannot be generated for {user.id}.')
return False
Expand Down Expand Up @@ -305,9 +255,6 @@ def _can_set_v2_cert_status(user, course_key):
"""
Determine whether we can set a custom (non-downloadable) cert status for a V2 certificate
"""
if not is_using_v2_course_certificates(course_key):
return False

if _is_ccx_course(course_key):
return False

Expand Down Expand Up @@ -341,16 +288,6 @@ def _can_set_cert_status_common(user, course_key):
return True


def is_using_v2_course_certificates(course_key): # pylint: disable=unused-argument
"""
Return True if the course run is using v2 course certificates

Note: this currently always returns True. This is an interim step as we roll out the feature to all course runs,
and the method will be removed entirely in MICROBA-1083.
"""
return True


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
Expand Down Expand Up @@ -432,7 +369,7 @@ def _is_cert_downloadable(user, course_key):
return True


def generate_user_certificates(student, course_key, insecure=False, generation_mode='batch', forced_grade=None):
def generate_user_certificates(student, course_key, insecure=False, generation_mode='batch', forced_grade=None): # pylint: disable=unused-argument
"""
It will add the add-cert request into the xqueue.

Expand All @@ -456,56 +393,14 @@ def generate_user_certificates(student, course_key, insecure=False, generation_m
forced_grade - a string indicating to replace grade parameter. if present grading
will be skipped.
"""
if can_generate_certificate_task(student, course_key):
# Note that this will launch an asynchronous task, and so cannot return the certificate status. This is a
# change from the older certificate code that tries to immediately create a cert.
log.info(f'{course_key} is using V2 certificates. Attempt will be made to regenerate a V2 certificate for user '
f'{student.id}.')
return generate_certificate_task(student, course_key)

beta_testers_queryset = list_with_level(course_key, 'beta')
if beta_testers_queryset.filter(username=student.username):
log.info(f"Canceling Certificate Generation task for user {student.id} : {course_key}. User is a Beta Tester.")
return

xqueue = XQueueCertInterface()
if insecure:
xqueue.use_https = False

course_overview = get_course_overview_or_none(course_key)
if not course_overview:
log.info(f"Canceling certificate generation for user {student.id} : {course_key} due to a missing course "
f"overview.")
return

generate_pdf = not has_html_certificates_enabled(course_overview)

cert = xqueue.add_cert(
student,
course_key,
generate_pdf=generate_pdf,
forced_grade=forced_grade
)
# Note that this will launch an asynchronous task, and so cannot return the certificate status. This is a
# change from the older certificate code that tries to immediately create a cert.
log.info(f'{course_key} is using V2 certificates. Attempt will be made to regenerate a V2 certificate for user '
f'{student.id}.')
return generate_certificate_task(student, course_key)

log.info(f"Queued Certificate Generation task for {student.id} : {course_key}")

# If cert_status is not present in certificate valid_statuses (for example unverified) then
# add_cert returns None and raises AttributeError while accessing cert attributes.
if cert is None:
return

if CertificateStatuses.is_passing_status(cert.status):
emit_certificate_event('created', student, course_key, course_overview, {
'user_id': student.id,
'course_id': str(course_key),
'certificate_id': cert.verify_uuid,
'enrollment_mode': cert.mode,
'generation_mode': generation_mode
})
return cert.status


def regenerate_user_certificates(student, course_key, forced_grade=None, template_file=None, insecure=False):
def regenerate_user_certificates(student, course_key, forced_grade=None, template_file=None, insecure=False): # pylint: disable=unused-argument
"""
Add the regen-cert request into the xqueue.

Expand All @@ -524,30 +419,6 @@ def regenerate_user_certificates(student, course_key, forced_grade=None, templat
template_file - The template file used to render this certificate
insecure - (Boolean)
"""
if can_generate_certificate_task(student, course_key):
log.info(f"{course_key} is using V2 certificates. Attempt will be made to regenerate a V2 certificate for "
f"user {student.id}.")
return generate_certificate_task(student, course_key)

xqueue = XQueueCertInterface()
if insecure:
xqueue.use_https = False

course_overview = get_course_overview_or_none(course_key)
if not course_overview:
log.info(f"Canceling certificate generation for user {student.id} : {course_key} due to a missing course "
f"overview.")
return False

generate_pdf = not has_html_certificates_enabled(course_overview)
log.info(f"Started regenerating certificates for user {student.id} in course {course_key} with generate_pdf "
f"status: {generate_pdf}.")

xqueue.regen_cert(
student,
course_key,
forced_grade=forced_grade,
template_file=template_file,
generate_pdf=generate_pdf
)
return True
log.info(f"{course_key} is using V2 certificates. Attempt will be made to regenerate a V2 certificate for "
f"user {student.id}.")
return generate_certificate_task(student, course_key)
10 changes: 0 additions & 10 deletions lms/djangoapps/certificates/tests/test_generation_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@
_can_generate_allowlist_certificate,
_can_generate_certificate_for_status,
_can_generate_v2_certificate,
can_generate_certificate_task,
generate_allowlist_certificate_task,
generate_certificate_task,
generate_regular_certificate_task,
is_on_certificate_allowlist,
is_using_v2_course_certificates,
_set_allowlist_cert_status,
_set_v2_cert_status
)
Expand Down Expand Up @@ -145,7 +143,6 @@ def test_handle_valid_general_methods(self):
"""
Test handling of a valid user/course run combo for the general (non-allowlist) generation methods
"""
assert can_generate_certificate_task(self.user, self.course_run_key)
assert generate_certificate_task(self.user, self.course_run_key)

def test_can_generate_not_verified(self):
Expand Down Expand Up @@ -298,7 +295,6 @@ def test_handle_valid(self):
Test handling of a valid user/course run combo.
"""
assert _can_generate_v2_certificate(self.user, self.course_run_key)
assert can_generate_certificate_task(self.user, self.course_run_key)
assert generate_certificate_task(self.user, self.course_run_key)

def test_handle_valid_task(self):
Expand Down Expand Up @@ -373,12 +369,6 @@ def test_handle_not_passing_id_verified_cert(self):
assert generate_regular_certificate_task(different_user, self.course_run_key) is True
assert not _can_generate_v2_certificate(different_user, self.course_run_key)

def test_is_using_updated_true(self):
"""
Test the updated flag
"""
assert is_using_v2_course_certificates(self.course_run_key)

@ddt.data(
(CertificateStatuses.deleted, True),
(CertificateStatuses.deleting, True),
Expand Down