diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index efe67fc4e6..9e438f9ff2 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -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( diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index d61a01f428..e05e808369 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -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}.") diff --git a/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py b/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py index 162cb6be30..b241a65a78 100644 --- a/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py +++ b/lms/djangoapps/certificates/management/commands/resubmit_error_certificates.py @@ -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 diff --git a/lms/djangoapps/certificates/management/commands/tests/test_cert_management.py b/lms/djangoapps/certificates/management/commands/tests/test_cert_management.py index d930558ce4..67a17dcc4b 100644 --- a/lms/djangoapps/certificates/management/commands/tests/test_cert_management.py +++ b/lms/djangoapps/certificates/management/commands/tests/test_cert_management.py @@ -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 diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index cf2a6f1d35..d6d240893c 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -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 diff --git a/lms/djangoapps/certificates/tests/test_generation_handler.py b/lms/djangoapps/certificates/tests/test_generation_handler.py index 60964bf783..783244de7e 100644 --- a/lms/djangoapps/certificates/tests/test_generation_handler.py +++ b/lms/djangoapps/certificates/tests/test_generation_handler.py @@ -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): """ diff --git a/openedx/core/djangoapps/content/course_overviews/api.py b/openedx/core/djangoapps/content/course_overviews/api.py index 032ebc6396..c04750d5e6 100644 --- a/openedx/core/djangoapps/content/course_overviews/api.py +++ b/openedx/core/djangoapps/content/course_overviews/api.py @@ -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. diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_api.py b/openedx/core/djangoapps/content/course_overviews/tests/test_api.py index dbb37fddf0..1e4fd238de 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_api.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_api.py @@ -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