Commit 9b70981a by Nimisha Asthagiri

Merge pull request #11066 from edx/course-api/all-blocks-support

Course Blocks API support for all blocks when no username
parents 7af75531 59605159
......@@ -20,23 +20,46 @@ def get_blocks(
return_type='dict'
):
"""
Return a serialized representation of the course blocks
"""
# TODO support user=None by returning all blocks, not just user-specific ones
if user is None:
raise NotImplementedError
Return a serialized representation of the course blocks.
# transform blocks
Arguments:
request (HTTPRequest): Used for calling django reverse.
usage_key (UsageKey): Identifies the root block of interest.
user (User): Optional user object for whom the blocks are being
retrieved. If None, blocks are returned regardless of access checks.
depth (integer or None): Identifies the depth of the tree to return
starting at the root block. If None, the entire tree starting at
the root is returned.
nav_depth (integer): Optional parameter that indicates how far deep to
traverse into the block hierarchy before bundling all the
descendants for navigation.
requested_fields (list): Optional list of names of additional fields
to return for each block. Supported fields are listed in
transformers.SUPPORTED_FIELDS.
block_counts (list): Optional list of names of block types for which to
return an aggregate count of blocks.
student_view_data (list): Optional list of names of block types for
which blocks to return their student_view_data.
return_type (string): Possible values are 'dict' or 'list'. Indicates
the format for returning the blocks.
"""
# construct BlocksAPITransformer
blocks_api_transformer = BlocksAPITransformer(
block_counts,
student_view_data,
depth,
nav_depth
)
# list of transformers to apply, adding user-specific ones if user is provided
transformers = [blocks_api_transformer]
if user is not None:
transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS + [ProctoredExamTransformer()]
blocks = get_course_blocks(
user,
usage_key,
transformers=COURSE_BLOCK_ACCESS_TRANSFORMERS + [ProctoredExamTransformer(), blocks_api_transformer],
transformers=transformers,
)
# serialize
......
......@@ -9,37 +9,28 @@ from rest_framework.exceptions import PermissionDenied
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey
from openedx.core.djangoapps.util.forms import MultiValueField
from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField, MultiValueField
from xmodule.modulestore.django import modulestore
from .permissions import can_access_other_users_blocks, can_access_users_blocks
from . import permissions
class BlockListGetForm(Form):
"""
A form to validate query parameters in the block list retrieval endpoint
"""
username = CharField(required=True) # TODO return all blocks if user is not specified by requesting staff user
usage_key = CharField(required=True)
requested_fields = MultiValueField(required=False)
student_view_data = MultiValueField(required=False)
all_blocks = ExtendedNullBooleanField(required=False)
block_counts = MultiValueField(required=False)
depth = CharField(required=False)
nav_depth = IntegerField(required=False, min_value=0)
requested_fields = MultiValueField(required=False)
return_type = ChoiceField(
required=False,
choices=[(choice, choice) for choice in ['dict', 'list']],
)
def clean_requested_fields(self):
"""
Return a set of `requested_fields`, merged with defaults of `type`
and `display_name`
"""
requested_fields = self.cleaned_data['requested_fields']
# add default requested_fields
return (requested_fields or set()) | {'type', 'display_name'}
student_view_data = MultiValueField(required=False)
usage_key = CharField(required=True)
username = CharField(required=False)
def clean_depth(self):
"""
......@@ -56,19 +47,15 @@ class BlockListGetForm(Form):
except ValueError:
raise ValidationError("'{}' is not a valid depth value.".format(value))
def clean_usage_key(self):
def clean_requested_fields(self):
"""
Ensure a valid `usage_key` was provided.
Return a set of `requested_fields`, merged with defaults of `type`
and `display_name`
"""
usage_key = self.cleaned_data['usage_key']
try:
usage_key = UsageKey.from_string(usage_key)
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
except InvalidKeyError:
raise ValidationError("'{}' is not a valid usage key.".format(unicode(usage_key)))
requested_fields = self.cleaned_data['requested_fields']
return usage_key
# add default requested_fields
return (requested_fields or set()) | {'type', 'display_name'}
def clean_return_type(self):
"""
......@@ -76,38 +63,18 @@ class BlockListGetForm(Form):
"""
return self.cleaned_data['return_type'] or 'dict'
def clean_requested_user(self, cleaned_data, course_key):
def clean_usage_key(self):
"""
Validates and returns the requested_user, while checking permissions.
Ensure a valid `usage_key` was provided.
"""
requested_username = cleaned_data.get('username', '')
requesting_user = self.initial['requesting_user']
usage_key = self.cleaned_data['usage_key']
if requesting_user.username.lower() == requested_username.lower():
requested_user = requesting_user
else:
# the requesting user is trying to access another user's view
# verify requesting user can access another user's blocks
if not can_access_other_users_blocks(requesting_user, course_key):
raise PermissionDenied(
"'{requesting_username}' does not have permission to access view for '{requested_username}'."
.format(requesting_username=requesting_user.username, requested_username=requested_username)
)
# update requested user object
try:
requested_user = User.objects.get(username=requested_username)
except User.DoesNotExist:
raise Http404("Requested user '{username}' does not exist.".format(username=requested_username))
# verify whether the requested user's blocks can be accessed
if not can_access_users_blocks(requested_user, course_key):
raise PermissionDenied(
"Course blocks for '{requested_username}' cannot be accessed."
.format(requested_username=requested_username)
)
try:
usage_key = UsageKey.from_string(usage_key)
except InvalidKeyError:
raise ValidationError("'{}' is not a valid usage key.".format(unicode(usage_key)))
return requested_user
return usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
def clean(self):
"""
......@@ -115,7 +82,8 @@ class BlockListGetForm(Form):
"""
cleaned_data = super(BlockListGetForm, self).clean()
# add additional requested_fields that are specified as separate parameters, if they were requested
# Add additional requested_fields that are specified as separate
# parameters, if they were requested.
additional_requested_fields = [
'student_view_data',
'block_counts',
......@@ -130,5 +98,72 @@ class BlockListGetForm(Form):
if not usage_key:
return
cleaned_data['user'] = self.clean_requested_user(cleaned_data, usage_key.course_key)
cleaned_data['user'] = self._clean_requested_user(cleaned_data, usage_key.course_key)
return cleaned_data
def _clean_requested_user(self, cleaned_data, course_key):
"""
Validates and returns the requested_user, while checking permissions.
"""
requesting_user = self.initial['requesting_user']
requested_username = cleaned_data.get('username', None)
if not requested_username:
return self._verify_no_user(requesting_user, cleaned_data, course_key)
elif requesting_user.username.lower() == requested_username.lower():
return self._verify_requesting_user(requesting_user, course_key)
else:
return self._verify_other_user(requesting_user, requested_username, course_key)
@staticmethod
def _verify_no_user(requesting_user, cleaned_data, course_key):
"""
Verifies form for when no username is specified, including permissions.
"""
# Verify that access to all blocks is requested
# (and not unintentionally requested).
if not cleaned_data.get('all_blocks', None):
raise ValidationError({'username': ['This field is required unless all_blocks is requested.']})
# Verify all blocks can be accessed for the course.
if not permissions.can_access_all_blocks(requesting_user, course_key):
raise PermissionDenied(
"'{requesting_username}' does not have permission to access all blocks in '{course_key}'."
.format(requesting_username=requesting_user.username, course_key=unicode(course_key))
)
# return None for user
return None
@staticmethod
def _verify_requesting_user(requesting_user, course_key):
"""
Verifies whether the requesting user can access blocks in the course.
"""
if not permissions.can_access_self_blocks(requesting_user, course_key):
raise PermissionDenied(
"Course blocks for '{requesting_username}' cannot be accessed."
.format(requesting_username=requesting_user.username)
)
return requesting_user
@staticmethod
def _verify_other_user(requesting_user, requested_username, course_key):
"""
Verifies whether the requesting user can access another user's view of
the blocks in the course.
"""
# Verify requesting user can access the user's blocks.
if not permissions.can_access_others_blocks(requesting_user, course_key):
raise PermissionDenied(
"'{requesting_username}' does not have permission to access view for '{requested_username}'."
.format(requesting_username=requesting_user.username, requested_username=requested_username)
)
# Verify user exists.
try:
return User.objects.get(username=requested_username)
except User.DoesNotExist:
raise Http404(
"Requested user '{requested_username}' does not exist.".format(requested_username=requested_username)
)
......@@ -4,21 +4,30 @@ Encapsulates permissions checks for Course Blocks API
from courseware.access import has_access
from student.models import CourseEnrollment
from student.roles import CourseStaffRole
def can_access_other_users_blocks(requesting_user, course_key):
def can_access_all_blocks(requesting_user, course_key):
"""
Returns whether the requesting_user can access all the blocks
in the course.
"""
return has_access(requesting_user, CourseStaffRole.ROLE, course_key)
def can_access_others_blocks(requesting_user, course_key):
"""
Returns whether the requesting_user can access the blocks for
other users in the given course.
"""
return has_access(requesting_user, 'staff', course_key)
return has_access(requesting_user, CourseStaffRole.ROLE, course_key)
def can_access_users_blocks(requested_user, course_key):
def can_access_self_blocks(requesting_user, course_key):
"""
Returns whether blocks for the requested_user is accessible.
Returns whether the requesting_user can access own blocks.
"""
return (
(requested_user.id and CourseEnrollment.is_enrolled(requested_user, course_key)) or
has_access(requested_user, 'staff', course_key)
(requesting_user.id and CourseEnrollment.is_enrolled(requesting_user, course_key)) or
has_access(requesting_user, CourseStaffRole.ROLE, course_key)
)
......@@ -4,19 +4,29 @@ Tests for Blocks api.py
from django.test.client import RequestFactory
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import SampleCourseFactory
from ..api import get_blocks
class TestGetBlocks(ModuleStoreTestCase):
class TestGetBlocks(SharedModuleStoreTestCase):
"""
Tests for the get_blocks function
"""
@classmethod
def setUpClass(cls):
super(TestGetBlocks, cls).setUpClass()
cls.course = SampleCourseFactory.create()
# hide the html block
cls.html_block = cls.store.get_item(cls.course.id.make_usage_key('html', 'html_x1a_1'))
cls.html_block.visible_to_staff_only = True
cls.store.update_item(cls.html_block, ModuleStoreEnum.UserID.test)
def setUp(self):
super(TestGetBlocks, self).setUp()
self.course = SampleCourseFactory.create()
self.user = UserFactory.create()
self.request = RequestFactory().get("/dummy")
self.request.user = self.user
......@@ -24,9 +34,11 @@ class TestGetBlocks(ModuleStoreTestCase):
def test_basic(self):
blocks = get_blocks(self.request, self.course.location, self.user)
self.assertEquals(blocks['root'], unicode(self.course.location))
# add 1 for the orphaned course about block
self.assertEquals(len(blocks['blocks']) + 1, len(self.store.get_items(self.course.id)))
# subtract for (1) the orphaned course About block and (2) the hidden Html block
self.assertEquals(len(blocks['blocks']), len(self.store.get_items(self.course.id)) - 2)
self.assertNotIn(unicode(self.html_block.location), blocks['blocks'])
def test_no_user(self):
with self.assertRaises(NotImplementedError):
get_blocks(self.request, self.course.location)
blocks = get_blocks(self.request, self.course.location)
self.assertIn(unicode(self.html_block.location), blocks['blocks'])
......@@ -49,6 +49,7 @@ class TestBlockListGetForm(FormTestMixin, SharedModuleStoreTestCase):
mutable=True,
)
self.cleaned_data = {
'all_blocks': None,
'block_counts': set(),
'depth': 0,
'nav_depth': None,
......@@ -100,8 +101,31 @@ class TestBlockListGetForm(FormTestMixin, SharedModuleStoreTestCase):
#-- user
def test_no_user_param(self):
@ddt.data("True", "true", True)
def test_no_user_all_blocks_true(self, all_blocks_value):
self.initial = {'requesting_user': self.staff}
self.form_data.pop('username')
self.form_data['all_blocks'] = all_blocks_value
self.get_form(expected_valid=True)
@ddt.data("False", "false", False)
def test_no_user_all_blocks_false(self, all_blocks_value):
self.initial = {'requesting_user': self.staff}
self.form_data.pop('username')
self.form_data['all_blocks'] = all_blocks_value
self.assert_error('username', "This field is required unless all_blocks is requested.")
def test_no_user_all_blocks_none(self):
self.initial = {'requesting_user': self.staff}
self.form_data.pop('username')
self.assert_error('username', "This field is required unless all_blocks is requested.")
def test_no_user_non_staff(self):
self.form_data.pop('username')
self.form_data['all_blocks'] = True
self.assert_raises_permission_denied()
def test_nonexistent_user_by_student(self):
......@@ -134,7 +158,7 @@ class TestBlockListGetForm(FormTestMixin, SharedModuleStoreTestCase):
def test_unenrolled_student_by_staff(self):
CourseEnrollment.unenroll(self.student, self.course.id)
self.initial = {'requesting_user': self.staff}
self.assert_raises_permission_denied()
self.get_form(expected_valid=True)
#-- depth
......
......@@ -4,27 +4,31 @@ Tests for Blocks Views
from django.core.urlresolvers import reverse
from string import join
from urllib import urlencode
from urlparse import urlunparse
from opaque_keys.edx.locator import CourseLocator
from student.models import CourseEnrollment
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import ToyCourseFactory
from .test_utils import deserialize_usage_key
class TestBlocksViewMixin(object):
class TestBlocksView(SharedModuleStoreTestCase):
"""
Mixin class for test helpers for BlocksView related classes
Test class for BlocksView
"""
requested_fields = ['graded', 'format', 'student_view_multi_device', 'children', 'not_a_field']
@classmethod
def setup_course(cls):
"""
Create a sample course
"""
cls.course_key = ToyCourseFactory.create().id
def setUpClass(cls):
super(TestBlocksView, cls).setUpClass()
# create a toy course
cls.course_key = ToyCourseFactory.create().id
cls.course_usage_key = cls.store.make_course_usage_key(cls.course_key)
cls.non_orphaned_block_usage_keys = set(
unicode(item.location)
for item in cls.store.get_items(cls.course_key)
......@@ -32,34 +36,39 @@ class TestBlocksViewMixin(object):
if cls.store.get_parent_location(item.location) or item.category == 'course'
)
def setup_user(self):
"""
Create a user, enrolled in the sample course
"""
self.user = UserFactory.create() # pylint: disable=attribute-defined-outside-init
self.client.login(username=self.user.username, password='test')
def setUp(self):
super(TestBlocksView, self).setUp()
# create a user, enrolled in the toy course
self.user = UserFactory.create()
self.client.login(username=self.user.username, password='test')
CourseEnrollmentFactory.create(user=self.user, course_id=self.course_key)
# default values for url and query_params
self.url = reverse(
'blocks_in_block_tree',
kwargs={'usage_key_string': unicode(self.course_usage_key)}
)
self.query_params = {'depth': 'all', 'username': self.user.username}
def verify_response(self, expected_status_code=200, params=None, url=None):
"""
Ensure that the sending a GET request to the specified URL (or self.url)
returns the expected status code (200 by default).
Ensure that sending a GET request to the specified URL returns the
expected status code.
Arguments:
expected_status_code: (default 200)
params:
query parameters to include in the request (includes
username=[self.user.username]&depth=all by default)
url: (default [self.url])
expected_status_code: The status_code that is expected in the
response.
params: Parameters to add to self.query_params to include in the
request.
url: The URL to send the GET request. Default is self.url.
Returns:
response: The HttpResponse returned by the request
"""
query_params = {'username': self.user.username, 'depth': 'all'}
if params:
query_params.update(params)
response = self.client.get(url or self.url, query_params)
self.query_params.update(params)
response = self.client.get(url or self.url, self.query_params)
self.assertEquals(response.status_code, expected_status_code)
return response
......@@ -81,8 +90,6 @@ class TestBlocksViewMixin(object):
self.non_orphaned_block_usage_keys,
)
requested_fields = ['graded', 'format', 'student_view_multi_device', 'children', 'not_a_field']
def verify_response_with_requested_fields(self, response):
"""
Verify the response has the expected structure
......@@ -132,26 +139,6 @@ class TestBlocksViewMixin(object):
else:
self.assertFalse(expression)
# pylint: disable=no-member
class TestBlocksView(TestBlocksViewMixin, SharedModuleStoreTestCase):
"""
Test class for BlocksView
"""
@classmethod
def setUpClass(cls):
super(TestBlocksView, cls).setUpClass()
cls.setup_course()
cls.course_usage_key = cls.store.make_course_usage_key(cls.course_key)
cls.url = reverse(
'blocks_in_block_tree',
kwargs={'usage_key_string': unicode(cls.course_usage_key)}
)
def setUp(self):
super(TestBlocksView, self).setUp()
self.setup_user()
def test_not_authenticated(self):
self.client.logout()
self.verify_response(401)
......@@ -168,6 +155,21 @@ class TestBlocksView(TestBlocksViewMixin, SharedModuleStoreTestCase):
)
self.verify_response(403, url=url)
def test_no_user_non_staff(self):
self.query_params.pop('username')
self.query_params['all_blocks'] = True
self.verify_response(403)
def test_no_user_staff_not_all_blocks(self):
self.query_params.pop('username')
self.verify_response(400)
def test_no_user_staff_all_blocks(self):
self.client.login(username=AdminFactory.create().username, password='test')
self.query_params.pop('username')
self.query_params['all_blocks'] = True
self.verify_response()
def test_basic(self):
response = self.verify_response()
self.assertEquals(response.data['root'], unicode(self.course_usage_key))
......@@ -217,42 +219,34 @@ class TestBlocksView(TestBlocksViewMixin, SharedModuleStoreTestCase):
)
self.verify_response_with_requested_fields(response)
def test_with_list_field_url(self):
query = urlencode(self.query_params.items() + [
('requested_fields', self.requested_fields[0]),
('requested_fields', self.requested_fields[1]),
('requested_fields', join(self.requested_fields[1:], ',')),
])
self.query_params = None
response = self.verify_response(
url=urlunparse(("", "", self.url, "", query, ""))
)
self.verify_response_with_requested_fields(response)
class TestBlocksInCourseView(TestBlocksViewMixin, SharedModuleStoreTestCase):
class TestBlocksInCourseView(TestBlocksView): # pylint: disable=test-inherits-tests
"""
Test class for BlocksInCourseView
"""
@classmethod
def setUpClass(cls):
super(TestBlocksInCourseView, cls).setUpClass()
cls.setup_course()
cls.url = reverse('blocks_in_course')
def setUp(self):
super(TestBlocksInCourseView, self).setUp()
self.setup_user()
def test_basic(self):
response = self.verify_response(params={'course_id': unicode(self.course_key)})
self.verify_response_block_dict(response)
self.url = reverse('blocks_in_course')
self.query_params['course_id'] = unicode(self.course_key)
def test_no_course_id(self):
self.query_params.pop('course_id')
self.verify_response(400)
def test_invalid_course_id(self):
self.verify_response(400, params={'course_id': 'invalid_course_id'})
def test_with_list_field_url(self):
url = '{base_url}?course_id={course_id}&username={username}&depth=all'.format(
course_id=unicode(self.course_key),
base_url=self.url.format(),
username=self.user.username,
)
url += '&requested_fields={0}&requested_fields={1}&requested_fields={2}'.format(
self.requested_fields[0],
self.requested_fields[1],
join(self.requested_fields[1:], ','),
)
response = self.client.get(url)
self.assertEquals(response.status_code, 200)
self.verify_response_with_requested_fields(response)
def test_non_existent_course(self):
self.verify_response(403, params={'course_id': unicode(CourseLocator('non', 'existent', 'course'))})
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment