feat: list courses details by keys
This adds the ability to get a list of detailed courses based on their keys provided in the newly added `keys` query param in the `GET /courses/v1/courses/` endpoint.
This commit is contained in:
@@ -2633,6 +2633,10 @@ paths:
|
||||
date are returned. This is different from search_term because this filtering is done on
|
||||
CourseOverview and not ElasticSearch.
|
||||
|
||||
course_keys (optional):
|
||||
If specified, it filters visible `CourseOverview` objects by
|
||||
the course keys (ids) provided
|
||||
|
||||
**Returns**
|
||||
|
||||
* 200 on success, with a list of course discovery objects as returned
|
||||
|
||||
@@ -14,7 +14,7 @@ from opaque_keys.edx.keys import CourseKey
|
||||
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
|
||||
|
||||
|
||||
def get_visible_courses(org=None, filter_=None, active_only=False):
|
||||
def get_visible_courses(org=None, filter_=None, active_only=False, course_keys=None):
|
||||
"""
|
||||
Yield the CourseOverviews that should be visible in this branded
|
||||
instance.
|
||||
@@ -25,6 +25,8 @@ def get_visible_courses(org=None, filter_=None, active_only=False):
|
||||
filter_ (dict): Optional parameter that allows custom filtering by
|
||||
fields on the course.
|
||||
active_only (bool): Optional parameter that enables fetching active courses only.
|
||||
course_keys (list[str]): Optional parameter that allows for selecting which
|
||||
courses to fetch the `CourseOverviews` for
|
||||
"""
|
||||
# Import is placed here to avoid model import at project startup.
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
@@ -36,12 +38,16 @@ def get_visible_courses(org=None, filter_=None, active_only=False):
|
||||
if org:
|
||||
# Check the current site's orgs to make sure the org's courses should be displayed
|
||||
if not current_site_orgs or org in current_site_orgs:
|
||||
courses = CourseOverview.get_all_courses(orgs=[org], filter_=filter_, active_only=active_only)
|
||||
courses = CourseOverview.get_all_courses(
|
||||
orgs=[org], filter_=filter_, active_only=active_only, course_keys=course_keys
|
||||
)
|
||||
elif current_site_orgs:
|
||||
# Only display courses that should be displayed on this site
|
||||
courses = CourseOverview.get_all_courses(orgs=current_site_orgs, filter_=filter_, active_only=active_only)
|
||||
courses = CourseOverview.get_all_courses(
|
||||
orgs=current_site_orgs, filter_=filter_, active_only=active_only, course_keys=course_keys
|
||||
)
|
||||
else:
|
||||
courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only)
|
||||
courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only, course_keys=course_keys)
|
||||
|
||||
courses = courses.order_by('id')
|
||||
|
||||
|
||||
@@ -116,7 +116,8 @@ def list_courses(request,
|
||||
filter_=None,
|
||||
search_term=None,
|
||||
permissions=None,
|
||||
active_only=False):
|
||||
active_only=False,
|
||||
course_keys=None):
|
||||
"""
|
||||
Yield all available courses.
|
||||
|
||||
@@ -146,12 +147,17 @@ def list_courses(request,
|
||||
If specified, it filters visible `CourseOverview` objects by
|
||||
checking if each permission specified is granted for the username.
|
||||
active_only (bool): Optional parameter that enables fetching active courses only.
|
||||
course_keys (list[str]):
|
||||
If specified, it filters visible `CourseOverview` objects by
|
||||
the course keys (ids) provided
|
||||
|
||||
Return value:
|
||||
Yield `CourseOverview` objects representing the collection of courses.
|
||||
"""
|
||||
user = get_effective_user(request.user, username)
|
||||
course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions, active_only=active_only)
|
||||
course_qs = get_courses(
|
||||
user, org=org, filter_=filter_, permissions=permissions, active_only=active_only, course_keys=course_keys
|
||||
)
|
||||
course_qs = _filter_by_search(course_qs, search_term)
|
||||
return course_qs
|
||||
|
||||
|
||||
@@ -64,6 +64,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
|
||||
mobile = ExtendedNullBooleanField(required=False)
|
||||
active_only = ExtendedNullBooleanField(required=False)
|
||||
permissions = MultiValueField(required=False)
|
||||
course_keys = MultiValueField(required=False)
|
||||
|
||||
def clean(self):
|
||||
"""
|
||||
@@ -80,6 +81,20 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
|
||||
|
||||
return cleaned_data
|
||||
|
||||
def clean_course_keys(self):
|
||||
"""
|
||||
Ensure valid course_keys were provided.
|
||||
"""
|
||||
course_keys = self.cleaned_data['course_keys']
|
||||
if course_keys:
|
||||
for course_key in course_keys:
|
||||
try:
|
||||
CourseKey.from_string(course_key)
|
||||
except InvalidKeyError:
|
||||
raise ValidationError(f"'{str(course_key)}' is not a valid course key.") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
|
||||
return course_keys
|
||||
|
||||
|
||||
class CourseIdListGetForm(UsernameValidatorMixin, Form):
|
||||
"""
|
||||
|
||||
@@ -107,7 +107,8 @@ class CourseListTestMixin(CourseApiTestMixin):
|
||||
specified_user,
|
||||
org=None,
|
||||
filter_=None,
|
||||
permissions=None):
|
||||
permissions=None,
|
||||
course_keys=None):
|
||||
"""
|
||||
Call the list_courses api endpoint to get information about
|
||||
`specified_user` on behalf of `requesting_user`.
|
||||
@@ -121,6 +122,7 @@ class CourseListTestMixin(CourseApiTestMixin):
|
||||
org=org,
|
||||
filter_=filter_,
|
||||
permissions=permissions,
|
||||
course_keys=course_keys,
|
||||
)
|
||||
|
||||
def verify_courses(self, courses):
|
||||
@@ -244,6 +246,39 @@ class TestGetCourseListMultipleCourses(CourseListTestMixin, ModuleStoreTestCase)
|
||||
|
||||
self.assertEqual({c.id for c in filtered_courses}, {self.course.id})
|
||||
|
||||
def test_filter_by_keys(self):
|
||||
"""
|
||||
Verify that courses are filtered by the provided course keys.
|
||||
"""
|
||||
|
||||
# Create alternative courses to be included in the `course_keys` filter.
|
||||
alternative_course_1 = self.create_course(course='alternative-course-1')
|
||||
alternative_course_2 = self.create_course(course='alternative-course-2')
|
||||
|
||||
# No filtering.
|
||||
unfiltered_expected_courses = [
|
||||
self.course,
|
||||
alternative_course_1,
|
||||
alternative_course_2,
|
||||
]
|
||||
unfiltered_courses = self._make_api_call(self.honor_user, self.honor_user)
|
||||
assert {course.id for course in unfiltered_courses} == {course.id for course in unfiltered_expected_courses}
|
||||
|
||||
# With filtering.
|
||||
filtered_expected_courses = [
|
||||
alternative_course_1,
|
||||
alternative_course_2,
|
||||
]
|
||||
filtered_courses = self._make_api_call(
|
||||
self.honor_user,
|
||||
self.honor_user,
|
||||
course_keys={
|
||||
alternative_course_1.id,
|
||||
alternative_course_2.id
|
||||
}
|
||||
)
|
||||
assert {course.id for course in filtered_courses} == {course.id for course in filtered_expected_courses}
|
||||
|
||||
|
||||
class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
|
||||
"""
|
||||
|
||||
@@ -71,6 +71,7 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT
|
||||
'filter_': None,
|
||||
'permissions': set(),
|
||||
'active_only': None,
|
||||
'course_keys': set(),
|
||||
}
|
||||
|
||||
def test_basic(self):
|
||||
@@ -100,6 +101,14 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT
|
||||
|
||||
self.assert_valid(self.cleaned_data)
|
||||
|
||||
def test_invalid_course_keys(self):
|
||||
"""
|
||||
Verify form checks for validity of course keys provided
|
||||
"""
|
||||
|
||||
self.form_data['course_keys'] = 'course-v1:edX+DemoX+Demo_Course,invalid_course_key'
|
||||
self.assert_error('course_keys', "'invalid_course_key' is not a valid course key.")
|
||||
|
||||
|
||||
class TestCourseIdListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring
|
||||
FORM_CLASS = CourseIdListGetForm
|
||||
|
||||
@@ -286,6 +286,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
date are returned. This is different from search_term because this filtering is done on
|
||||
CourseOverview and not ElasticSearch.
|
||||
|
||||
course_keys (optional):
|
||||
If specified, it fetches the `CourseOverview` objects for the
|
||||
the specified course keys
|
||||
|
||||
**Returns**
|
||||
|
||||
* 200 on success, with a list of course discovery objects as returned
|
||||
@@ -343,7 +347,8 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
filter_=form.cleaned_data['filter_'],
|
||||
search_term=form.cleaned_data['search_term'],
|
||||
permissions=form.cleaned_data['permissions'],
|
||||
active_only=form.cleaned_data.get('active_only', False)
|
||||
active_only=form.cleaned_data.get('active_only', False),
|
||||
course_keys=form.cleaned_data['course_keys'],
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -753,7 +753,7 @@ def get_course_syllabus_section(course, section_key):
|
||||
|
||||
|
||||
@function_trace('get_courses')
|
||||
def get_courses(user, org=None, filter_=None, permissions=None, active_only=False):
|
||||
def get_courses(user, org=None, filter_=None, permissions=None, active_only=False, course_keys=None):
|
||||
"""
|
||||
Return a LazySequence of courses available, optionally filtered by org code
|
||||
(case-insensitive) or a set of permissions to be satisfied for the specified
|
||||
@@ -763,7 +763,8 @@ def get_courses(user, org=None, filter_=None, permissions=None, active_only=Fals
|
||||
courses = branding.get_visible_courses(
|
||||
org=org,
|
||||
filter_=filter_,
|
||||
active_only=active_only
|
||||
active_only=active_only,
|
||||
course_keys=course_keys
|
||||
).prefetch_related(
|
||||
'modes',
|
||||
).select_related(
|
||||
|
||||
@@ -662,7 +662,7 @@ class CourseOverview(TimeStampedModel):
|
||||
log.info('Finished generating course overviews.')
|
||||
|
||||
@classmethod
|
||||
def get_all_courses(cls, orgs=None, filter_=None, active_only=False):
|
||||
def get_all_courses(cls, orgs=None, filter_=None, active_only=False, course_keys=None):
|
||||
"""
|
||||
Return a queryset containing all CourseOverview objects in the database.
|
||||
|
||||
@@ -671,12 +671,17 @@ class CourseOverview(TimeStampedModel):
|
||||
filtering by organization.
|
||||
filter_ (dict): Optional parameter that allows custom filtering.
|
||||
active_only (bool): If provided, only the courses that have not ended will be returned.
|
||||
course_keys (list[string]): Optional parameter that allows case-insensitive
|
||||
filter by course ids
|
||||
"""
|
||||
# Note: If a newly created course is not returned in this QueryList,
|
||||
# make sure the "publish" signal was emitted when the course was
|
||||
# created. For tests using CourseFactory, use emit_signals=True.
|
||||
course_overviews = CourseOverview.objects.all()
|
||||
|
||||
if course_keys:
|
||||
course_overviews = course_overviews.filter(id__in=course_keys)
|
||||
|
||||
if orgs:
|
||||
# In rare cases, courses belonging to the same org may be accidentally assigned
|
||||
# an org code with a different casing (e.g., Harvardx as opposed to HarvardX).
|
||||
|
||||
Reference in New Issue
Block a user