Skip to content

Commit

Permalink
feat: list courses details by keys
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit b14cf0cc76928d3903eafcfda47dbefb44044a7a)
  • Loading branch information
yusuf-musleh committed Jul 5, 2023
1 parent 838f6ea commit 35d0905
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 10 deletions.
10 changes: 10 additions & 0 deletions docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2039,6 +2039,16 @@ paths:
provided org code (e.g., "HarvardX") are returned.
Case-insensitive.
permissions (optional):
If specified, it filters visible `CourseOverview` objects by
checking if each permission specified is granted for the username.
Notice that Staff users are always granted permission to list any
course.
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
Expand Down
14 changes: 10 additions & 4 deletions lms/djangoapps/branding/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers


def get_visible_courses(org=None, filter_=None):
def get_visible_courses(org=None, filter_=None, course_keys=None):
"""
Yield the CourseOverviews that should be visible in this branded
instance.
Expand All @@ -24,6 +24,8 @@ def get_visible_courses(org=None, filter_=None):
filtering by organization.
filter_ (dict): Optional parameter that allows custom filtering by
fields on the course.
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
Expand All @@ -35,12 +37,16 @@ def get_visible_courses(org=None, filter_=None):
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_)
courses = CourseOverview.get_all_courses(
orgs=[org], filter_=filter_, 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_)
courses = CourseOverview.get_all_courses(
orgs=current_site_orgs, filter_=filter_, course_keys=course_keys
)
else:
courses = CourseOverview.get_all_courses(filter_=filter_)
courses = CourseOverview.get_all_courses(filter_=filter_, course_keys=course_keys)

courses = courses.order_by('id')

Expand Down
10 changes: 8 additions & 2 deletions lms/djangoapps/course_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ def list_courses(request,
org=None,
filter_=None,
search_term=None,
permissions=None):
permissions=None,
course_keys=None):
"""
Yield all available courses.
Expand Down Expand Up @@ -142,12 +143,17 @@ def list_courses(request,
permissions (list[str]):
If specified, it filters visible `CourseOverview` objects by
checking if each permission specified is granted for the username.
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)
course_qs = get_courses(
user, org=org, filter_=filter_, permissions=permissions, course_keys=course_keys
)
course_qs = _filter_by_search(course_qs, search_term)
return course_qs

Expand Down
15 changes: 15 additions & 0 deletions lms/djangoapps/course_api/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
]
mobile = ExtendedNullBooleanField(required=False)
permissions = MultiValueField(required=False)
course_keys = MultiValueField(required=False)

def clean(self):
"""
Expand All @@ -79,6 +80,20 @@ def clean(self):

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):
"""
Expand Down
37 changes: 36 additions & 1 deletion lms/djangoapps/course_api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ def _make_api_call(self,
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`.
Expand All @@ -119,6 +120,7 @@ def _make_api_call(self,
org=org,
filter_=filter_,
permissions=permissions,
course_keys=course_keys,
)

def verify_courses(self, courses):
Expand Down Expand Up @@ -242,6 +244,39 @@ def test_permissions(self):

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):
"""
Expand Down
9 changes: 9 additions & 0 deletions lms/djangoapps/course_api/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def set_up_data(self, user):
'search_term': '',
'filter_': None,
'permissions': set(),
'course_keys': set(),
}

def test_basic(self):
Expand Down Expand Up @@ -99,6 +100,14 @@ def test_filter(self, param_field_name, param_field_value):

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
Expand Down
7 changes: 6 additions & 1 deletion lms/djangoapps/course_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
Notice that Staff users are always granted permission to list any
course.
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
Expand Down Expand Up @@ -333,7 +337,8 @@ def get_queryset(self):
org=form.cleaned_data['org'],
filter_=form.cleaned_data['filter_'],
search_term=form.cleaned_data['search_term'],
permissions=form.cleaned_data['permissions']
permissions=form.cleaned_data['permissions'],
course_keys=form.cleaned_data['course_keys'],
)


Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ def get_course_syllabus_section(course, section_key):


@function_trace('get_courses')
def get_courses(user, org=None, filter_=None, permissions=None):
def get_courses(user, org=None, filter_=None, permissions=None, 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
Expand All @@ -724,6 +724,7 @@ def get_courses(user, org=None, filter_=None, permissions=None):
courses = branding.get_visible_courses(
org=org,
filter_=filter_,
course_keys=course_keys
).prefetch_related(
'modes',
).select_related(
Expand Down
7 changes: 6 additions & 1 deletion openedx/core/djangoapps/content/course_overviews/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,20 +653,25 @@ def update_select_courses(cls, course_keys, force_update=False):
log.info('Finished generating course overviews.')

@classmethod
def get_all_courses(cls, orgs=None, filter_=None):
def get_all_courses(cls, orgs=None, filter_=None, course_keys=None):
"""
Return a queryset containing all CourseOverview objects in the database.
Arguments:
orgs (list[string]): Optional parameter that allows case-insensitive
filtering by organization.
filter_ (dict): Optional parameter that allows custom filtering.
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).
Expand Down

0 comments on commit 35d0905

Please sign in to comment.