From 31d5d513f85ab80007d1c45ff8a4f0ef3bc8c802 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 18 Sep 2015 08:58:37 -0400 Subject: [PATCH] MA-1337 xBlock Rendering View --- lms/djangoapps/courseware/tests/test_views.py | 5 +- lms/djangoapps/courseware/testutils.py | 22 +++++- lms/djangoapps/courseware/views.py | 7 +- .../lti_provider/tests/test_views.py | 15 +++- lms/envs/common.py | 3 +- lms/envs/test.py | 1 - lms/urls.py | 77 +++++++++++++------ 7 files changed, 94 insertions(+), 36 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index cd4f67d839..a5e3cbd538 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1128,14 +1128,15 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): This class overrides the get_response method, which is used by the tests defined in RenderXBlockTestMixin. """ - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_RENDER_XBLOCK_API': True}) def setUp(self): reload_django_url_config() super(TestRenderXBlock, self).setUp() - def get_response(self): + def get_response(self, url_encoded_params=None): """ Overridable method to get the response from the endpoint that is being tested. """ url = reverse('render_xblock', kwargs={"usage_key_string": unicode(self.html_block.location)}) + if url_encoded_params: + url += '?' + url_encoded_params return self.client.get(url) diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index b0e4e8b31f..e6e18deeba 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -6,6 +6,7 @@ from abc import ABCMeta, abstractmethod from datetime import datetime import ddt from mock import patch +from urllib import urlencode from lms.djangoapps.courseware.url_helpers import get_redirect_url from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory @@ -40,9 +41,12 @@ class RenderXBlockTestMixin(object): ] @abstractmethod - def get_response(self): + def get_response(self, url_encoded_params=None): """ Abstract method to get the response from the endpoint that is being tested. + + Arguments: + url_encoded_params - URL encoded parameters that should be appended to the requested URL. """ pass # pragma: no cover @@ -79,11 +83,13 @@ class RenderXBlockTestMixin(object): if login: self.login() - def verify_response(self, expected_response_code=200): + def verify_response(self, expected_response_code=200, url_params=None): """ Helper method that calls the endpoint, verifies the expected response code, and returns the response. """ - response = self.get_response() + if url_params: + url_params = urlencode(url_params) + response = self.get_response(url_params) if expected_response_code == 200: self.assertContains(response, self.html_block.data, status_code=expected_response_code) for chrome_element in [self.COURSEWARE_CHROME_HTML_ELEMENTS + self.XBLOCK_REMOVED_HTML_ELEMENTS]: @@ -175,3 +181,13 @@ class RenderXBlockTestMixin(object): self.html_block.visible_to_staff_only = True modulestore().update_item(self.html_block, self.user.id) self.verify_response(expected_response_code=404) + + def test_student_view_param(self): + self.setup_course() + self.setup_user(admin=False, enroll=True, login=True) + self.verify_response(url_params={'view': 'student_view'}) + + def test_unsupported_view_param(self): + self.setup_course() + self.setup_user(admin=False, enroll=True, login=True) + self.verify_response(url_params={'view': 'author_view'}, expected_response_code=400) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index d1d595317e..7cc48e9286 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -5,12 +5,9 @@ Courseware views functions import logging import urllib import json -import cgi from datetime import datetime -from django.utils import translation from django.utils.translation import ugettext as _ -from django.utils.translation import ungettext from django.conf import settings from django.core.context_processors import csrf @@ -1415,6 +1412,10 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) course_key = usage_key.course_key + requested_view = request.GET.get('view', 'student_view') + if requested_view != 'student_view': + return HttpResponseBadRequest("Rendering of the xblock view '{}' is not supported.".format(requested_view)) + with modulestore().bulk_operations(course_key): # verify the user has access to the course, including enrollment check try: diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py index 44fc311332..6533f18a1a 100644 --- a/lms/djangoapps/lti_provider/tests/test_views.py +++ b/lms/djangoapps/lti_provider/tests/test_views.py @@ -163,7 +163,7 @@ class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCa This class overrides the get_response method, which is used by the tests defined in RenderXBlockTestMixin. """ - def get_response(self): + def get_response(self, url_encoded_params=None): """ Overridable method to get the response from the endpoint that is being tested. """ @@ -174,15 +174,28 @@ class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCa 'usage_id': unicode(self.html_block.location) } ) + if url_encoded_params: + lti_launch_url += '?' + url_encoded_params SignatureValidator.verify = MagicMock(return_value=True) return self.client.post(lti_launch_url, data=LTI_DEFAULT_PARAMS) + # The following test methods override the base tests for verifying access + # by unenrolled and unauthenticated students, since there is a discrepancy + # of access rules between the 2 endpoints (LTI and xBlock_render). + # TODO fix this access discrepancy to the same underlying data. + def test_unenrolled_student(self): + """ + Override since LTI allows access to unenrolled students. + """ self.setup_course() self.setup_user(admin=False, enroll=False, login=True) self.verify_response() def test_unauthenticated(self): + """ + Override since LTI allows access to unauthenticated users. + """ self.setup_course() self.setup_user(admin=False, enroll=True, login=False) self.verify_response() diff --git a/lms/envs/common.py b/lms/envs/common.py index 41924c2217..4ae34ef7e2 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -319,8 +319,7 @@ FEATURES = { 'ENABLE_MOBILE_REST_API': False, 'ENABLE_MOBILE_SOCIAL_FACEBOOK_FEATURES': False, - # Enable APIs required for xBlocks on Mobile, and supported in general - 'ENABLE_RENDER_XBLOCK_API': False, + # Enable temporary APIs required for xBlocks on Mobile 'ENABLE_COURSE_BLOCKS_NAVIGATION_API': False, # Enable the combined login/registration form diff --git a/lms/envs/test.py b/lms/envs/test.py index d371d14ae5..c695f7700a 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -277,7 +277,6 @@ FEATURES['ENABLE_MOBILE_REST_API'] = True FEATURES['ENABLE_MOBILE_SOCIAL_FACEBOOK_FEATURES'] = True FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True FEATURES['ENABLE_COURSE_BLOCKS_NAVIGATION_API'] = True -FEATURES['ENABLE_RENDER_XBLOCK_API'] = True ###################### Payment ##############################3 # Enable fake payment processing page diff --git a/lms/urls.py b/lms/urls.py index abd2236535..19d7f09081 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -248,24 +248,62 @@ if settings.COURSEWARE_ENABLED: ) ) urlpatterns += ( - url(r'^courses/{}/jump_to/(?P.*)$'.format(settings.COURSE_ID_PATTERN), - 'courseware.views.jump_to', name="jump_to"), - url(r'^courses/{}/jump_to_id/(?P.*)$'.format(settings.COURSE_ID_PATTERN), - 'courseware.views.jump_to_id', name="jump_to_id"), - url(r'^courses/{course_key}/xblock/{usage_key}/handler/(?P[^/]*)(?:/(?P.*))?$'.format(course_key=settings.COURSE_ID_PATTERN, usage_key=settings.USAGE_ID_PATTERN), + # jump_to URLs for direct access to a location in the course + url( + r'^courses/{}/jump_to/(?P.*)$'.format(settings.COURSE_ID_PATTERN), + 'courseware.views.jump_to', name="jump_to", + ), + url( + r'^courses/{}/jump_to_id/(?P.*)$'.format(settings.COURSE_ID_PATTERN), + 'courseware.views.jump_to_id', name="jump_to_id", + ), + + # xblock Handler APIs + url( + r'^courses/{course_key}/xblock/{usage_key}/handler/(?P[^/]*)(?:/(?P.*))?$'.format( + course_key=settings.COURSE_ID_PATTERN, + usage_key=settings.USAGE_ID_PATTERN, + ), 'courseware.module_render.handle_xblock_callback', - name='xblock_handler'), - url(r'^courses/{course_key}/xblock/{usage_key}/view/(?P[^/]*)$'.format( - course_key=settings.COURSE_ID_PATTERN, - usage_key=settings.USAGE_ID_PATTERN), - 'courseware.module_render.xblock_view', - name='xblock_view'), - url(r'^courses/{course_key}/xblock/{usage_key}/handler_noauth/(?P[^/]*)(?:/(?P.*))?$'.format(course_key=settings.COURSE_ID_PATTERN, usage_key=settings.USAGE_ID_PATTERN), + name='xblock_handler', + ), + url( + r'^courses/{course_key}/xblock/{usage_key}/handler_noauth/(?P[^/]*)(?:/(?P.*))?$'.format( + course_key=settings.COURSE_ID_PATTERN, + usage_key=settings.USAGE_ID_PATTERN, + ), 'courseware.module_render.handle_xblock_callback_noauth', - name='xblock_handler_noauth'), - url(r'xblock/resource/(?P[^/]+)/(?P.*)$', + name='xblock_handler_noauth', + ), + + # xblock View API + # (unpublished) API that returns JSON with the HTML fragment and related resources + # for the xBlock's requested view. + url( + r'^courses/{course_key}/xblock/{usage_key}/view/(?P[^/]*)$'.format( + course_key=settings.COURSE_ID_PATTERN, + usage_key=settings.USAGE_ID_PATTERN, + ), + 'courseware.module_render.xblock_view', + name='xblock_view', + ), + + # xblock Rendering View URL + # URL to provide an HTML view of an xBlock. The view type (e.g., student_view) is + # passed as a "view" parameter to the URL. + # Note: This is not an API. Compare this with the xblock_view API above. + url( + r'^xblock/{usage_key_string}$'.format(usage_key_string=settings.USAGE_KEY_PATTERN), + 'courseware.views.render_xblock', + name='render_xblock', + ), + + # xblock Resource URL + url( + r'xblock/resource/(?P[^/]+)/(?P.*)$', 'courseware.module_render.xblock_resource', - name='xblock_resource_url'), + name='xblock_resource_url', + ), # Software Licenses @@ -440,15 +478,6 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/{}/teams'.format(settings.COURSE_ID_PATTERN), include('teams.urls'), name="teams_endpoints"), ) - if settings.FEATURES.get('ENABLE_RENDER_XBLOCK_API'): - # TODO (MA-789) This endpoint path still needs to be approved by the arch council. - # Until then, keep the version at v0. - urlpatterns += ( - url(r'api/xblock/v0/xblock/{usage_key_string}$'.format(usage_key_string=settings.USAGE_KEY_PATTERN), - 'courseware.views.render_xblock', - name='render_xblock'), - ) - # allow course staff to change to student view of courseware if settings.FEATURES.get('ENABLE_MASQUERADE'): urlpatterns += (