Skip to content

Commit

Permalink
Merge pull request #32573 from open-craft/yusuf-musleh/list-courses-d…
Browse files Browse the repository at this point in the history
…etails-by-key

feat: list courses details by keys
  • Loading branch information
Feanil Patel authored Jul 12, 2023
2 parents cb3a0e9 + 4ad8ba1 commit f60735a
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 11 deletions.
4 changes: 4 additions & 0 deletions docs/lms-openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
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, 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.
Expand All @@ -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
Expand All @@ -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')

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 @@ -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.
Expand Down Expand Up @@ -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

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 @@ -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):
"""
Expand All @@ -80,6 +81,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 @@ -107,7 +107,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 @@ -121,6 +122,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 @@ -244,6 +246,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 @@ -71,6 +71,7 @@ def set_up_data(self, user):
'filter_': None,
'permissions': set(),
'active_only': None,
'course_keys': set(),
}

def test_basic(self):
Expand Down Expand Up @@ -100,6 +101,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 @@ -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
Expand Down Expand Up @@ -343,7 +347,8 @@ def get_queryset(self):
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'],
)


Expand Down
5 changes: 3 additions & 2 deletions lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
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 @@ -662,7 +662,7 @@ 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, 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.
Expand All @@ -671,12 +671,17 @@ def get_all_courses(cls, orgs=None, filter_=None, active_only=False):
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).
Expand Down

0 comments on commit f60735a

Please sign in to comment.