Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: list courses details by keys #553

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2084,6 +2084,21 @@ 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.

active_only (optional):
If this boolean is specified, only the courses that have not ended or do not have any end
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 @@ -657,7 +657,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 @@ -666,12 +666,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
Loading