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

feat: Add permissions filter to list courses API

Browse Source 
The new filter, called `permissions`, allows callers to filter courses
per access granted to the specified username.  Callers can now filter
courses per roles, actions, etc.
This commit is contained in:
J. Victor Martins
2021-06-24 17:34:48 -03:00
committed by João Victor Martins
parent d25ace50b4
commit 6adf45df2f
7 changed files with 97 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -108,7 +108,12 @@ def _filter_by_search(course_queryset, search_term):
)
def list_courses(request, username, org=None, filter_=None, search_term=None):
def list_courses(request,
username,
org=None,
filter_=None,
search_term=None,
permissions=None):
"""
Yield all available courses.
@@ -134,12 +139,15 @@ def list_courses(request, username, org=None, filter_=None, search_term=None):
by the given key-value pairs.
search_term (string):
Search term to filter courses (used by ElasticSearch).
permissions (list[str]):
If specified, it filters visible `CourseOverview` objects by
checking if each permission specified is granted for the username.
Return value:
Yield `CourseOverview` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
course_qs = get_courses(user, org=org, filter_=filter_)
course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions)
course_qs = _filter_by_search(course_qs, search_term)
return course_qs

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

@@ -10,7 +10,10 @@ 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:
@@ -59,6 +62,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
filter_type(param_name='mobile', field_name='mobile_available'),
]
mobile = ExtendedNullBooleanField(required=False)
permissions = MultiValueField(required=False)
def clean(self):
"""

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

@@ -100,7 +100,12 @@ class CourseListTestMixin(CourseApiTestMixin):
Common behavior for list_courses tests
"""
def _make_api_call(self, requesting_user, specified_user, org=None, filter_=None):
def _make_api_call(self,
requesting_user,
specified_user,
org=None,
filter_=None,
permissions=None):
"""
Call the list_courses api endpoint to get information about
`specified_user` on behalf of `requesting_user`.
@@ -108,7 +113,13 @@ class CourseListTestMixin(CourseApiTestMixin):
request = Request(self.request_factory.get('/'))
request.user = requesting_user
with check_mongo_calls(0):
return list_courses(request, specified_user.username, org=org, filter_=filter_)
return list_courses(
request,
specified_user.username,
org=org,
filter_=filter_,
permissions=permissions,
)
def verify_courses(self, courses):
"""
@@ -209,6 +220,28 @@ class TestGetCourseListMultipleCourses(CourseListTestMixin, ModuleStoreTestCase)
assert {course.id for course in filtered_courses} == {course.id for course in expected_courses},\
f'testing course_api.api.list_courses with filter_={filter_}'
def test_permissions(self):
# Create a second course to be filtered out of queries.
self.create_course(course='should-be-hidden-course')
# Create instructor (non-staff), and enroll him in the course.
instructor_user = self.create_user('the-instructor', is_staff=False)
self.create_enrollment(user=instructor_user, course_id=self.course.id)
self.create_courseaccessrole(
user=instructor_user,
course_id=self.course.id,
role='instructor',
org='edX',
)
filtered_courses = self._make_api_call(
instructor_user,
instructor_user,
permissions={'instructor'})
self.assertEqual({c.id for c in filtered_courses}, {self.course.id})
class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
"""

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

@@ -69,6 +69,7 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT
'mobile': None,
'search_term': '',
'filter_': None,
'permissions': set(),
}
def test_basic(self):

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

@@ -201,6 +201,36 @@ class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreT
response = self.verify_response(params=params)
assert {course['course_id'] for course in response.data['results']} == {str(course.id) for course in expected_courses}, f'testing course_api.views.CourseListView with filter_={filter_}' # pylint: disable=line-too-long
def test_get_when_no_permission_then_filters_correctly(self):
"""
Given a user that is instructor in a course
And another course he is not an instructor
When get
Then only the first course is returned
"""
# Create a second course to be filtered out of queries.
self.create_course(course='should-be-hidden-course')
# Create instructor (non-staff), and enroll him in the course.
instructor_user = self.create_user('the-instructor', is_staff=False)
self.setup_user(instructor_user)
self.create_enrollment(user=instructor_user, course_id=self.course.id)
self.create_courseaccessrole(
user=instructor_user,
course_id=self.course.id,
role='instructor',
org='edX',
)
params = {'permissions': 'instructor',
'username': instructor_user.username}
response = self.verify_response(params=params)
self.assertEqual(response.status_code, 200)
ids = {c['course_id'] for c in response.json()['results']}
self.assertEqual(ids, {str(self.course.id)})
class CourseDetailViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase):
"""

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

@@ -271,6 +271,12 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
provided org code (e.g., "HarvardX") are returned.
Case-insensitive.
permissions (optional):
If specified, it filters visible `CourseOverview` objects by
checking if each permission specified is granted for the username.
Notice that Staff users are always granted permission to list any
course.
**Returns**
* 200 on success, with a list of course discovery objects as returned
@@ -321,13 +327,13 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
form = CourseListGetForm(self.request.query_params, initial={'requesting_user': self.request.user})
if not form.is_valid():
raise ValidationError(form.errors)
return list_courses(
self.request,
form.cleaned_data['username'],
org=form.cleaned_data['org'],
filter_=form.cleaned_data['filter_'],
search_term=form.cleaned_data['search_term']
search_term=form.cleaned_data['search_term'],
permissions=form.cleaned_data['permissions']
)

11
lms/djangoapps/courseware/courses.py
View File

@@ -702,10 +702,13 @@ def get_course_syllabus_section(course, section_key):
@function_trace('get_courses')
def get_courses(user, org=None, filter_=None):
def get_courses(user, org=None, filter_=None, permissions=None):
"""
Return a LazySequence of courses available, optionally filtered by org code (case-insensitive).
Return a LazySequence of courses available, optionally filtered by org code
(case-insensitive) or a set of permissions to be satisfied for the specified
user.
"""
courses = branding.get_visible_courses(
org=org,
filter_=filter_,
@@ -715,13 +718,15 @@ def get_courses(user, org=None, filter_=None):
'image_set'
)
permissions = set(permissions or '')
permission_name = configuration_helpers.get_value(
'COURSE_CATALOG_VISIBILITY_PERMISSION',
settings.COURSE_CATALOG_VISIBILITY_PERMISSION
)
permissions.add(permission_name)
return LazySequence(
(c for c in courses if has_access(user, permission_name, c)),
(c for c in courses if all(has_access(user, p, c) for p in permissions)),
est_len=courses.count()
)