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

Add optional role parameter to course API endpoint. (#21059)

Browse Source 
* Add optional role parameter to course API endpoint.

* fixup! Add optional role parameter to course API endpoint.
This commit is contained in:
Julia Eskew
2019-08-02 12:52:30 -04:00
committed by GitHub
parent bf414335cc
commit a2dac21843
5 changed files with 159 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -4,14 +4,19 @@ Course API
from __future__ import absolute_import
from django.conf import settings
from django.contrib.auth.models import AnonymousUser, User
from rest_framework.exceptions import PermissionDenied
import search
import six
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.courses import (
get_course_overview_with_access,
get_courses,
get_permission_for_course_about
)
from openedx.core.lib.api.view_utils import LazySequence
from .permissions import can_view_courses_for_username
@@ -57,7 +62,54 @@ def course_detail(request, username, course_key):
)
def list_courses(request, username, org=None, filter_=None):
def _filter_by_role(course_queryset, user, roles):
"""
Filters a course queryset by the roles for which the user has access.
"""
# Global staff have access to all courses. Filter course roles for non-global staff only.
if not user.is_staff:
if roles:
for role in roles:
# Filter the courses again to return only the courses for which the user has each specified role.
course_queryset = LazySequence(
(
course for course in course_queryset
if has_access(user, role, course.id)
),
est_len=len(course_queryset)
)
return course_queryset
def _filter_by_search(course_queryset, search_term):
"""
Filters a course queryset by the specified search term.
"""
if not settings.FEATURES['ENABLE_COURSEWARE_SEARCH'] or not search_term:
return course_queryset
# Return all the results, 10K is the maximum allowed value for ElasticSearch.
# We should use 0 after upgrading to 1.1+:
# - https://github.com/elastic/elasticsearch/commit/8b0a863d427b4ebcbcfb1dcd69c996c52e7ae05e
results_size_infinity = 10000
search_courses = search.api.course_discovery_search(
search_term,
size=results_size_infinity,
)
search_courses_ids = {course['data']['id'] for course in search_courses['results']}
return LazySequence(
(
course for course in course_queryset
if six.text_type(course.id) in search_courses_ids
),
est_len=len(course_queryset)
)
def list_courses(request, username, org=None, roles=None, filter_=None, search_term=None):
"""
Yield all available courses.
@@ -78,12 +130,21 @@ def list_courses(request, username, org=None, filter_=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.
roles (list of strings):
If specified, visible `CourseOverview` objects are filtered
such that only those for which the user has the specified role(s)
are returned. Multiple role parameters can be specified.
filter_ (dict):
If specified, visible `CourseOverview` objects are filtered
by the given key-value pairs.
search_term (string):
Search term to filter courses (used by ElasticSearch).
Return value:
Yield `CourseOverview` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
return get_courses(user, org=org, filter_=filter_)
course_qs = get_courses(user, org=org, filter_=filter_)
course_qs = _filter_by_role(course_qs, user, roles)
course_qs = _filter_by_search(course_qs, search_term)
return course_qs

3
lms/djangoapps/course_api/forms.py
View File

@@ -12,7 +12,7 @@ from django.forms import CharField, Form
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField
from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField, MultiValueField
class UsernameValidatorMixin(object):
@@ -54,6 +54,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
search_term = CharField(required=False)
username = CharField(required=False)
org = CharField(required=False)
role = MultiValueField(required=False)
# white list of all supported filter fields
filter_type = namedtuple('filter_type', ['param_name', 'field_name'])

1
lms/djangoapps/course_api/tests/test_forms.py
View File

@@ -69,6 +69,7 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT
self.cleaned_data = {
'username': user.username,
'org': '',
'role': set([]),
'mobile': None,
'search_term': '',
'filter_': None,

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

@@ -23,6 +23,9 @@ from course_modes.models import CourseMode
from course_modes.tests.factories import CourseModeFactory
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
@@ -159,6 +162,7 @@ class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreT
self.url = reverse('course-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_org(self):
"""Verify that CourseOverviews are filtered by the provided org key."""
@@ -206,6 +210,86 @@ class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreT
u"testing course_api.views.CourseListView with filter_={}".format(filter_),
)
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)
# Create a second course to be filtered out of queries.
alternate_course = self.create_course(org=md5(self.course.org).hexdigest())
# Request the courses as the staff user with the different roles specified.
for roles in ('', 'staff', 'staff,instructor'):
filtered_response = self.verify_response(params={'username': self.staff_user.username, 'role': roles})
# Both courses should be returned in the course list.
for org in [self.course.org, alternate_course.org]:
self.assertTrue(
any(course['org'] == org for course in filtered_response.data['results'])
)
def test_filter_by_roles_non_staff(self):
"""
Verify that a non-staff user can't access CourseOverviews by role.
"""
# Requesting the courses as the non-staff user should *not* be allowed.
self.setup_user(self.honor_user)
filtered_response = self.verify_response(params={'username': self.honor_user.username, 'role': 'staff'})
self.assertEquals(len(filtered_response.data['results']), 0)
def test_filter_by_roles_course_staff(self):
"""
Verify that CourseOverviews 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 to be filtered out of queries, along with an instructor user for it.
alternate_course = self.create_course(org=md5(self.course.org).hexdigest())
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.assertEquals(len(filtered_response.data['results']), 1)
self.assertEquals(filtered_response.data['results'][0]['org'], 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.assertEquals(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.assertEquals(len(filtered_response.data['results']), 1)
self.assertEquals(filtered_response.data['results'][0]['org'], alternate_course.org)
# The honor user does *not* have the course staff -or- instructor role on any courses.
filtered_response = self.verify_response(params={
'username': self.honor_user.username,
'role': 'staff,instructor'
})
self.assertEquals(len(filtered_response.data['results']), 0)
# 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.assertEquals(len(filtered_response.data['results']), 1)
class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
"""

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

@@ -4,15 +4,12 @@ Course API Views
from __future__ import absolute_import
import search
import six
from django.conf import settings
from django.core.exceptions import ValidationError
from edx_rest_framework_extensions.paginators import NamespacedPageNumberPagination
from rest_framework.generics import ListAPIView, RetrieveAPIView
from rest_framework.throttling import UserRateThrottle
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, LazySequence, view_auth_classes
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
@@ -193,9 +190,11 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
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.
role (optional):
If specified, visible `CourseOverview` objects are filtered
such that only those for which the user has the specified role
are returned. Multiple role parameters can be specified.
Case-insensitive.
**Returns**
@@ -239,11 +238,6 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
serializer_class = CourseSerializer
throttle_classes = (CourseListUserThrottle,)
# Return all the results, 10K is the maximum allowed value for ElasticSearch.
# We should use 0 after upgrading to 1.1+:
# - https://github.com/elastic/elasticsearch/commit/8b0a863d427b4ebcbcfb1dcd69c996c52e7ae05e
results_size_infinity = 10000
def get_queryset(self):
"""
Yield courses visible to the user.
@@ -252,27 +246,11 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
if not form.is_valid():
raise ValidationError(form.errors)
db_courses = list_courses(
return list_courses(
self.request,
form.cleaned_data['username'],
org=form.cleaned_data['org'],
roles=form.cleaned_data['role'],
filter_=form.cleaned_data['filter_'],
)
if not settings.FEATURES['ENABLE_COURSEWARE_SEARCH'] or not form.cleaned_data['search_term']:
return db_courses
search_courses = search.api.course_discovery_search(
form.cleaned_data['search_term'],
size=self.results_size_infinity,
)
search_courses_ids = {course['data']['id'] for course in search_courses['results']}
return LazySequence(
(
course for course in db_courses
if six.text_type(course.id) in search_courses_ids
),
est_len=len(db_courses)
search_term=form.cleaned_data['search_term']
)