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

fix: Handle missing course overviews (#27849)

Browse Source 
MICROBA-1082
This commit is contained in:
Christie Rice
2021-06-14 12:57:45 -04:00
committed by GitHub
parent 0ee5a64939
commit 417b2cc959
8 changed files with 97 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
lms/djangoapps/certificates/generation.py
View File

@@ -19,7 +19,7 @@ from lms.djangoapps.certificates.queue import XQueueCertInterface
from lms.djangoapps.certificates.utils import emit_certificate_event, has_html_certificates_enabled
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.instructor.access import list_with_level
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
log = logging.getLogger(__name__)
@@ -132,7 +132,12 @@ def generate_user_certificates(student, course_key, insecure=False, generation_m
if insecure:
xqueue.use_https = False
course_overview = get_course_overview(course_key)
course_overview = get_course_overview_or_none(course_key)
if not course_overview:
log.info(f"Canceling Certificate Generation task 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(

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

@@ -27,7 +27,7 @@ from lms.djangoapps.certificates.utils import (
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
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__)
@@ -215,7 +215,11 @@ def _can_generate_certificate_common(user, course_key):
if not _can_generate_certificate_for_status(user, course_key):
return False
course_overview = get_course_overview(course_key)
course_overview = get_course_overview_or_none(course_key)
if not course_overview:
log.info(f'{course_key} does not a course overview. Certificate cannot be generated for {user.id}.')
return False
if not has_html_certificates_enabled(course_overview):
log.info(f'{course_key} does not have HTML certificates enabled. Certificate cannot be generated for '
f'{user.id}.')
@@ -328,7 +332,10 @@ def _can_set_cert_status_common(user, course_key):
if not modes_api.is_eligible_for_certificate(enrollment_mode):
return False
course_overview = get_course_overview(course_key)
course_overview = get_course_overview_or_none(course_key)
if not course_overview:
return False
if not has_html_certificates_enabled(course_overview):
return False
@@ -452,7 +459,12 @@ def generate_user_certificates(student, course_key, insecure=False, generation_m
if insecure:
xqueue.use_https = False
course_overview = get_course_overview(course_key)
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(
@@ -508,7 +520,12 @@ def regenerate_user_certificates(student, course_key, forced_grade=None, templat
if insecure:
xqueue.use_https = False
course_overview = get_course_overview(course_key)
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}.")

4
lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py
View File

@@ -28,7 +28,7 @@ from opaque_keys.edx.keys import CourseKey
from lms.djangoapps.certificates.api import generate_user_certificates
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
LOGGER = logging.getLogger(__name__)
@@ -117,7 +117,7 @@ class Command(BaseCommand):
"""Retrieve the course-overview for the given course-key and store it."""
course_overview = (
course_cache[course_key] if course_key in course_cache
else get_course_overview(course_key)
else get_course_overview_or_none(course_key)
)
course_cache[course_key] = course_overview
return course_overview

2
lms/djangoapps/certificates/management/commands/tests/test_cert_management.py
View File

@@ -134,7 +134,7 @@ class ResubmitErrorCertificatesTest(CertificateManagementTest):
)
with patch(
'lms.djangoapps.certificates.management.commands.resubmit_error_certificates.get_course_overview'
'lms.djangoapps.certificates.management.commands.resubmit_error_certificates.get_course_overview_or_none'
) as mock_get_course_overview:
mock_get_course_overview.return_value = course_overview

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

@@ -27,7 +27,7 @@ from lms.djangoapps.certificates.models import (
)
from lms.djangoapps.grades.api import CourseGradeFactory
from lms.djangoapps.verify_student.services import IDVerificationService
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none
LOGGER = logging.getLogger(__name__)
@@ -401,7 +401,10 @@ class XQueueCertInterface:
sends a request to XQueue.
"""
course_id = str(cert.course_id)
course_overview = get_course_overview(course_id)
course_overview = get_course_overview_or_none(course_id)
if not course_overview:
LOGGER.warning(f"Skipping cert generation for {student.id} due to missing course overview for {course_id}")
return cert
key = make_hashkey(random.random())
cert.key = key

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

@@ -35,6 +35,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
log = logging.getLogger(__name__)
BETA_TESTER_METHOD = 'lms.djangoapps.certificates.generation_handler._is_beta_tester'
COURSE_OVERVIEW_METHOD = 'lms.djangoapps.certificates.generation_handler.get_course_overview_or_none'
CCX_COURSE_METHOD = 'lms.djangoapps.certificates.generation_handler._is_ccx_course'
ID_VERIFIED_METHOD = 'lms.djangoapps.verify_student.services.IDVerificationService.user_is_verified'
PASSING_GRADE_METHOD = 'lms.djangoapps.certificates.generation_handler._has_passing_grade'
@@ -240,6 +241,14 @@ class AllowlistTests(ModuleStoreTestCase):
assert not _can_generate_allowlist_certificate(self.user, self.course_run_key)
assert _set_allowlist_cert_status(self.user, self.course_run_key) is None
def test_can_generate_no_overview(self):
"""
Test handling when the course overview is missing
"""
with mock.patch(COURSE_OVERVIEW_METHOD, return_value=None):
assert not _can_generate_allowlist_certificate(self.user, self.course_run_key)
assert _set_allowlist_cert_status(self.user, self.course_run_key) is None
def test_cert_status_downloadable(self):
"""
Test cert status when status is already downloadable
@@ -461,6 +470,14 @@ class CertificateTests(ModuleStoreTestCase):
assert not _can_generate_v2_certificate(self.user, self.course_run_key)
assert _set_v2_cert_status(self.user, self.course_run_key) is None
def test_can_generate_no_overview(self):
"""
Test handling when the course overview is missing
"""
with mock.patch(COURSE_OVERVIEW_METHOD, return_value=None):
assert not _can_generate_v2_certificate(self.user, self.course_run_key)
assert _set_v2_cert_status(self.user, self.course_run_key) is None
@override_waffle_flag(CERTIFICATES_USE_UPDATED, active=False)
def test_cert_status_v1(self):
"""

19
openedx/core/djangoapps/content/course_overviews/api.py
View File

@@ -1,11 +1,15 @@
"""
CourseOverview internal api
CourseOverview api
"""
import logging
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.content.course_overviews.serializers import (
CourseOverviewBaseSerializer,
)
log = logging.getLogger(__name__)
def get_course_overview(course_id):
"""
@@ -14,6 +18,19 @@ def get_course_overview(course_id):
return CourseOverview.get_from_id(course_id)
def get_course_overview_or_none(course_id):
"""
Retrieve and return course overview data for the provided course id.
If the course overview does not exist, return None.
"""
try:
return get_course_overview(course_id)
except CourseOverview.DoesNotExist:
log.warning(f"Course overview does not exist for {course_id}")
return None
def get_course_overviews(course_ids):
"""
Return course_overview data for a given list of opaque_key course_ids.

29
openedx/core/djangoapps/content/course_overviews/tests/test_api.py
View File

@@ -1,15 +1,20 @@
"""
course_overview api tests
"""
from django.test import TestCase
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview, get_course_overviews
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.content.course_overviews.api import (
get_course_overview,
get_course_overview_or_none,
get_course_overviews
)
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from ..models import CourseOverview
class TestCourseOverviewsApi(TestCase):
class TestCourseOverviewsApi(ModuleStoreTestCase):
"""
TestCourseOverviewsApi tests.
"""
@@ -27,6 +32,22 @@ class TestCourseOverviewsApi(TestCase):
retrieved_course_overview = get_course_overview(course_overview.id)
assert course_overview.id == retrieved_course_overview.id
def test_get_course_overview_or_none(self):
"""
Test for `test_get_course_overview_or_none` function when the overview exists.
"""
course_overview = CourseOverviewFactory.create()
retrieved_course_overview = get_course_overview_or_none(course_overview.id)
assert course_overview.id == retrieved_course_overview.id
def test_get_course_overview_or_none_missing(self):
"""
Test for `test_get_course_overview_or_none` function when the overview does not exist.
"""
course_run_key = CourseKey.from_string('course-v1:coping+with+deletions')
retrieved_course_overview = get_course_overview_or_none(course_run_key)
assert retrieved_course_overview is None
def test_get_course_overviews(self):
"""
get_course_overviews should return the expected CourseOverview data