Course Blocks API: Fix handling of incorrectly-cased course keys (#25911)
Commit 7f59688 attempted to solve this using
`CourseOverview.course_exists`, but that check is case-
insensitive. This commit instead does a small refactor
to `lms.djangoapps.courseware.get_course` so that we can
handle a failed course lookup without broadly catching
a `ValueError`.
This commit is contained in:
@@ -21,6 +21,7 @@ from six.moves import range
|
||||
|
||||
from lms.djangoapps.bulk_email.models import SEND_TO_MYSELF, BulkEmailFlag, CourseEmail
|
||||
from lms.djangoapps.bulk_email.tasks import perform_delegate_email_batches, send_course_email
|
||||
from lms.djangoapps.courseware.exceptions import CourseRunNotFound
|
||||
from lms.djangoapps.instructor_task.exceptions import DuplicateTaskException
|
||||
from lms.djangoapps.instructor_task.models import InstructorTask
|
||||
from lms.djangoapps.instructor_task.subtasks import (
|
||||
@@ -200,8 +201,7 @@ class TestEmailErrors(ModuleStoreTestCase):
|
||||
email.save()
|
||||
entry = InstructorTask.create(course_id, "task_type", "task_key", "task_input", self.instructor)
|
||||
task_input = {"email_id": email.id}
|
||||
# (?i) is a regex for ignore case
|
||||
with self.assertRaisesRegex(ValueError, r"(?i)course not found"):
|
||||
with self.assertRaises(CourseRunNotFound):
|
||||
perform_delegate_email_batches(entry.id, course_id, task_input, "action_name")
|
||||
|
||||
def test_nonexistent_to_option(self):
|
||||
|
||||
@@ -7,6 +7,7 @@ from opaque_keys.edx.keys import CourseKey
|
||||
from lms.djangoapps.courseware.access import has_access
|
||||
from lms.djangoapps.courseware.access_response import AccessResponse
|
||||
from lms.djangoapps.courseware.access_utils import ACCESS_DENIED, ACCESS_GRANTED, check_public_access
|
||||
from lms.djangoapps.courseware.exceptions import CourseRunNotFound
|
||||
from lms.djangoapps.courseware.courses import get_course
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from common.djangoapps.student.models import CourseEnrollment
|
||||
@@ -47,7 +48,8 @@ def is_course_public(course_key: CourseKey) -> AccessResponse:
|
||||
"""
|
||||
This checks if a course is publicly accessible or not.
|
||||
"""
|
||||
if not CourseOverview.course_exists(course_key):
|
||||
try:
|
||||
course = get_course(course_key, depth=0)
|
||||
except CourseRunNotFound:
|
||||
return ACCESS_DENIED
|
||||
course = get_course(course_key, depth=0)
|
||||
return check_public_access(course, [COURSE_VISIBILITY_PUBLIC])
|
||||
|
||||
@@ -41,6 +41,7 @@ from lms.djangoapps.courseware.date_summary import (
|
||||
VerificationDeadlineDate,
|
||||
VerifiedUpgradeDeadlineDate
|
||||
)
|
||||
from lms.djangoapps.courseware.exceptions import CourseRunNotFound
|
||||
from lms.djangoapps.courseware.masquerade import check_content_start_date_for_masquerade_user
|
||||
from lms.djangoapps.courseware.model_data import FieldDataCache
|
||||
from lms.djangoapps.courseware.module_render import get_module
|
||||
@@ -80,7 +81,7 @@ def get_course(course_id, depth=0):
|
||||
"""
|
||||
Given a course id, return the corresponding course descriptor.
|
||||
|
||||
If the course does not exist, raises a ValueError. This is appropriate
|
||||
If the course does not exist, raises a CourseRunNotFound. This is appropriate
|
||||
for internal use.
|
||||
|
||||
depth: The number of levels of children for the modulestore to cache.
|
||||
@@ -88,7 +89,7 @@ def get_course(course_id, depth=0):
|
||||
"""
|
||||
course = modulestore().get_course(course_id, depth=depth)
|
||||
if course is None:
|
||||
raise ValueError(u"Course not found: {0}".format(course_id))
|
||||
raise CourseRunNotFound(course_key=course_id)
|
||||
return course
|
||||
|
||||
|
||||
|
||||
@@ -25,3 +25,19 @@ class CourseAccessRedirect(Redirect):
|
||||
def __init__(self, url, access_error=None):
|
||||
super(CourseAccessRedirect, self).__init__(url)
|
||||
self.access_error = access_error
|
||||
|
||||
|
||||
class CourseRunNotFound(ValueError):
|
||||
"""
|
||||
Indicate that a supplied course run key does not map to a course run in the system.
|
||||
"""
|
||||
|
||||
def __init__(self, course_key):
|
||||
"""
|
||||
Initialize CourseRunNotFound exception.
|
||||
|
||||
Arguments:
|
||||
course_key (CourseKey|str):
|
||||
course run key or stringified version thereof.
|
||||
"""
|
||||
super().__init__(f"Course run not found: {course_key}")
|
||||
|
||||
Reference in New Issue
Block a user