Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Paginate results of Courses API list endpoint

Browse Source 
* Catalog results are now paginated
* Implements the new namespaced pagination described at
  https://openedx.atlassian.net/wiki/pages/viewpage.action?pageId=47481813
* API level code returns pythonic business objects
* View layer performs serialize at the view layer
* Convert views to use DRF generic views
* Removes an unintentional authentication decorator that caused
  the detail endpoint to return a 401 for anonymous users

MA-1724
This commit is contained in:
J. Cliff Dyer
2015-11-20 21:59:54 +00:00
committed by Nimisha Asthagiri
parent 11f34c1e8c
commit f53de2c04a
8 changed files with 219 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
lms/djangoapps/course_api/api.py
View File

@@ -9,7 +9,6 @@ from rest_framework.exceptions import NotFound, PermissionDenied
from lms.djangoapps.courseware.courses import get_courses, get_course_with_access
from .permissions import can_view_courses_for_username
from .serializers import CourseSerializer
def get_effective_user(requesting_user, target_username):
@@ -41,14 +40,14 @@ def course_detail(request, username, course_key):
course_key (CourseKey): Identifies the course of interest
Return value:
CourseSerializer object representing the requested course
`CourseDescriptor` object representing the requested course
"""
user = get_effective_user(request.user, username)
try:
course = get_course_with_access(user, 'see_exists', course_key)
except Http404:
raise NotFound()
return CourseSerializer(course, context={'request': request}).data
return course
def list_courses(request, username):
@@ -69,8 +68,8 @@ def list_courses(request, username):
Return value:
A CourseSerializer object representing the collection of courses.
List of `CourseDescriptor` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
courses = get_courses(user)
return CourseSerializer(courses, context={'request': request}, many=True).data
return courses

5
lms/djangoapps/course_api/serializers.py
View File

@@ -42,7 +42,7 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth
Serializer for Course objects
"""
course_id = serializers.CharField(source='id', read_only=True) # pylint: disable=invalid-name
course_id = serializers.CharField(source='id', read_only=True)
name = serializers.CharField(source='display_name_with_default')
number = serializers.CharField(source='display_number_with_default')
org = serializers.CharField(source='display_org_with_default')
@@ -88,7 +88,8 @@ class CourseSerializer(serializers.Serializer): # pylint: disable=abstract-meth
"""
Get the representation for SerializerMethodField `blocks_url`
"""
return '?'.join([
base_url = '?'.join([
reverse('blocks_in_course'),
urllib.urlencode({'course_id': course.id}),
])
return self.context['request'].build_absolute_uri(base_url)

89
lms/djangoapps/course_api/tests/test_api.py
View File

@@ -2,14 +2,14 @@
Test for course API
"""
from datetime import datetime
from django.contrib.auth.models import AnonymousUser
from django.test import RequestFactory
from rest_framework.exceptions import NotFound, PermissionDenied
from rest_framework.request import Request
from rest_framework.test import APIRequestFactory
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ModuleStoreTestCase
from xmodule.course_module import DEFAULT_START_DATE
from xmodule.course_module import CourseDescriptor
from ..api import course_detail, list_courses
from .mixins import CourseApiFactoryMixin
@@ -42,10 +42,20 @@ class CourseApiTestMixin(CourseApiFactoryMixin):
'blocks_url': '/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
}
def verify_course(self, course, course_id=None):
"""
Ensure that the returned course is the course we just created
"""
if course_id is None:
course_id = self.expected_course_data['course_id']
self.assertIsInstance(course, CourseDescriptor)
self.assertEqual(course_id, str(course.id))
@classmethod
def setUpClass(cls):
super(CourseApiTestMixin, cls).setUpClass()
cls.request_factory = RequestFactory()
cls.request_factory = APIRequestFactory()
class CourseDetailTestMixin(CourseApiTestMixin):
@@ -57,9 +67,9 @@ class CourseDetailTestMixin(CourseApiTestMixin):
Call the `course_detail` api endpoint to get information on the course
identified by `course_key`.
"""
request = self.request_factory.get('/')
request = Request(self.request_factory.get('/'))
request.user = requesting_user
return course_detail(request, target_user, course_key)
return course_detail(request, target_user.username, course_key)
class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
@@ -75,54 +85,25 @@ class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
cls.staff_user = cls.create_user('staff', is_staff=True)
def test_get_existing_course(self):
result = self._make_api_call(self.honor_user, self.honor_user.username, self.course.id)
self.assertEqual(self.expected_course_data, result)
course = self._make_api_call(self.honor_user, self.honor_user, self.course.id)
self.verify_course(course)
def test_get_nonexistent_course(self):
course_key = CourseKey.from_string(u'edX/toy/nope')
with self.assertRaises(NotFound):
self._make_api_call(self.honor_user, self.honor_user.username, course_key)
self._make_api_call(self.honor_user, self.honor_user, course_key)
def test_hidden_course_for_honor(self):
with self.assertRaises(NotFound):
self._make_api_call(self.honor_user, self.honor_user.username, self.hidden_course.id)
self._make_api_call(self.honor_user, self.honor_user, self.hidden_course.id)
def test_hidden_course_for_staff(self):
result = self._make_api_call(self.staff_user, self.staff_user.username, self.hidden_course.id)
self.assertIsInstance(result, dict)
self.assertEqual(result['course_id'], u'edX/hidden/2012_Fall')
course = self._make_api_call(self.staff_user, self.staff_user, self.hidden_course.id)
self.verify_course(course, course_id=u'edX/hidden/2012_Fall')
def test_hidden_course_for_staff_as_honor(self):
with self.assertRaises(NotFound):
self._make_api_call(self.staff_user, self.honor_user.username, self.hidden_course.id)
class TestGetCourseDetailStartDate(CourseDetailTestMixin, ModuleStoreTestCase):
"""
Test variations of start_date field responses
"""
def setUp(self):
super(TestGetCourseDetailStartDate, self).setUp()
self.staff_user = self.create_user('staff', is_staff=True)
def test_course_with_advertised_start(self):
course = self.create_course(
course=u'custom',
start=datetime(2015, 3, 15),
advertised_start=u'The Ides of March'
)
result = self._make_api_call(self.staff_user, self.staff_user.username, course.id)
self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
self.assertEqual(result['start_type'], u'string')
self.assertEqual(result['start_display'], u'The Ides of March')
def test_course_with_empty_start_date(self):
course = self.create_course(start=DEFAULT_START_DATE, course=u'custom2')
result = self._make_api_call(self.staff_user, self.staff_user.username, course.id)
self.assertEqual(result['course_id'], u'edX/custom2/2012_Fall')
self.assertEqual(result['start_type'], u'empty')
self.assertIsNone(result['start_display'])
self._make_api_call(self.staff_user, self.honor_user, self.hidden_course.id)
class CourseListTestMixin(CourseApiTestMixin):
@@ -134,15 +115,23 @@ class CourseListTestMixin(CourseApiTestMixin):
Call the list_courses api endpoint to get information about
`specified_user` on behalf of `requesting_user`.
"""
request = self.request_factory.get('/')
request = Request(self.request_factory.get('/'))
request.user = requesting_user
return list_courses(request, specified_user.username)
def verify_courses(self, courses):
"""
Verify that there is one course, and that it has the expected format.
"""
self.assertEqual(len(courses), 1)
self.verify_course(courses[0])
class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase):
"""
Test the behavior of the `list_courses` api function.
"""
@classmethod
def setUpClass(cls):
super(TestGetCourseList, cls).setUpClass()
@@ -153,17 +142,15 @@ class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase):
def test_as_staff(self):
courses = self._make_api_call(self.staff_user, self.staff_user)
self.assertEqual(len(courses), 1)
self.assertEqual(courses[0], self.expected_course_data)
self.verify_courses(courses)
def test_for_honor_user_as_staff(self):
courses = self._make_api_call(self.staff_user, self.honor_user)
self.assertEqual(len(courses), 1)
self.assertEqual(courses[0], self.expected_course_data)
self.verify_courses(courses)
def test_as_honor(self):
courses = self._make_api_call(self.honor_user, self.honor_user)
self.assertEqual(len(courses), 1)
self.assertEqual(courses[0], self.expected_course_data)
self.verify_courses(courses)
def test_for_staff_user_as_honor(self):
with self.assertRaises(PermissionDenied):
@@ -172,8 +159,7 @@ class TestGetCourseList(CourseListTestMixin, SharedModuleStoreTestCase):
def test_as_anonymous(self):
anonuser = AnonymousUser()
courses = self._make_api_call(anonuser, anonuser)
self.assertEqual(len(courses), 1)
self.assertEqual(courses[0], self.expected_course_data)
self.verify_courses(courses)
def test_for_honor_user_as_anonymous(self):
anonuser = AnonymousUser()
@@ -209,5 +195,4 @@ class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
def test_hidden_course_for_staff(self):
self.create_course(visible_to_staff_only=True)
courses = self._make_api_call(self.staff_user, self.staff_user)
self.assertEqual(len(courses), 1)
self.assertEqual(courses[0], self.expected_course_data)
self.verify_courses(courses)

67
lms/djangoapps/course_api/tests/test_serializers.py Normal file
View File

@@ -0,0 +1,67 @@
"""
Test data created by CourseSerializer
"""
from datetime import datetime
from rest_framework.test import APIRequestFactory
from rest_framework.request import Request
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.course_module import DEFAULT_START_DATE
from ..serializers import CourseSerializer
from .mixins import CourseApiFactoryMixin
class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
"""
Test variations of start_date field responses
"""
def setUp(self):
super(TestCourseSerializerFields, self).setUp()
self.staff_user = self.create_user('staff', is_staff=True)
self.honor_user = self.create_user('honor', is_staff=False)
self.request_factory = APIRequestFactory()
def _get_request(self, user=None):
"""
Build a Request object for the specified user
"""
if user is None:
user = self.honor_user
request = Request(self.request_factory.get('/'))
request.user = user
return request
def test_advertised_start(self):
course = self.create_course(
course=u'custom',
start=datetime(2015, 3, 15),
advertised_start=u'The Ides of March'
)
result = CourseSerializer(course, context={'request': self._get_request()}).data
self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
self.assertEqual(result['start_type'], u'string')
self.assertEqual(result['start_display'], u'The Ides of March')
def test_empty_start(self):
course = self.create_course(start=DEFAULT_START_DATE, course=u'custom')
result = CourseSerializer(course, context={'request': self._get_request()}).data
self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
self.assertEqual(result['start_type'], u'empty')
self.assertIsNone(result['start_display'])
def test_description(self):
course = self.create_course()
result = CourseSerializer(course, context={'request': self._get_request()}).data
self.assertEqual(result['description'], u'A course about toys.')
def test_blocks_url(self):
course = self.create_course()
result = CourseSerializer(course, context={'request': self._get_request()}).data
self.assertEqual(
result['blocks_url'],
u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall'
)

7
lms/djangoapps/course_api/tests/test_views.py
View File

@@ -4,7 +4,6 @@ Tests for Blocks Views
from django.core.urlresolvers import reverse
from django.test import RequestFactory
from rest_framework.exceptions import NotFound
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
@@ -121,7 +120,7 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
self.verify_response(params={'username': self.honor_user.username})
def test_as_anonymous_user(self):
self.verify_response(expected_status_code=401)
self.verify_response(expected_status_code=200)
def test_hidden_course_as_honor(self):
self.setup_user(self.honor_user)
@@ -142,5 +141,5 @@ class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase
request = request_factory.get('/')
request.query_params = {}
request.user = self.staff_user
with self.assertRaises(NotFound):
CourseDetailView().get(request, 'a:b:c')
response = CourseDetailView().dispatch(request, course_key_string='a:b:c')
self.assertEqual(404, response.status_code)

41
lms/djangoapps/course_api/views.py
View File

@@ -3,19 +3,18 @@ Course API Views
"""
from rest_framework.exceptions import NotFound
from rest_framework.views import APIView, Response
from rest_framework.generics import ListAPIView, RetrieveAPIView
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from openedx.core.lib.api.view_utils import view_auth_classes
from openedx.core.lib.api.paginators import NamespacedPageNumberPagination
from .api import course_detail, list_courses
from .serializers import CourseSerializer
@view_auth_classes()
class CourseDetailView(APIView):
class CourseDetailView(RetrieveAPIView):
"""
**Use Cases**
@@ -89,21 +88,26 @@ class CourseDetailView(APIView):
}
"""
def get(self, request, course_key_string):
serializer_class = CourseSerializer
lookup_url_kwarg = 'course_key_string'
def get_object(self):
"""
GET /api/courses/v1/courses/{course_key}/
Return the requested course object, if the user has appropriate
permissions.
"""
username = request.query_params.get('username', request.user.username)
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 NotFound()
content = course_detail(request, username, course_key)
return Response(content)
return course_detail(self.request, username, course_key)
class CourseListView(APIView):
class CourseListView(ListAPIView):
"""
**Use Cases**
@@ -158,11 +162,12 @@ class CourseListView(APIView):
]
"""
def get(self, request):
"""
GET /api/courses/v1/courses/
"""
username = request.query_params.get('username', request.user.username)
pagination_class = NamespacedPageNumberPagination
serializer_class = CourseSerializer
content = list_courses(request, username)
return Response(content)
def get_queryset(self):
"""
Return a list of courses visible to the user.
"""
username = self.request.query_params.get('username', self.request.user.username)
return list_courses(self.request, username)

32
openedx/core/lib/api/paginators.py
View File

@@ -29,6 +29,38 @@ class DefaultPagination(pagination.PageNumberPagination):
})
class NamespacedPageNumberPagination(pagination.PageNumberPagination):
"""
Pagination scheme that returns results with pagination metadata
embedded in a "pagination" attribute. Can be used with data
that comes as a list of items, or as a dict with a "results"
attribute that contains a list of items.
"""
page_size_query_param = "page_size"
def get_paginated_response(self, data):
"""
Annotate the response with pagination information
"""
metadata = {
'next': self.get_next_link(),
'previous': self.get_previous_link(),
'count': self.page.paginator.count,
'num_pages': self.page.paginator.num_pages,
}
if isinstance(data, dict):
if 'results' not in data:
raise TypeError(u'Malformed result dict')
data['pagination'] = metadata
else:
data = {
'results': data,
'pagination': metadata,
}
return Response(data)
def paginate_search_results(object_class, search_results, page_size, page):
"""
Takes edx-search results and returns a Page object populated

51
openedx/core/lib/api/tests/test_paginators.py
View File

@@ -1,10 +1,15 @@
""" Tests paginator methods """
from collections import namedtuple
import ddt
from mock import Mock, MagicMock
from unittest import TestCase
from django.http import Http404
from django.test import RequestFactory
from rest_framework import serializers
from openedx.core.lib.api.paginators import paginate_search_results
from openedx.core.lib.api.paginators import NamespacedPageNumberPagination, paginate_search_results
@ddt.ddt
@@ -118,6 +123,50 @@ class PaginateSearchResultsTestCase(TestCase):
paginate_search_results(self.mock_model, self.search_results, self.default_size, page_num)
class NamespacedPaginationTestCase(TestCase):
"""
Test behavior of `NamespacedPageNumberPagination`
"""
TestUser = namedtuple('TestUser', ['username', 'email'])
class TestUserSerializer(serializers.Serializer): # pylint: disable=abstract-method
"""
Simple serializer to paginate results from
"""
username = serializers.CharField()
email = serializers.CharField()
expected_data = {
'results': [
{'username': 'user_5', 'email': 'user_5@example.com'},
{'username': 'user_6', 'email': 'user_6@example.com'},
{'username': 'user_7', 'email': 'user_7@example.com'},
{'username': 'user_8', 'email': 'user_8@example.com'},
{'username': 'user_9', 'email': 'user_9@example.com'},
],
'pagination': {
'next': 'http://testserver/endpoint?page=3&page_size=5',
'previous': 'http://testserver/endpoint?page_size=5',
'count': 25,
'num_pages': 5,
}
}
def setUp(self):
super(NamespacedPaginationTestCase, self).setUp()
self.paginator = NamespacedPageNumberPagination()
self.users = [self.TestUser('user_{}'.format(idx), 'user_{}@example.com'.format(idx)) for idx in xrange(25)]
self.request_factory = RequestFactory()
def test_basic_pagination(self):
request = self.request_factory.get('/endpoint', data={'page': 2, 'page_size': 5})
request.query_params = {'page': 2, 'page_size': 5}
paged_users = self.paginator.paginate_queryset(self.users, request)
results = self.TestUserSerializer(paged_users, many=True).data
self.assertEqual(self.expected_data, self.paginator.get_paginated_response(results).data)
def build_mock_object(obj_id):
""" Build a mock object with the passed id"""
mock_object = Mock()