diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py
index 17244fb..d14489d 100644
--- a/lms/djangoapps/branding/__init__.py
+++ b/lms/djangoapps/branding/__init__.py
@@ -17,22 +17,32 @@ from django.contrib.staticfiles.storage import staticfiles_storage
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
-def get_visible_courses(org=None):
+def get_visible_courses(org=None, filter_=None):
"""
- Return the set of CourseOverviews that should be visible in this branded instance
+ Return the set of CourseOverviews that should be visible in this branded
+ instance.
+
+ Arguments:
+ org (string): Optional parameter that allows case-insensitive
+ filtering by organization.
+ filter_ (dict): Optional parameter that allows custom filtering by
+ fields on the course.
"""
microsite_org = microsite.get_value('course_org_filter')
if org and microsite_org:
# When called in the context of a microsite, return an empty result if the org
# passed by the caller does not match the designated microsite org.
- courses = CourseOverview.get_all_courses(org=org) if org == microsite_org else []
+ courses = CourseOverview.get_all_courses(
+ org=org,
+ filter_=filter_,
+ ) if org == microsite_org else []
else:
# We only make it to this point if one of org or microsite_org is defined.
# If both org and microsite_org were defined, the code would have fallen into the
# first branch of the conditional above, wherein an equality check is performed.
target_org = org or microsite_org
- courses = CourseOverview.get_all_courses(org=target_org)
+ courses = CourseOverview.get_all_courses(org=target_org, filter_=filter_)
courses = sorted(courses, key=lambda course: course.number)
diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py
index 1c6f932..cfe0207 100644
--- a/lms/djangoapps/course_api/api.py
+++ b/lms/djangoapps/course_api/api.py
@@ -52,7 +52,7 @@ def course_detail(request, username, course_key):
)
-def list_courses(request, username, org=None):
+def list_courses(request, username, org=None, filter_=None):
"""
Return a list of available courses.
@@ -73,9 +73,12 @@ def list_courses(request, username, org=None):
If specified, visible `CourseOverview` objects are filtered
such that only those belonging to the organization with the provided
org code (e.g., "HarvardX") are returned. Case-insensitive.
+ filter_ (dict):
+ If specified, visible `CourseOverview` objects are filtered
+ by the given key-value pairs.
Return value:
List of `CourseOverview` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
- return get_courses(user, org=org)
+ return get_courses(user, org=org, filter_=filter_)
diff --git a/lms/djangoapps/course_api/blocks/forms.py b/lms/djangoapps/course_api/blocks/forms.py
index ca258b3..111cd38 100644
--- a/lms/djangoapps/course_api/blocks/forms.py
+++ b/lms/djangoapps/course_api/blocks/forms.py
@@ -111,7 +111,7 @@ class BlockListGetForm(Form):
def clean(self):
"""
- Return cleanded data, including additional requested fields.
+ Return cleaned data, including additional requested fields.
"""
cleaned_data = super(BlockListGetForm, self).clean()
diff --git a/lms/djangoapps/course_api/forms.py b/lms/djangoapps/course_api/forms.py
new file mode 100644
index 0000000..aca01f7
--- /dev/null
+++ b/lms/djangoapps/course_api/forms.py
@@ -0,0 +1,76 @@
+"""
+Course API forms
+"""
+
+from collections import namedtuple
+from django.core.exceptions import ValidationError
+from django.forms import Form, CharField
+
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.keys import CourseKey
+from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField
+
+
+class UsernameValidatorMixin(object):
+ """
+ Mixin class for validating the username parameter.
+ """
+
+ def clean_username(self):
+ """
+ Ensures the username is provided unless the request is made
+ as an anonymous user.
+ """
+ username = self.cleaned_data.get('username')
+ if not username:
+ if not self.initial['requesting_user'].is_anonymous():
+ raise ValidationError("A username is required for non-anonymous access.")
+ return username or ''
+
+
+class CourseDetailGetForm(UsernameValidatorMixin, Form):
+ """
+ A form to validate query parameters in the course detail endpoint
+ """
+ username = CharField(required=False)
+ course_key = CharField(required=True)
+
+ def clean_course_key(self):
+ """
+ Ensure a valid `course_key` was provided.
+ """
+ course_key_string = self.cleaned_data['course_key']
+ try:
+ return CourseKey.from_string(course_key_string)
+ except InvalidKeyError:
+ raise ValidationError("'{}' is not a valid course key.".format(unicode(course_key_string)))
+
+
+class CourseListGetForm(UsernameValidatorMixin, Form):
+ """
+ A form to validate query parameters in the course list retrieval endpoint
+ """
+ username = CharField(required=False)
+ org = CharField(required=False)
+
+ # white list of all supported filter fields
+ filter_type = namedtuple('filter_type', ['param_name', 'field_name'])
+ supported_filters = [
+ filter_type(param_name='mobile', field_name='mobile_available'),
+ ]
+ mobile = ExtendedNullBooleanField(required=False)
+
+ def clean(self):
+ """
+ Return cleaned data, including additional filters.
+ """
+ cleaned_data = super(CourseListGetForm, self).clean()
+
+ # create a filter for all supported filter fields
+ filter_ = dict()
+ for supported_filter in self.supported_filters:
+ if cleaned_data.get(supported_filter.param_name) is not None:
+ filter_[supported_filter.field_name] = cleaned_data[supported_filter.param_name]
+ cleaned_data['filter_'] = filter_ or None
+
+ return cleaned_data
diff --git a/lms/djangoapps/course_api/serializers.py b/lms/djangoapps/course_api/serializers.py
index 6504e7d..d92e548 100644
--- a/lms/djangoapps/course_api/serializers.py
+++ b/lms/djangoapps/course_api/serializers.py
@@ -38,24 +38,20 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth
Serializer for Course objects
"""
+ blocks_url = serializers.SerializerMethodField()
course_id = serializers.CharField(source='id', read_only=True)
+ effort = serializers.CharField()
+ end = serializers.DateTimeField()
+ enrollment_start = serializers.DateTimeField()
+ enrollment_end = serializers.DateTimeField()
+ media = _CourseApiMediaCollectionSerializer(source='*')
name = serializers.CharField(source='display_name_with_default_escaped')
number = serializers.CharField(source='display_number_with_default')
org = serializers.CharField(source='display_org_with_default')
short_description = serializers.CharField()
- effort = serializers.CharField()
-
- media = _CourseApiMediaCollectionSerializer(source='*')
-
start = serializers.DateTimeField()
- start_type = serializers.CharField()
start_display = serializers.CharField()
- end = serializers.DateTimeField()
-
- enrollment_start = serializers.DateTimeField()
- enrollment_end = serializers.DateTimeField()
-
- blocks_url = serializers.SerializerMethodField()
+ start_type = serializers.CharField()
def get_blocks_url(self, course_overview):
"""
diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py
index 26ce210..5c447ec 100644
--- a/lms/djangoapps/course_api/tests/test_api.py
+++ b/lms/djangoapps/course_api/tests/test_api.py
@@ -87,7 +87,7 @@ class CourseListTestMixin(CourseApiTestMixin):
"""
Common behavior for list_courses tests
"""
- def _make_api_call(self, requesting_user, specified_user, org=None):
+ def _make_api_call(self, requesting_user, specified_user, org=None, filter_=None):
"""
Call the list_courses api endpoint to get information about
`specified_user` on behalf of `requesting_user`.
@@ -95,7 +95,7 @@ class CourseListTestMixin(CourseApiTestMixin):
request = Request(self.request_factory.get('/'))
request.user = requesting_user
with check_mongo_calls(0):
- return list_courses(request, specified_user.username, org=org)
+ return list_courses(request, specified_user.username, org=org, filter_=filter_)
def verify_courses(self, courses):
"""
@@ -173,6 +173,24 @@ class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase):
all(course.org == self.course.org for course in filtered_courses)
)
+ @SharedModuleStoreTestCase.modifies_courseware
+ def test_filter(self):
+ # Create a second course to be filtered out of queries.
+ alternate_course = self.create_course(course='mobile', mobile_available=True)
+
+ test_cases = [
+ (None, [alternate_course, self.course]),
+ (dict(mobile_available=True), [alternate_course]),
+ (dict(mobile_available=False), [self.course]),
+ ]
+ for filter_, expected_courses in test_cases:
+ filtered_courses = self._make_api_call(self.staff_user, self.staff_user, filter_=filter_)
+ self.assertEquals(
+ {course.id for course in filtered_courses},
+ {course.id for course in expected_courses},
+ "testing course_api.api.list_courses with filter_={}".format(filter_),
+ )
+
class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
"""
diff --git a/lms/djangoapps/course_api/tests/test_forms.py b/lms/djangoapps/course_api/tests/test_forms.py
new file mode 100644
index 0000000..2d3aa6f
--- /dev/null
+++ b/lms/djangoapps/course_api/tests/test_forms.py
@@ -0,0 +1,144 @@
+"""
+Tests for Course API forms.
+"""
+
+import ddt
+from django.contrib.auth.models import AnonymousUser
+from django.http import QueryDict
+from itertools import product
+from urllib import urlencode
+
+from openedx.core.djangoapps.util.test_forms import FormTestMixin
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+from ..forms import CourseDetailGetForm, CourseListGetForm
+
+
+class UsernameTestMixin(object):
+ """
+ Tests the username Form field.
+ """
+ def test_no_user_param_anonymous_access(self):
+ self.set_up_data(AnonymousUser())
+ self.form_data.pop('username')
+ self.assert_valid(self.cleaned_data)
+
+ def test_no_user_param(self):
+ self.form_data.pop('username')
+ self.assert_error('username', "A username is required for non-anonymous access.")
+
+
+@ddt.ddt
+class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase):
+ """
+ Tests for CourseListGetForm
+ """
+ FORM_CLASS = CourseListGetForm
+
+ @classmethod
+ def setUpClass(cls):
+ super(TestCourseListGetForm, cls).setUpClass()
+
+ cls.course = CourseFactory.create()
+
+ def setUp(self):
+ super(TestCourseListGetForm, self).setUp()
+
+ self.student = UserFactory.create()
+ self.set_up_data(self.student)
+
+ def set_up_data(self, user):
+ """
+ Sets up the initial form data and the expected clean data.
+ """
+ self.initial = {'requesting_user': user}
+ self.form_data = QueryDict(
+ urlencode({
+ 'username': user.username,
+ }),
+ mutable=True,
+ )
+ self.cleaned_data = {
+ 'username': user.username,
+ 'org': '',
+ 'mobile': None,
+ 'filter_': None,
+ }
+
+ def test_basic(self):
+ self.assert_valid(self.cleaned_data)
+
+ def test_org(self):
+ org_value = 'test org name'
+ self.form_data['org'] = org_value
+ self.cleaned_data['org'] = org_value
+ self.assert_valid(self.cleaned_data)
+
+ @ddt.data(
+ *product(
+ [('mobile', 'mobile_available')],
+ [(True, True), (False, False), ('1', True), ('0', False), (None, None)],
+ )
+ )
+ @ddt.unpack
+ def test_filter(self, param_field_name, param_field_value):
+ param_name, field_name = param_field_name
+ param_value, field_value = param_field_value
+
+ self.form_data[param_name] = param_value
+ self.cleaned_data[param_name] = field_value
+ if field_value is not None:
+ self.cleaned_data['filter_'] = {field_name: field_value}
+
+ self.assert_valid(self.cleaned_data)
+
+
+class TestCourseDetailGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase):
+ """
+ Tests for CourseDetailGetForm
+ """
+ FORM_CLASS = CourseDetailGetForm
+
+ @classmethod
+ def setUpClass(cls):
+ super(TestCourseDetailGetForm, cls).setUpClass()
+
+ cls.course = CourseFactory.create()
+
+ def setUp(self):
+ super(TestCourseDetailGetForm, self).setUp()
+
+ self.student = UserFactory.create()
+ self.set_up_data(self.student)
+
+ def set_up_data(self, user):
+ """
+ Sets up the initial form data and the expected clean data.
+ """
+ self.initial = {'requesting_user': user}
+ self.form_data = QueryDict(
+ urlencode({
+ 'username': user.username,
+ 'course_key': unicode(self.course.id),
+ }),
+ mutable=True,
+ )
+ self.cleaned_data = {
+ 'username': user.username,
+ 'course_key': self.course.id,
+ }
+
+ def test_basic(self):
+ self.assert_valid(self.cleaned_data)
+
+ #-- course key --#
+
+ def test_no_course_key_param(self):
+ self.form_data.pop('course_key')
+ self.assert_error('course_key', "This field is required.")
+
+ def test_invalid_course_key(self):
+ self.form_data['course_key'] = 'invalid_course_key'
+ self.assert_error('course_key', "'invalid_course_key' is not a valid course key.")
diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py
index 881ae68..166ece1 100644
--- a/lms/djangoapps/course_api/tests/test_views.py
+++ b/lms/djangoapps/course_api/tests/test_views.py
@@ -16,12 +16,14 @@ class CourseApiTestViewMixin(CourseApiFactoryMixin):
Mixin class for test helpers for Course API views
"""
- def setup_user(self, requesting_user):
+ def setup_user(self, requesting_user, make_inactive=False):
"""
- log in the specified user, and remember it as `self.user`
+ log in the specified user and set its is_active field
"""
- self.user = requesting_user # pylint: disable=attribute-defined-outside-init
- self.client.login(username=self.user.username, password=TEST_PASSWORD)
+ self.assertTrue(self.client.login(username=requesting_user.username, password=TEST_PASSWORD))
+ if make_inactive:
+ requesting_user.is_active = False
+ requesting_user.save()
def verify_response(self, expected_status_code=200, params=None, url=None):
"""
@@ -59,7 +61,7 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
def test_as_staff(self):
self.setup_user(self.staff_user)
- self.verify_response()
+ self.verify_response(params={'username': self.staff_user.username})
def test_as_staff_for_honor(self):
self.setup_user(self.staff_user)
@@ -67,7 +69,7 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
def test_as_honor(self):
self.setup_user(self.honor_user)
- self.verify_response()
+ self.verify_response(params={'username': self.honor_user.username})
def test_as_honor_for_explicit_self(self):
self.setup_user(self.honor_user)
@@ -77,6 +79,15 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
self.setup_user(self.honor_user)
self.verify_response(expected_status_code=403, params={'username': self.staff_user.username})
+ def test_as_inactive_user(self):
+ inactive_user = self.create_user(username='inactive', is_staff=False)
+ self.setup_user(inactive_user, make_inactive=True)
+ self.verify_response(params={'username': inactive_user.username})
+
+ def test_missing_username(self):
+ self.setup_user(self.honor_user)
+ self.verify_response(expected_status_code=400)
+
@SharedModuleStoreTestCase.modifies_courseware
def test_filter_by_org(self):
"""Verify that CourseOverviews are filtered by the provided org key."""
@@ -90,18 +101,41 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
self.assertNotEqual(alternate_course.org, self.course.org)
# No filtering.
- unfiltered_response = self.verify_response()
+ unfiltered_response = self.verify_response(params={'username': self.staff_user.username})
for org in [self.course.org, alternate_course.org]:
self.assertTrue(
any(course['org'] == org for course in unfiltered_response.data['results']) # pylint: disable=no-member
)
# With filtering.
- filtered_response = self.verify_response(params={'org': self.course.org})
+ filtered_response = self.verify_response(params={'org': self.course.org, 'username': self.staff_user.username})
self.assertTrue(
all(course['org'] == self.course.org for course in filtered_response.data['results']) # pylint: disable=no-member
)
+ @SharedModuleStoreTestCase.modifies_courseware
+ def test_filter(self):
+ self.setup_user(self.staff_user)
+
+ # Create a second course to be filtered out of queries.
+ alternate_course = self.create_course(course='mobile', mobile_available=True)
+
+ test_cases = [
+ (None, [alternate_course, self.course]),
+ (dict(mobile=True), [alternate_course]),
+ (dict(mobile=False), [self.course]),
+ ]
+ for filter_, expected_courses in test_cases:
+ params = {'username': self.staff_user.username}
+ if filter_:
+ params.update(filter_)
+ response = self.verify_response(params=params)
+ self.assertEquals(
+ {course['course_id'] for course in response.data['results']}, # pylint: disable=no-member
+ {unicode(course.id) for course in expected_courses},
+ "testing course_api.views.CourseListView with filter_={}".format(filter_),
+ )
+
def test_not_logged_in(self):
self.client.logout()
self.verify_response()
@@ -125,10 +159,6 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
def test_as_honor(self):
self.setup_user(self.honor_user)
- self.verify_response()
-
- def test_as_honor_for_explicit_self(self):
- self.setup_user(self.honor_user)
self.verify_response(params={'username': self.honor_user.username})
def test_as_honor_for_staff(self):
@@ -137,7 +167,7 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
def test_as_staff(self):
self.setup_user(self.staff_user)
- self.verify_response()
+ self.verify_response(params={'username': self.staff_user.username})
def test_as_staff_for_honor(self):
self.setup_user(self.staff_user)
@@ -146,17 +176,26 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
def test_as_anonymous_user(self):
self.verify_response(expected_status_code=200)
+ def test_as_inactive_user(self):
+ inactive_user = self.create_user(username='inactive', is_staff=False)
+ self.setup_user(inactive_user, make_inactive=True)
+ self.verify_response(params={'username': inactive_user.username})
+
def test_hidden_course_as_honor(self):
self.setup_user(self.honor_user)
- self.verify_response(expected_status_code=404, url=self.hidden_url)
+ self.verify_response(
+ expected_status_code=404, url=self.hidden_url, params={'username': self.honor_user.username}
+ )
def test_hidden_course_as_staff(self):
self.setup_user(self.staff_user)
- self.verify_response(url=self.hidden_url)
+ self.verify_response(url=self.hidden_url, params={'username': self.staff_user.username})
def test_nonexistent_course(self):
self.setup_user(self.staff_user)
- self.verify_response(expected_status_code=404, url=self.nonexistent_url)
+ self.verify_response(
+ expected_status_code=404, url=self.nonexistent_url, params={'username': self.staff_user.username}
+ )
def test_invalid_course_key(self):
# Our URL patterns try to block invalid course keys. If one got
@@ -166,4 +205,4 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
request.query_params = {}
request.user = self.staff_user
response = CourseDetailView().dispatch(request, course_key_string='a:b:c')
- self.assertEqual(404, response.status_code)
+ self.assertEquals(response.status_code, 400)
diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py
index 002c62d..705b9c5 100644
--- a/lms/djangoapps/course_api/views.py
+++ b/lms/djangoapps/course_api/views.py
@@ -2,17 +2,18 @@
Course API Views
"""
-from django.http import Http404
-from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import CourseKey
+from django.core.exceptions import ValidationError
from rest_framework.generics import ListAPIView, RetrieveAPIView
from openedx.core.lib.api.paginators import NamespacedPageNumberPagination
+from openedx.core.lib.api.view_utils import view_auth_classes, DeveloperErrorViewMixin
from .api import course_detail, list_courses
+from .forms import CourseDetailGetForm, CourseListGetForm
from .serializers import CourseSerializer
-class CourseDetailView(RetrieveAPIView):
+@view_auth_classes(is_authenticated=False)
+class CourseDetailView(DeveloperErrorViewMixin, RetrieveAPIView):
"""
**Use Cases**
@@ -27,21 +28,20 @@ class CourseDetailView(RetrieveAPIView):
Body consists of the following fields:
* blocks_url: used to fetch the course blocks
+ * course_id: Course key
+ * effort: A textual description of the weekly hours of effort expected
+ in the course.
+ * end: Date the course ends
+ * enrollment_end: Date enrollment ends
+ * enrollment_start: Date enrollment begins
* media: An object that contains named media items. Included here:
* course_image: An image to show for the course. Represented
as an object with the following fields:
* uri: The location of the image
- * name:
- * description:
- * type:
- * end: Date the course ends
- * enrollment_end: Date enrollment ends
- * enrollment_start: Date enrollment begins
- * course_id: Course key
* name: Name of the course
* number: Catalog number of the course
* org: Name of the organization that owns the course
- * description: A textual description of the course
+ * short_description: A textual description of the course
* start: Date the course begins
* start_display: Readably formatted start of the course
* start_type: Hint describing how `start_display` is set. One of:
@@ -52,12 +52,15 @@ class CourseDetailView(RetrieveAPIView):
**Parameters:**
username (optional):
- The username of the specified user whose visible courses we
- want to see. Defaults to the current user.
+ The username of the specified user for whom the course data
+ is being accessed. The username is not only required if the API is
+ requested by an Anonymous user.
**Returns**
* 200 on success with above fields.
+ * 400 if an invalid parameter was sent or the username was not provided
+ for an authenticated request.
* 403 if a user who does not have permission to masquerade as
another user specifies a username other than their own.
* 404 if the course is not available or cannot be seen.
@@ -76,7 +79,7 @@ class CourseDetailView(RetrieveAPIView):
"end": "2015-09-19T18:00:00Z",
"enrollment_end": "2015-07-15T00:00:00Z",
"enrollment_start": "2015-06-15T00:00:00Z",
- "id": "edX/example/2012_Fall",
+ "course_id": "edX/example/2012_Fall",
"name": "Example Course",
"number": "example",
"org": "edX",
@@ -87,25 +90,27 @@ class CourseDetailView(RetrieveAPIView):
"""
serializer_class = CourseSerializer
- lookup_url_kwarg = 'course_key_string'
def get_object(self):
"""
Return the requested course object, if the user has appropriate
permissions.
"""
+ requested_params = self.request.query_params.copy()
+ requested_params.update({'course_key': self.kwargs['course_key_string']})
+ form = CourseDetailGetForm(requested_params, initial={'requesting_user': self.request.user})
+ if not form.is_valid():
+ raise ValidationError(form.errors)
- username = self.request.query_params.get('username', self.request.user.username)
- course_key_string = self.kwargs[self.lookup_url_kwarg]
- try:
- course_key = CourseKey.from_string(course_key_string)
- except InvalidKeyError:
- raise Http404()
+ return course_detail(
+ self.request,
+ form.cleaned_data['username'],
+ form.cleaned_data['course_key'],
+ )
- return course_detail(self.request, username, course_key)
-
-class CourseListView(ListAPIView):
+@view_auth_classes(is_authenticated=False)
+class CourseListView(DeveloperErrorViewMixin, ListAPIView):
"""
**Use Cases**
@@ -123,17 +128,25 @@ class CourseListView(ListAPIView):
username (optional):
The username of the specified user whose visible courses we
- want to see. Defaults to the current user.
+ want to see. The username is not required only if the API is
+ requested by an Anonymous user.
org (optional):
If specified, visible `CourseOverview` objects are filtered
- such that only those belonging to the organization with the provided
- org code (e.g., "HarvardX") are returned. Case-insensitive.
+ such that only those belonging to the organization with the
+ provided org code (e.g., "HarvardX") are returned.
+ Case-insensitive.
+
+ mobile (optional):
+ If specified, only visible `CourseOverview` objects that are
+ designated as mobile_available are returned.
**Returns**
* 200 on success, with a list of course discovery objects as returned
by `CourseDetailView`.
+ * 400 if an invalid parameter was sent or the username was not provided
+ for an authenticated request.
* 403 if a user who does not have permission to masquerade as
another user specifies a username other than their own.
* 404 if the specified user does not exist, or the requesting user does
@@ -154,7 +167,7 @@ class CourseListView(ListAPIView):
"end": "2015-09-19T18:00:00Z",
"enrollment_end": "2015-07-15T00:00:00Z",
"enrollment_start": "2015-06-15T00:00:00Z",
- "id": "edX/example/2012_Fall",
+ "course_id": "edX/example/2012_Fall",
"name": "Example Course",
"number": "example",
"org": "edX",
@@ -172,7 +185,13 @@ class CourseListView(ListAPIView):
"""
Return a list of courses visible to the user.
"""
- username = self.request.query_params.get('username', self.request.user.username)
- org = self.request.query_params.get('org')
-
- return list_courses(self.request, username, org=org)
+ form = CourseListGetForm(self.request.query_params, initial={'requesting_user': self.request.user})
+ if not form.is_valid():
+ raise ValidationError(form.errors)
+
+ return list_courses(
+ self.request,
+ form.cleaned_data['username'],
+ org=form.cleaned_data['org'],
+ filter_=form.cleaned_data['filter_'],
+ )
diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py
index ae8852d..abaee4b 100644
--- a/lms/djangoapps/courseware/courses.py
+++ b/lms/djangoapps/courseware/courses.py
@@ -373,12 +373,12 @@ def get_course_syllabus_section(course, section_key):
raise KeyError("Invalid about key " + str(section_key))
-def get_courses(user, org=None):
+def get_courses(user, org=None, filter_=None):
"""
Returns a list of courses available, sorted by course.number and optionally
filtered by org code (case-insensitive).
"""
- courses = branding.get_visible_courses(org=org)
+ courses = branding.get_visible_courses(org=org, filter_=filter_)
permission_name = microsite.get_value(
'COURSE_CATALOG_VISIBILITY_PERMISSION',
diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py
index 858576b..c7f8bab 100644
--- a/lms/djangoapps/courseware/tests/test_courses.py
+++ b/lms/djangoapps/courseware/tests/test_courses.py
@@ -84,7 +84,7 @@ class CoursesTest(ModuleStoreTestCase):
with check_mongo_calls(num_mongo_calls):
course_access_func(user, 'load', course.id)
- def test_get_courses(self):
+ def test_get_courses_by_org(self):
"""
Verify that org filtering performs as expected, and that an empty result
is returned if the org passed by the caller does not match the designated
@@ -132,6 +132,30 @@ class CoursesTest(ModuleStoreTestCase):
all(course.org == alternate_course.org for course in microsite_courses)
)
+ def test_get_courses_with_filter(self):
+ """
+ Verify that filtering performs as expected.
+ """
+ user = UserFactory.create()
+ non_mobile_course = CourseFactory.create(emit_signals=True)
+ mobile_course = CourseFactory.create(mobile_available=True, emit_signals=True)
+
+ test_cases = (
+ (None, {non_mobile_course.id, mobile_course.id}),
+ (dict(mobile_available=True), {mobile_course.id}),
+ (dict(mobile_available=False), {non_mobile_course.id}),
+ )
+ for filter_, expected_courses in test_cases:
+ self.assertEqual(
+ {
+ course.id
+ for course in
+ get_courses(user, filter_=filter_)
+ },
+ expected_courses,
+ "testing get_courses with filter_={}".format(filter_),
+ )
+
@attr('shard_1')
class ModuleStoreBranchSettingTest(ModuleStoreTestCase):
diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index 9ebd101..bf0747f 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -470,13 +470,14 @@ class CourseOverview(TimeStampedModel):
return course_overviews
@classmethod
- def get_all_courses(cls, org=None):
+ def get_all_courses(cls, org=None, filter_=None):
"""
Returns all CourseOverview objects in the database.
Arguments:
- org (string): Optional parameter that allows filtering
- by organization.
+ org (string): Optional parameter that allows case-insensitive
+ filtering by organization.
+ filter_ (dict): Optional parameter that allows custom filtering.
"""
# Note: If a newly created course is not returned in this QueryList,
# make sure the "publish" signal was emitted when the course was
@@ -489,6 +490,9 @@ class CourseOverview(TimeStampedModel):
# Case-insensitive exact matching allows us to deal with this kind of dirty data.
course_overviews = course_overviews.filter(org__iexact=org)
+ if filter_:
+ course_overviews = course_overviews.filter(**filter_)
+
return course_overviews
@classmethod
diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py
index 1afb491..02a426c 100644
--- a/openedx/core/djangoapps/content/course_overviews/tests.py
+++ b/openedx/core/djangoapps/content/course_overviews/tests.py
@@ -826,3 +826,24 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
}
)
return course_overview
+
+ def test_get_all_courses_by_mobile_available(self):
+ non_mobile_course = CourseFactory.create(emit_signals=True)
+ mobile_course = CourseFactory.create(mobile_available=True, emit_signals=True)
+
+ test_cases = (
+ (None, {non_mobile_course.id, mobile_course.id}),
+ (dict(mobile_available=True), {mobile_course.id}),
+ (dict(mobile_available=False), {non_mobile_course.id}),
+ )
+
+ for filter_, expected_courses in test_cases:
+ self.assertEqual(
+ {
+ course_overview.id
+ for course_overview in
+ CourseOverview.get_all_courses(filter_=filter_)
+ },
+ expected_courses,
+ "testing CourseOverview.get_all_courses with filter_={}".format(filter_),
+ )
diff --git a/openedx/core/djangoapps/util/test_forms.py b/openedx/core/djangoapps/util/test_forms.py
index 818d76c..391ec6c 100644
--- a/openedx/core/djangoapps/util/test_forms.py
+++ b/openedx/core/djangoapps/util/test_forms.py
@@ -23,6 +23,15 @@ class FormTestMixin(object):
form = self.get_form(expected_valid=False)
self.assertEqual(form.errors, {expected_field: [expected_message]})
+ def assert_valid(self, expected_cleaned_data):
+ """
+ Check that the form returns the expected data
+ """
+ form = self.get_form(expected_valid=True)
+ print 'form.cleaned_data: ' + unicode(form.cleaned_data)
+ print 'expected_cleaned_data: ' + unicode(expected_cleaned_data)
+ self.assertDictEqual(form.cleaned_data, expected_cleaned_data)
+
def assert_field_value(self, field, expected_value):
"""
Create a form bound to self.form_data, assert its validity, and assert
diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py
index 8183e25..ad89211 100644
--- a/openedx/core/lib/api/view_utils.py
+++ b/openedx/core/lib/api/view_utils.py
@@ -116,7 +116,7 @@ def view_course_access(depth=0, access_action='load', check_for_milestones=False
return _decorator
-def view_auth_classes(is_user=False):
+def view_auth_classes(is_user=False, is_authenticated=True):
"""
Function and class decorator that abstracts the authentication and permission checks for api views.
"""
@@ -129,7 +129,9 @@ def view_auth_classes(is_user=False):
OAuth2AuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser
)
- func_or_class.permission_classes = (IsAuthenticated,)
+ func_or_class.permission_classes = ()
+ if is_authenticated:
+ func_or_class.permission_classes += (IsAuthenticated,)
if is_user:
func_or_class.permission_classes += (IsUserInUrl,)
return func_or_class