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

Merge pull request #22976 from edx/feanil/remove_course_api_role_option

Browse Source 
Remove the role parameter from the courses api.
This commit is contained in:
Feanil Patel
2020-01-30 09:23:58 -05:00
committed by GitHub
parent eb694528e7 79d097d3c8
commit ee5178604f
5 changed files with 2 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -74,25 +74,6 @@ def course_detail(request, username, course_key):
return overview
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.
@@ -121,8 +102,7 @@ def _filter_by_search(course_queryset, search_term):
)
@function_trace('list_courses')
def list_courses(request, username, org=None, roles=None, filter_=None, search_term=None):
def list_courses(request, username, org=None, filter_=None, search_term=None):
"""
Yield all available courses.
@@ -143,10 +123,6 @@ def list_courses(request, username, org=None, roles=None, filter_=None, search_t
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.
@@ -158,7 +134,6 @@ def list_courses(request, username, org=None, roles=None, filter_=None, search_t
"""
user = get_effective_user(request.user, username)
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

@@ -11,7 +11,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, MultiValueField
from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField
class UsernameValidatorMixin(object):
@@ -53,7 +53,6 @@ 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

@@ -68,7 +68,6 @@ 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,9 +23,6 @@ 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
@@ -162,7 +159,6 @@ 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."""
@@ -210,86 +206,6 @@ 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.encode('utf-8')).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.assertEqual(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.encode('utf-8')).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.assertEqual(len(filtered_response.data['results']), 1)
self.assertEqual(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.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.assertEqual(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.assertEqual(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.assertEqual(len(filtered_response.data['results']), 1)
class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
"""

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

@@ -191,12 +191,6 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
provided org code (e.g., "HarvardX") are returned.
Case-insensitive.
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**
* 200 on success, with a list of course discovery objects as returned
@@ -247,13 +241,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
if not form.is_valid():
raise ValidationError(form.errors)
set_custom_metric('query_param_roles', form.cleaned_data['role'])
return list_courses(
self.request,
form.cleaned_data['username'],
org=form.cleaned_data['org'],
roles=form.cleaned_data['role'],
filter_=form.cleaned_data['filter_'],
search_term=form.cleaned_data['search_term']
)