Merge pull request #22986 from edx/robrap/BOM-897-add-course-id-endpoint
BOM-897: add course_ids api
This commit is contained in:
@@ -3,7 +3,6 @@ Course API
|
||||
"""
|
||||
import logging
|
||||
|
||||
from edx_django_utils.monitoring import function_trace
|
||||
from edx_when.api import get_dates_for_course
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import AnonymousUser, User
|
||||
@@ -18,7 +17,9 @@ from lms.djangoapps.courseware.courses import (
|
||||
get_courses,
|
||||
get_permission_for_course_about
|
||||
)
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.lib.api.view_utils import LazySequence
|
||||
from student.roles import GlobalStaff
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
|
||||
@@ -138,6 +139,50 @@ def list_courses(request, username, org=None, filter_=None, search_term=None):
|
||||
return course_qs
|
||||
|
||||
|
||||
def list_course_keys(request, username, role):
|
||||
"""
|
||||
Yield all available CourseKeys for the user having the given role.
|
||||
|
||||
The courses returned include those for which the user identified by
|
||||
`username` has the given role. Additionally, the logged in user
|
||||
should have permission to view courses available to that user.
|
||||
|
||||
Note: This function does not use branding to determine courses.
|
||||
|
||||
Arguments:
|
||||
request (HTTPRequest):
|
||||
Used to identify the logged-in user and to instantiate the course
|
||||
module to retrieve the course about description
|
||||
username (string):
|
||||
The name of the user the logged-in user would like to be
|
||||
identified as
|
||||
|
||||
Keyword Arguments:
|
||||
role (string):
|
||||
Course keys are filtered such that only those for which the
|
||||
user has the specified role are returned.
|
||||
|
||||
Return value:
|
||||
Yield `CourseKey` objects representing the collection of courses.
|
||||
|
||||
"""
|
||||
user = get_effective_user(request.user, username)
|
||||
|
||||
course_keys = CourseOverview.get_all_course_keys()
|
||||
|
||||
# Global staff have access to all courses. Filter courses for non-global staff.
|
||||
if GlobalStaff().has_user(user):
|
||||
return course_keys
|
||||
|
||||
return LazySequence(
|
||||
(
|
||||
course_key for course_key in course_keys
|
||||
if has_access(user, role, course_key)
|
||||
),
|
||||
est_len=len(course_keys)
|
||||
)
|
||||
|
||||
|
||||
def get_due_dates(request, course_key, user):
|
||||
"""
|
||||
Get due date information for a user for blocks in a course.
|
||||
|
||||
@@ -75,3 +75,11 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
|
||||
cleaned_data['filter_'] = filter_ or None
|
||||
|
||||
return cleaned_data
|
||||
|
||||
|
||||
class CourseIdListGetForm(UsernameValidatorMixin, Form):
|
||||
"""
|
||||
A form to validate query parameters in the course list retrieval endpoint
|
||||
"""
|
||||
username = CharField(required=False)
|
||||
role = CharField(required=True)
|
||||
|
||||
@@ -121,3 +121,11 @@ class CourseDetailSerializer(CourseSerializer): # pylint: disable=abstract-meth
|
||||
# fields from CourseSerializer, which get their data
|
||||
# from the CourseOverview object in SQL.
|
||||
return CourseDetails.fetch_about_attribute(course_overview.id, 'overview')
|
||||
|
||||
|
||||
class CourseKeySerializer(serializers.BaseSerializer): # pylint:disable=abstract-method
|
||||
"""
|
||||
Serializer that takes a CourseKey and serializes it to a string course_id.
|
||||
"""
|
||||
def to_representation(self, instance):
|
||||
return str(instance)
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
"""
|
||||
Tests for Course API forms.
|
||||
"""
|
||||
|
||||
# pylint: disable=missing-docstring
|
||||
|
||||
from itertools import product
|
||||
|
||||
@@ -16,7 +16,7 @@ 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
|
||||
from ..forms import CourseDetailGetForm, CourseIdListGetForm, CourseListGetForm
|
||||
|
||||
|
||||
class UsernameTestMixin(object):
|
||||
@@ -101,6 +101,42 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT
|
||||
self.assert_valid(self.cleaned_data)
|
||||
|
||||
|
||||
class TestCourseIdListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase):
|
||||
FORM_CLASS = CourseIdListGetForm
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
super(TestCourseIdListGetForm, cls).setUpClass()
|
||||
|
||||
cls.course = CourseFactory.create()
|
||||
|
||||
def setUp(self):
|
||||
super(TestCourseIdListGetForm, 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,
|
||||
'role': 'staff',
|
||||
}),
|
||||
mutable=True,
|
||||
)
|
||||
self.cleaned_data = {
|
||||
'username': user.username,
|
||||
'role': 'staff',
|
||||
}
|
||||
|
||||
def test_basic(self):
|
||||
self.assert_valid(self.cleaned_data)
|
||||
|
||||
|
||||
class TestCourseDetailGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreTestCase):
|
||||
"""
|
||||
Tests for CourseDetailGetForm
|
||||
|
||||
@@ -4,11 +4,13 @@ Test data created by CourseSerializer and CourseDetailSerializer
|
||||
|
||||
|
||||
from datetime import datetime
|
||||
from unittest import TestCase
|
||||
|
||||
import ddt
|
||||
from rest_framework.request import Request
|
||||
from rest_framework.test import APIRequestFactory
|
||||
from xblock.core import XBlock
|
||||
from opaque_keys.edx.locator import CourseLocator
|
||||
|
||||
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
|
||||
from openedx.core.djangoapps.models.course_details import CourseDetails
|
||||
@@ -16,7 +18,7 @@ from xmodule.course_module import DEFAULT_START_DATE
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import check_mongo_calls
|
||||
|
||||
from ..serializers import CourseDetailSerializer, CourseSerializer
|
||||
from ..serializers import CourseDetailSerializer, CourseKeySerializer, CourseSerializer
|
||||
from .mixins import CourseApiFactoryMixin
|
||||
|
||||
|
||||
@@ -155,3 +157,11 @@ class TestCourseDetailSerializer(TestCourseSerializer):
|
||||
about_descriptor = XBlock.load_class('about')
|
||||
overview_template = about_descriptor.get_template('overview.yaml')
|
||||
self.expected_data['overview'] = overview_template.get('data')
|
||||
|
||||
|
||||
class TestCourseKeySerializer(TestCase):
|
||||
|
||||
def test_course_key_serializer(self):
|
||||
course_key = CourseLocator(org='org', course='course', run='2020_Q3')
|
||||
serializer = CourseKeySerializer(course_key)
|
||||
self.assertEqual(serializer.data, str(course_key))
|
||||
|
||||
@@ -5,6 +5,7 @@ Tests for Course API views.
|
||||
|
||||
from datetime import datetime
|
||||
from hashlib import md5
|
||||
from unittest import TestCase
|
||||
|
||||
import ddt
|
||||
import six
|
||||
@@ -21,12 +22,16 @@ from waffle.testutils import override_switch
|
||||
|
||||
from course_modes.models import CourseMode
|
||||
from course_modes.tests.factories import CourseModeFactory
|
||||
from openedx.core.lib.api.view_utils import LazySequence
|
||||
from openedx.features.content_type_gating.models import ContentTypeGatingConfig
|
||||
from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
|
||||
from student.auth import add_users
|
||||
from student.roles import CourseInstructorRole, CourseStaffRole
|
||||
from student.tests.factories import AdminFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
|
||||
from ..views import CourseDetailView, CourseListUserThrottle
|
||||
from ..views import CourseDetailView, CourseListUserThrottle, LazyPageNumberPagination
|
||||
from .mixins import TEST_PASSWORD, CourseApiFactoryMixin
|
||||
|
||||
|
||||
@@ -350,13 +355,12 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
|
||||
|
||||
def test_list_all_with_search_term(self):
|
||||
"""
|
||||
Test with search, should only the course that matches the search term.
|
||||
Test with search, should list only the course that matches the search term.
|
||||
"""
|
||||
res = self.verify_response(params={'search_term': 'unique search term'})
|
||||
self.assertIn('results', res.data)
|
||||
self.assertNotEqual(res.data['results'], [])
|
||||
# Returns a count of 3 courses because that's the estimate before filtering
|
||||
self.assertEqual(res.data['pagination']['count'], 3)
|
||||
self.assertEqual(res.data['pagination']['count'], 1)
|
||||
self.assertEqual(len(res.data['results']), 1) # Should return a single course
|
||||
|
||||
def test_too_many_courses(self):
|
||||
@@ -408,3 +412,118 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
|
||||
[c['id'] for c in response.data['results']],
|
||||
ordered_course_ids[(page - 1) * 30:page * 30]
|
||||
)
|
||||
|
||||
|
||||
class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase):
|
||||
"""
|
||||
Test responses returned from CourseIdListView (with tests that modify the courseware).
|
||||
"""
|
||||
ENABLED_SIGNALS = ['course_published']
|
||||
|
||||
def setUp(self):
|
||||
super(CourseIdListViewTestCase, self).setUp()
|
||||
self.course = self.create_course()
|
||||
self.url = reverse('course-id-list')
|
||||
self.staff_user = self.create_user(username='staff', is_staff=True)
|
||||
self.honor_user = self.create_user(username='honor', is_staff=False)
|
||||
self.global_admin = AdminFactory()
|
||||
|
||||
def test_filter_by_roles_global_staff(self):
|
||||
"""
|
||||
Verify that global staff are always returned all courses irregardless of role filter.
|
||||
"""
|
||||
self.setup_user(self.staff_user)
|
||||
|
||||
# Request the courses as the staff user with the different roles specified.
|
||||
for role in ('staff', 'instructor'):
|
||||
filtered_response = self.verify_response(params={'username': self.staff_user.username, 'role': role})
|
||||
self.assertEqual(len(filtered_response.data['results']), 1)
|
||||
|
||||
def test_filter_by_roles_non_staff(self):
|
||||
"""
|
||||
Verify that a non-staff user can't access course_ids by role.
|
||||
"""
|
||||
self.setup_user(self.honor_user)
|
||||
|
||||
# Request the courses as the non-staff user with the different roles should *not* be allowed.
|
||||
for role in ('staff', 'instructor'):
|
||||
filtered_response = self.verify_response(params={'username': self.honor_user.username, 'role': role})
|
||||
self.assertEqual(len(filtered_response.data['results']), 0)
|
||||
|
||||
def test_filter_by_roles_course_staff(self):
|
||||
"""
|
||||
Verify that course_ids are filtered by the provided roles.
|
||||
"""
|
||||
# Make this user a course staff user for the course.
|
||||
course_staff_user = self.create_user(username='course_staff', is_staff=False)
|
||||
add_users(self.global_admin, CourseStaffRole(self.course.id), course_staff_user)
|
||||
|
||||
# Create a second course, along with an instructor user for it.
|
||||
alternate_course = self.create_course(org='test')
|
||||
course_instructor_user = self.create_user(username='course_instructor', is_staff=False)
|
||||
add_users(self.global_admin, CourseInstructorRole(alternate_course.id), course_instructor_user)
|
||||
|
||||
# Requesting the courses for which the course staff user is staff should return *only* the single course.
|
||||
self.setup_user(self.staff_user)
|
||||
filtered_response = self.verify_response(params={
|
||||
'username': course_staff_user.username,
|
||||
'role': 'staff'
|
||||
})
|
||||
self.assertEqual(len(filtered_response.data['results']), 1)
|
||||
self.assertTrue(filtered_response.data['results'][0].startswith(self.course.org))
|
||||
|
||||
# The course staff user does *not* have the course instructor role on any courses.
|
||||
filtered_response = self.verify_response(params={
|
||||
'username': course_staff_user.username,
|
||||
'role': 'instructor'
|
||||
})
|
||||
self.assertEqual(len(filtered_response.data['results']), 0)
|
||||
|
||||
# The course instructor user only has the course instructor role on one course.
|
||||
filtered_response = self.verify_response(params={
|
||||
'username': course_instructor_user.username,
|
||||
'role': 'instructor'
|
||||
})
|
||||
self.assertEqual(len(filtered_response.data['results']), 1)
|
||||
self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course.org))
|
||||
|
||||
# The course instructor user has the inferred course staff role on one course.
|
||||
self.setup_user(course_instructor_user)
|
||||
filtered_response = self.verify_response(params={
|
||||
'username': course_instructor_user.username,
|
||||
'role': 'staff'
|
||||
})
|
||||
self.assertEqual(len(filtered_response.data['results']), 1)
|
||||
self.assertTrue(filtered_response.data['results'][0].startswith(alternate_course.org))
|
||||
|
||||
|
||||
class LazyPageNumberPaginationTestCase(TestCase):
|
||||
|
||||
def test_lazy_page_number_pagination(self):
|
||||
number_sequence = range(20)
|
||||
even_numbers_lazy_sequence = LazySequence(
|
||||
(
|
||||
number for number in number_sequence
|
||||
if (number % 2) == 0
|
||||
),
|
||||
est_len=len(number_sequence)
|
||||
)
|
||||
|
||||
expected_response = {
|
||||
'results': [10, 12, 14, 16, 18],
|
||||
'pagination': {
|
||||
'previous': 'http://testserver/endpoint?page_size=5',
|
||||
'num_pages': 2,
|
||||
'next': None,
|
||||
'count': 10}
|
||||
}
|
||||
|
||||
request = RequestFactory().get('/endpoint', data={'page': 2, 'page_size': 5})
|
||||
request.query_params = {'page': 2, 'page_size': 5}
|
||||
|
||||
pagination = LazyPageNumberPagination()
|
||||
pagination.max_page_size = 5
|
||||
pagination.page_size = 5
|
||||
paginated_queryset = pagination.paginate_queryset(even_numbers_lazy_sequence, request)
|
||||
paginated_response = pagination.get_paginated_response(paginated_queryset)
|
||||
self.assertDictEqual(expected_response, paginated_response.data)
|
||||
|
||||
@@ -6,10 +6,11 @@ Course API URLs
|
||||
from django.conf import settings
|
||||
from django.conf.urls import include, url
|
||||
|
||||
from .views import CourseDetailView, CourseListView
|
||||
from .views import CourseDetailView, CourseIdListView, CourseListView
|
||||
|
||||
urlpatterns = [
|
||||
url(r'^v1/courses/$', CourseListView.as_view(), name="course-list"),
|
||||
url(r'^v1/courses/{}'.format(settings.COURSE_KEY_PATTERN), CourseDetailView.as_view(), name="course-detail"),
|
||||
url(r'^v1/course_ids/$', CourseIdListView.as_view(), name="course-id-list"),
|
||||
url(r'', include('course_api.blocks.urls'))
|
||||
]
|
||||
|
||||
@@ -4,8 +4,6 @@ Course API Views
|
||||
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
from edx_django_utils.monitoring import set_custom_metric
|
||||
|
||||
from edx_rest_framework_extensions.paginators import NamespacedPageNumberPagination
|
||||
from rest_framework.generics import ListAPIView, RetrieveAPIView
|
||||
from rest_framework.throttling import UserRateThrottle
|
||||
@@ -13,9 +11,9 @@ from rest_framework.throttling import UserRateThrottle
|
||||
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
|
||||
|
||||
from . import USE_RATE_LIMIT_2_FOR_COURSE_LIST_API, USE_RATE_LIMIT_10_FOR_COURSE_LIST_API
|
||||
from .api import course_detail, list_courses
|
||||
from .forms import CourseDetailGetForm, CourseListGetForm
|
||||
from .serializers import CourseDetailSerializer, CourseSerializer
|
||||
from .api import course_detail, list_course_keys, list_courses
|
||||
from .forms import CourseDetailGetForm, CourseIdListGetForm, CourseListGetForm
|
||||
from .serializers import CourseDetailSerializer, CourseKeySerializer, CourseSerializer
|
||||
|
||||
|
||||
@view_auth_classes(is_authenticated=False)
|
||||
@@ -161,6 +159,25 @@ class CourseListUserThrottle(UserRateThrottle):
|
||||
return super(CourseListUserThrottle, self).allow_request(request, view)
|
||||
|
||||
|
||||
class LazyPageNumberPagination(NamespacedPageNumberPagination):
|
||||
"""
|
||||
NamespacedPageNumberPagination that works with a LazySequence queryset.
|
||||
|
||||
The paginator cache uses ``@cached_property`` to cache the property values for
|
||||
count and num_pages. It assumes these won't change, but in the case of a
|
||||
LazySquence, its count gets updated as we move through it. This class clears
|
||||
the cached property values before reporting results so they will be recalculated.
|
||||
|
||||
"""
|
||||
|
||||
def get_paginated_response(self, data):
|
||||
# Clear the cached property values to recalculate the estimated count from the LazySequence
|
||||
del self.page.paginator.__dict__['count']
|
||||
del self.page.paginator.__dict__['num_pages']
|
||||
|
||||
return super(LazyPageNumberPagination, self).get_paginated_response(data)
|
||||
|
||||
|
||||
@view_auth_classes(is_authenticated=False)
|
||||
class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
"""
|
||||
@@ -177,6 +194,7 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
Body comprises a list of objects as returned by `CourseDetailView`.
|
||||
|
||||
**Parameters**
|
||||
|
||||
search_term (optional):
|
||||
Search term to filter courses (used by ElasticSearch).
|
||||
|
||||
@@ -227,9 +245,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
}
|
||||
]
|
||||
"""
|
||||
class CourseListPageNumberPagination(LazyPageNumberPagination):
|
||||
max_page_size = 100
|
||||
|
||||
pagination_class = NamespacedPageNumberPagination
|
||||
pagination_class.max_page_size = 100
|
||||
pagination_class = CourseListPageNumberPagination
|
||||
serializer_class = CourseSerializer
|
||||
throttle_classes = (CourseListUserThrottle,)
|
||||
|
||||
@@ -248,3 +267,98 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
filter_=form.cleaned_data['filter_'],
|
||||
search_term=form.cleaned_data['search_term']
|
||||
)
|
||||
|
||||
|
||||
class CourseIdListUserThrottle(UserRateThrottle):
|
||||
"""Limit the number of requests users can make to the course list id API."""
|
||||
|
||||
THROTTLE_RATES = {
|
||||
'user': '20/minute',
|
||||
'staff': '40/minute',
|
||||
}
|
||||
|
||||
def allow_request(self, request, view):
|
||||
# Use a special scope for staff to allow for a separate throttle rate
|
||||
user = request.user
|
||||
if user.is_authenticated and (user.is_staff or user.is_superuser):
|
||||
self.scope = 'staff'
|
||||
self.rate = self.get_rate()
|
||||
self.num_requests, self.duration = self.parse_rate(self.rate)
|
||||
|
||||
return super(CourseIdListUserThrottle, self).allow_request(request, view)
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class CourseIdListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
"""
|
||||
**Use Cases**
|
||||
|
||||
Request a list of course IDs for all courses the specified user can
|
||||
access based on the provided parameters.
|
||||
|
||||
**Example Requests**
|
||||
|
||||
GET /api/courses/v1/courses_ids/
|
||||
|
||||
**Response Values**
|
||||
|
||||
Body comprises a list of course ids and pagination details.
|
||||
|
||||
**Parameters**
|
||||
|
||||
username (optional):
|
||||
The username of the specified user whose visible courses we
|
||||
want to see.
|
||||
|
||||
role (required):
|
||||
Course ids are filtered such that only those for which the
|
||||
user has the specified role are returned. Role can be "staff"
|
||||
or "instructor".
|
||||
Case-insensitive.
|
||||
|
||||
**Returns**
|
||||
|
||||
* 200 on success, with a list of course ids and pagination details
|
||||
* 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 who specifies a username other than their own.
|
||||
* 404 if the specified user does not exist, or the requesting user does
|
||||
not have permission to view their courses.
|
||||
|
||||
Example response:
|
||||
|
||||
{
|
||||
"results":
|
||||
[
|
||||
"course-v1:edX+DemoX+Demo_Course"
|
||||
],
|
||||
"pagination": {
|
||||
"previous": null,
|
||||
"num_pages": 1,
|
||||
"next": null,
|
||||
"count": 1
|
||||
}
|
||||
}
|
||||
|
||||
"""
|
||||
class CourseIdListPageNumberPagination(LazyPageNumberPagination):
|
||||
max_page_size = 1000
|
||||
|
||||
pagination_class = CourseIdListPageNumberPagination
|
||||
serializer_class = CourseKeySerializer
|
||||
throttle_classes = (CourseIdListUserThrottle,)
|
||||
|
||||
def get_queryset(self):
|
||||
"""
|
||||
Returns CourseKeys for courses which the user has the provided role.
|
||||
"""
|
||||
form = CourseIdListGetForm(self.request.query_params, initial={'requesting_user': self.request.user})
|
||||
if not form.is_valid():
|
||||
raise ValidationError(form.errors)
|
||||
|
||||
return list_course_keys(
|
||||
self.request,
|
||||
form.cleaned_data['username'],
|
||||
role=form.cleaned_data['role'],
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user