Commit 8e3f4e05 by Nimisha Asthagiri

Update Course Catalog API to support filters

parent 766c3dca
...@@ -17,22 +17,32 @@ from django.contrib.staticfiles.storage import staticfiles_storage ...@@ -17,22 +17,32 @@ from django.contrib.staticfiles.storage import staticfiles_storage
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview 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') microsite_org = microsite.get_value('course_org_filter')
if org and microsite_org: if org and microsite_org:
# When called in the context of a microsite, return an empty result if the 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. # 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: else:
# We only make it to this point if one of org or microsite_org is defined. # 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 # 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. # first branch of the conditional above, wherein an equality check is performed.
target_org = org or microsite_org 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) courses = sorted(courses, key=lambda course: course.number)
......
...@@ -52,7 +52,7 @@ def course_detail(request, username, course_key): ...@@ -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. Return a list of available courses.
...@@ -73,9 +73,12 @@ def list_courses(request, username, org=None): ...@@ -73,9 +73,12 @@ def list_courses(request, username, org=None):
If specified, visible `CourseOverview` objects are filtered If specified, visible `CourseOverview` objects are filtered
such that only those belonging to the organization with the provided such that only those belonging to the organization with the provided
org code (e.g., "HarvardX") are returned. Case-insensitive. 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: Return value:
List of `CourseOverview` objects representing the collection of courses. List of `CourseOverview` objects representing the collection of courses.
""" """
user = get_effective_user(request.user, username) user = get_effective_user(request.user, username)
return get_courses(user, org=org) return get_courses(user, org=org, filter_=filter_)
...@@ -111,7 +111,7 @@ class BlockListGetForm(Form): ...@@ -111,7 +111,7 @@ class BlockListGetForm(Form):
def clean(self): def clean(self):
""" """
Return cleanded data, including additional requested fields. Return cleaned data, including additional requested fields.
""" """
cleaned_data = super(BlockListGetForm, self).clean() cleaned_data = super(BlockListGetForm, self).clean()
......
"""
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
...@@ -38,24 +38,20 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth ...@@ -38,24 +38,20 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth
Serializer for Course objects Serializer for Course objects
""" """
blocks_url = serializers.SerializerMethodField()
course_id = serializers.CharField(source='id', read_only=True) 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') name = serializers.CharField(source='display_name_with_default_escaped')
number = serializers.CharField(source='display_number_with_default') number = serializers.CharField(source='display_number_with_default')
org = serializers.CharField(source='display_org_with_default') org = serializers.CharField(source='display_org_with_default')
short_description = serializers.CharField() short_description = serializers.CharField()
effort = serializers.CharField()
media = _CourseApiMediaCollectionSerializer(source='*')
start = serializers.DateTimeField() start = serializers.DateTimeField()
start_type = serializers.CharField()
start_display = serializers.CharField() start_display = serializers.CharField()
end = serializers.DateTimeField() start_type = serializers.CharField()
enrollment_start = serializers.DateTimeField()
enrollment_end = serializers.DateTimeField()
blocks_url = serializers.SerializerMethodField()
def get_blocks_url(self, course_overview): def get_blocks_url(self, course_overview):
""" """
......
...@@ -87,7 +87,7 @@ class CourseListTestMixin(CourseApiTestMixin): ...@@ -87,7 +87,7 @@ class CourseListTestMixin(CourseApiTestMixin):
""" """
Common behavior for list_courses tests 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 Call the list_courses api endpoint to get information about
`specified_user` on behalf of `requesting_user`. `specified_user` on behalf of `requesting_user`.
...@@ -95,7 +95,7 @@ class CourseListTestMixin(CourseApiTestMixin): ...@@ -95,7 +95,7 @@ class CourseListTestMixin(CourseApiTestMixin):
request = Request(self.request_factory.get('/')) request = Request(self.request_factory.get('/'))
request.user = requesting_user request.user = requesting_user
with check_mongo_calls(0): 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): def verify_courses(self, courses):
""" """
...@@ -173,6 +173,24 @@ class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase): ...@@ -173,6 +173,24 @@ class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase):
all(course.org == self.course.org for course in filtered_courses) 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): class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
""" """
......
"""
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.")
...@@ -16,12 +16,14 @@ class CourseApiTestViewMixin(CourseApiFactoryMixin): ...@@ -16,12 +16,14 @@ class CourseApiTestViewMixin(CourseApiFactoryMixin):
Mixin class for test helpers for Course API views 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.assertTrue(self.client.login(username=requesting_user.username, password=TEST_PASSWORD))
self.client.login(username=self.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): def verify_response(self, expected_status_code=200, params=None, url=None):
""" """
...@@ -59,7 +61,7 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): ...@@ -59,7 +61,7 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
def test_as_staff(self): def test_as_staff(self):
self.setup_user(self.staff_user) 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): def test_as_staff_for_honor(self):
self.setup_user(self.staff_user) self.setup_user(self.staff_user)
...@@ -67,7 +69,7 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): ...@@ -67,7 +69,7 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
def test_as_honor(self): def test_as_honor(self):
self.setup_user(self.honor_user) 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): def test_as_honor_for_explicit_self(self):
self.setup_user(self.honor_user) self.setup_user(self.honor_user)
...@@ -77,6 +79,15 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): ...@@ -77,6 +79,15 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
self.setup_user(self.honor_user) self.setup_user(self.honor_user)
self.verify_response(expected_status_code=403, params={'username': self.staff_user.username}) 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 @SharedModuleStoreTestCase.modifies_courseware
def test_filter_by_org(self): def test_filter_by_org(self):
"""Verify that CourseOverviews are filtered by the provided org key.""" """Verify that CourseOverviews are filtered by the provided org key."""
...@@ -90,18 +101,41 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): ...@@ -90,18 +101,41 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
self.assertNotEqual(alternate_course.org, self.course.org) self.assertNotEqual(alternate_course.org, self.course.org)
# No filtering. # 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]: for org in [self.course.org, alternate_course.org]:
self.assertTrue( self.assertTrue(
any(course['org'] == org for course in unfiltered_response.data['results']) # pylint: disable=no-member any(course['org'] == org for course in unfiltered_response.data['results']) # pylint: disable=no-member
) )
# With filtering. # 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( self.assertTrue(
all(course['org'] == self.course.org for course in filtered_response.data['results']) # pylint: disable=no-member 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): def test_not_logged_in(self):
self.client.logout() self.client.logout()
self.verify_response() self.verify_response()
...@@ -125,10 +159,6 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase ...@@ -125,10 +159,6 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
def test_as_honor(self): def test_as_honor(self):
self.setup_user(self.honor_user) 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}) self.verify_response(params={'username': self.honor_user.username})
def test_as_honor_for_staff(self): def test_as_honor_for_staff(self):
...@@ -137,7 +167,7 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase ...@@ -137,7 +167,7 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
def test_as_staff(self): def test_as_staff(self):
self.setup_user(self.staff_user) 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): def test_as_staff_for_honor(self):
self.setup_user(self.staff_user) self.setup_user(self.staff_user)
...@@ -146,17 +176,26 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase ...@@ -146,17 +176,26 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
def test_as_anonymous_user(self): def test_as_anonymous_user(self):
self.verify_response(expected_status_code=200) 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): def test_hidden_course_as_honor(self):
self.setup_user(self.honor_user) 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): def test_hidden_course_as_staff(self):
self.setup_user(self.staff_user) 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): def test_nonexistent_course(self):
self.setup_user(self.staff_user) 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): def test_invalid_course_key(self):
# Our URL patterns try to block invalid course keys. If one got # Our URL patterns try to block invalid course keys. If one got
...@@ -166,4 +205,4 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase ...@@ -166,4 +205,4 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
request.query_params = {} request.query_params = {}
request.user = self.staff_user request.user = self.staff_user
response = CourseDetailView().dispatch(request, course_key_string='a:b:c') response = CourseDetailView().dispatch(request, course_key_string='a:b:c')
self.assertEqual(404, response.status_code) self.assertEquals(response.status_code, 400)
...@@ -2,17 +2,18 @@ ...@@ -2,17 +2,18 @@
Course API Views Course API Views
""" """
from django.http import Http404 from django.core.exceptions import ValidationError
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from rest_framework.generics import ListAPIView, RetrieveAPIView from rest_framework.generics import ListAPIView, RetrieveAPIView
from openedx.core.lib.api.paginators import NamespacedPageNumberPagination 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 .api import course_detail, list_courses
from .forms import CourseDetailGetForm, CourseListGetForm
from .serializers import CourseSerializer from .serializers import CourseSerializer
class CourseDetailView(RetrieveAPIView): @view_auth_classes(is_authenticated=False)
class CourseDetailView(DeveloperErrorViewMixin, RetrieveAPIView):
""" """
**Use Cases** **Use Cases**
...@@ -27,21 +28,20 @@ class CourseDetailView(RetrieveAPIView): ...@@ -27,21 +28,20 @@ class CourseDetailView(RetrieveAPIView):
Body consists of the following fields: Body consists of the following fields:
* blocks_url: used to fetch the course blocks * 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: * media: An object that contains named media items. Included here:
* course_image: An image to show for the course. Represented * course_image: An image to show for the course. Represented
as an object with the following fields: as an object with the following fields:
* uri: The location of the image * 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 * name: Name of the course
* number: Catalog number of the course * number: Catalog number of the course
* org: Name of the organization that owns 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: Date the course begins
* start_display: Readably formatted start of the course * start_display: Readably formatted start of the course
* start_type: Hint describing how `start_display` is set. One of: * start_type: Hint describing how `start_display` is set. One of:
...@@ -52,12 +52,15 @@ class CourseDetailView(RetrieveAPIView): ...@@ -52,12 +52,15 @@ class CourseDetailView(RetrieveAPIView):
**Parameters:** **Parameters:**
username (optional): username (optional):
The username of the specified user whose visible courses we The username of the specified user for whom the course data
want to see. Defaults to the current user. is being accessed. The username is not only required if the API is
requested by an Anonymous user.
**Returns** **Returns**
* 200 on success with above fields. * 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 * 403 if a user who does not have permission to masquerade as
another user specifies a username other than their own. another user specifies a username other than their own.
* 404 if the course is not available or cannot be seen. * 404 if the course is not available or cannot be seen.
...@@ -76,7 +79,7 @@ class CourseDetailView(RetrieveAPIView): ...@@ -76,7 +79,7 @@ class CourseDetailView(RetrieveAPIView):
"end": "2015-09-19T18:00:00Z", "end": "2015-09-19T18:00:00Z",
"enrollment_end": "2015-07-15T00:00:00Z", "enrollment_end": "2015-07-15T00:00:00Z",
"enrollment_start": "2015-06-15T00:00:00Z", "enrollment_start": "2015-06-15T00:00:00Z",
"id": "edX/example/2012_Fall", "course_id": "edX/example/2012_Fall",
"name": "Example Course", "name": "Example Course",
"number": "example", "number": "example",
"org": "edX", "org": "edX",
...@@ -87,25 +90,27 @@ class CourseDetailView(RetrieveAPIView): ...@@ -87,25 +90,27 @@ class CourseDetailView(RetrieveAPIView):
""" """
serializer_class = CourseSerializer serializer_class = CourseSerializer
lookup_url_kwarg = 'course_key_string'
def get_object(self): def get_object(self):
""" """
Return the requested course object, if the user has appropriate Return the requested course object, if the user has appropriate
permissions. 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) return course_detail(
course_key_string = self.kwargs[self.lookup_url_kwarg] self.request,
try: form.cleaned_data['username'],
course_key = CourseKey.from_string(course_key_string) form.cleaned_data['course_key'],
except InvalidKeyError: )
raise Http404()
return course_detail(self.request, username, course_key)
@view_auth_classes(is_authenticated=False)
class CourseListView(ListAPIView): class CourseListView(DeveloperErrorViewMixin, ListAPIView):
""" """
**Use Cases** **Use Cases**
...@@ -123,17 +128,25 @@ class CourseListView(ListAPIView): ...@@ -123,17 +128,25 @@ class CourseListView(ListAPIView):
username (optional): username (optional):
The username of the specified user whose visible courses we 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): org (optional):
If specified, visible `CourseOverview` objects are filtered If specified, visible `CourseOverview` objects are filtered
such that only those belonging to the organization with the provided such that only those belonging to the organization with the
org code (e.g., "HarvardX") are returned. Case-insensitive. 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** **Returns**
* 200 on success, with a list of course discovery objects as returned * 200 on success, with a list of course discovery objects as returned
by `CourseDetailView`. 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 * 403 if a user who does not have permission to masquerade as
another user specifies a username other than their own. another user specifies a username other than their own.
* 404 if the specified user does not exist, or the requesting user does * 404 if the specified user does not exist, or the requesting user does
...@@ -154,7 +167,7 @@ class CourseListView(ListAPIView): ...@@ -154,7 +167,7 @@ class CourseListView(ListAPIView):
"end": "2015-09-19T18:00:00Z", "end": "2015-09-19T18:00:00Z",
"enrollment_end": "2015-07-15T00:00:00Z", "enrollment_end": "2015-07-15T00:00:00Z",
"enrollment_start": "2015-06-15T00:00:00Z", "enrollment_start": "2015-06-15T00:00:00Z",
"id": "edX/example/2012_Fall", "course_id": "edX/example/2012_Fall",
"name": "Example Course", "name": "Example Course",
"number": "example", "number": "example",
"org": "edX", "org": "edX",
...@@ -172,7 +185,13 @@ class CourseListView(ListAPIView): ...@@ -172,7 +185,13 @@ class CourseListView(ListAPIView):
""" """
Return a list of courses visible to the user. Return a list of courses visible to the user.
""" """
username = self.request.query_params.get('username', self.request.user.username) form = CourseListGetForm(self.request.query_params, initial={'requesting_user': self.request.user})
org = self.request.query_params.get('org') if not form.is_valid():
raise ValidationError(form.errors)
return list_courses(self.request, username, org=org)
return list_courses(
self.request,
form.cleaned_data['username'],
org=form.cleaned_data['org'],
filter_=form.cleaned_data['filter_'],
)
...@@ -373,12 +373,12 @@ def get_course_syllabus_section(course, section_key): ...@@ -373,12 +373,12 @@ def get_course_syllabus_section(course, section_key):
raise KeyError("Invalid about key " + str(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 Returns a list of courses available, sorted by course.number and optionally
filtered by org code (case-insensitive). 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( permission_name = microsite.get_value(
'COURSE_CATALOG_VISIBILITY_PERMISSION', 'COURSE_CATALOG_VISIBILITY_PERMISSION',
......
...@@ -84,7 +84,7 @@ class CoursesTest(ModuleStoreTestCase): ...@@ -84,7 +84,7 @@ class CoursesTest(ModuleStoreTestCase):
with check_mongo_calls(num_mongo_calls): with check_mongo_calls(num_mongo_calls):
course_access_func(user, 'load', course.id) 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 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 is returned if the org passed by the caller does not match the designated
...@@ -132,6 +132,30 @@ class CoursesTest(ModuleStoreTestCase): ...@@ -132,6 +132,30 @@ class CoursesTest(ModuleStoreTestCase):
all(course.org == alternate_course.org for course in microsite_courses) 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') @attr('shard_1')
class ModuleStoreBranchSettingTest(ModuleStoreTestCase): class ModuleStoreBranchSettingTest(ModuleStoreTestCase):
......
...@@ -470,13 +470,14 @@ class CourseOverview(TimeStampedModel): ...@@ -470,13 +470,14 @@ class CourseOverview(TimeStampedModel):
return course_overviews return course_overviews
@classmethod @classmethod
def get_all_courses(cls, org=None): def get_all_courses(cls, org=None, filter_=None):
""" """
Returns all CourseOverview objects in the database. Returns all CourseOverview objects in the database.
Arguments: Arguments:
org (string): Optional parameter that allows filtering org (string): Optional parameter that allows case-insensitive
by organization. filtering by organization.
filter_ (dict): Optional parameter that allows custom filtering.
""" """
# Note: If a newly created course is not returned in this QueryList, # Note: If a newly created course is not returned in this QueryList,
# make sure the "publish" signal was emitted when the course was # make sure the "publish" signal was emitted when the course was
...@@ -489,6 +490,9 @@ class CourseOverview(TimeStampedModel): ...@@ -489,6 +490,9 @@ class CourseOverview(TimeStampedModel):
# Case-insensitive exact matching allows us to deal with this kind of dirty data. # Case-insensitive exact matching allows us to deal with this kind of dirty data.
course_overviews = course_overviews.filter(org__iexact=org) course_overviews = course_overviews.filter(org__iexact=org)
if filter_:
course_overviews = course_overviews.filter(**filter_)
return course_overviews return course_overviews
@classmethod @classmethod
......
...@@ -826,3 +826,24 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase): ...@@ -826,3 +826,24 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
} }
) )
return course_overview 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_),
)
...@@ -23,6 +23,15 @@ class FormTestMixin(object): ...@@ -23,6 +23,15 @@ class FormTestMixin(object):
form = self.get_form(expected_valid=False) form = self.get_form(expected_valid=False)
self.assertEqual(form.errors, {expected_field: [expected_message]}) 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): def assert_field_value(self, field, expected_value):
""" """
Create a form bound to self.form_data, assert its validity, and assert Create a form bound to self.form_data, assert its validity, and assert
......
...@@ -116,7 +116,7 @@ def view_course_access(depth=0, access_action='load', check_for_milestones=False ...@@ -116,7 +116,7 @@ def view_course_access(depth=0, access_action='load', check_for_milestones=False
return _decorator 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. 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): ...@@ -129,7 +129,9 @@ def view_auth_classes(is_user=False):
OAuth2AuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser,
SessionAuthenticationAllowInactiveUser SessionAuthenticationAllowInactiveUser
) )
func_or_class.permission_classes = (IsAuthenticated,) func_or_class.permission_classes = ()
if is_authenticated:
func_or_class.permission_classes += (IsAuthenticated,)
if is_user: if is_user:
func_or_class.permission_classes += (IsUserInUrl,) func_or_class.permission_classes += (IsUserInUrl,)
return func_or_class return func_or_class
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment