From d240785b17bdcaeb11e4ef24ee4a3697da26c9b4 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 27 May 2015 11:48:53 -0400 Subject: [PATCH] MA-722 Render xBlock API Support --- common/djangoapps/util/url.py | 22 +++ lms/djangoapps/courseware/access.py | 32 ++-- lms/djangoapps/courseware/courses.py | 24 +-- lms/djangoapps/courseware/module_render.py | 73 +++++--- lms/djangoapps/courseware/tests/test_views.py | 23 +++ lms/djangoapps/courseware/testutils.py | 176 ++++++++++++++++++ lms/djangoapps/courseware/views.py | 40 +++- lms/djangoapps/discussion_api/api.py | 2 +- .../django_comment_client/base/views.py | 2 +- .../django_comment_client/forum/views.py | 10 +- .../lti_provider/tests/test_views.py | 121 +++--------- lms/djangoapps/lti_provider/urls.py | 6 +- lms/djangoapps/lti_provider/views.py | 37 +--- lms/envs/common.py | 1 + .../courseware/course_navigation.html | 2 +- lms/templates/main.html | 4 + lms/templates/staff_problem_info.html | 4 +- lms/urls.py | 9 + openedx/core/lib/api/view_utils.py | 3 +- openedx/core/lib/xblock_utils.py | 3 +- 20 files changed, 387 insertions(+), 207 deletions(-) create mode 100644 common/djangoapps/util/url.py create mode 100644 lms/djangoapps/courseware/testutils.py diff --git a/common/djangoapps/util/url.py b/common/djangoapps/util/url.py new file mode 100644 index 0000000000..c90d83dea2 --- /dev/null +++ b/common/djangoapps/util/url.py @@ -0,0 +1,22 @@ +""" +Utility functions related to urls. +""" + +import sys +from django.conf import settings +from django.core.urlresolvers import set_urlconf +from django.utils.importlib import import_module + + +def reload_django_url_config(): + """ + Reloads Django's URL config. + This is useful, for example, when a test enables new URLs + with a django setting and the URL config needs to be refreshed. + """ + urlconf = settings.ROOT_URLCONF + if urlconf and urlconf in sys.modules: + reload(sys.modules[urlconf]) + reloaded = import_module(urlconf) + reloaded_urls = getattr(reloaded, 'urlpatterns') + set_urlconf(tuple(reloaded_urls)) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 8824b9e7db..372cc04c2b 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -1,7 +1,14 @@ """ This file contains (or should), all access control logic for the courseware. Ideally, it will be the only place that needs to know about any special settings -like DISABLE_START_DATES +like DISABLE_START_DATES. + +Note: The access control logic in this file does NOT check for enrollment in + a course. It is expected that higher layers check for enrollment so we + don't have to hit the enrollments table on every module load. + + If enrollment is to be checked, use get_course_with_access in courseware.courses. + It is a wrapper around has_access that additionally checks for enrollment. """ import logging from datetime import datetime, timedelta @@ -27,7 +34,7 @@ from xmodule.util.django import get_current_request_hostname from external_auth.models import ExternalAuthMap from courseware.masquerade import get_masquerade_role, is_masquerading_as_student from student import auth -from student.models import CourseEnrollment, CourseEnrollmentAllowed +from student.models import CourseEnrollmentAllowed from student.roles import ( GlobalStaff, CourseStaffRole, CourseInstructorRole, OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole @@ -140,18 +147,6 @@ def _has_access_course_desc(user, action, course): # delegate to generic descriptor check to check start dates return _has_access_descriptor(user, 'load', course, course.id) - def can_load_forum(): - """ - Can this user access the forums in this course? - """ - return ( - can_load() and - ( - CourseEnrollment.is_enrolled(user, course.id) or - _has_staff_access_to_descriptor(user, course, course.id) - ) - ) - def can_load_mobile(): """ Can this user access this course from a mobile device? @@ -164,12 +159,8 @@ def _has_access_course_desc(user, action, course): ( # either is a staff user or _has_staff_access_to_descriptor(user, course, course.id) or - ( - # check enrollment - CourseEnrollment.is_enrolled(user, course.id) and - # check for unfulfilled milestones - not any_unfulfilled_milestones(course.id, user.id) - ) + # check for unfulfilled milestones + not any_unfulfilled_milestones(course.id, user.id) ) ) @@ -294,7 +285,6 @@ def _has_access_course_desc(user, action, course): checkers = { 'load': can_load, 'view_courseware_with_prerequisites': can_view_courseware_with_prerequisites, - 'load_forum': can_load_forum, 'load_mobile': can_load_mobile, 'enroll': can_enroll, 'see_exists': see_exists, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 192ba1943f..af3618c466 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -92,33 +92,27 @@ def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled= Raises a 404 if the course_key is invalid, or the user doesn't have access. depth: The number of levels of children for the modulestore to cache. None means infinite depth + + check_if_enrolled: If true, additionally verifies that the user is either enrolled in the course + or has staff access. """ assert isinstance(course_key, CourseKey) course = get_course_by_id(course_key, depth=depth) if not has_access(user, action, course, course_key): - if check_if_enrolled and not CourseEnrollment.is_enrolled(user, course_key): - # If user is not enrolled, raise UserNotEnrolled exception that will - # be caught by middleware - raise UserNotEnrolled(course_key) - # Deliberately return a non-specific error message to avoid # leaking info about access control settings raise Http404("Course not found.") + if check_if_enrolled: + # Verify that the user is either enrolled in the course or a staff member. + # If user is not enrolled, raise UserNotEnrolled exception that will be caught by middleware. + if not ((user.id and CourseEnrollment.is_enrolled(user, course_key)) or has_access(user, 'staff', course)): + raise UserNotEnrolled(course_key) + return course -def get_opt_course_with_access(user, action, course_key): - """ - Same as get_course_with_access, except that if course_key is None, - return None without performing any access checks. - """ - if course_key is None: - return None - return get_course_with_access(user, action, course_key) - - def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 3981e59ff4..9a1d844aa3 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -8,13 +8,11 @@ import logging import mimetypes import static_replace -import xblock.reference.plugins from collections import OrderedDict from functools import partial from requests.auth import HTTPBasicAuth import dogstats_wrapper as dog_stats_api -from opaque_keys import InvalidKeyError from django.conf import settings from django.contrib.auth.models import User @@ -37,42 +35,42 @@ from courseware.entrance_exams import ( get_entrance_exam_score, user_must_complete_entrance_exam ) +from edxmako.shortcuts import render_to_string +from eventtracking import tracker from lms.djangoapps.lms_xblock.field_data import LmsFieldData from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig -from edxmako.shortcuts import render_to_string -from eventtracking import tracker -from psychometrics.psychoanalyze import make_psychometrics_data_update_handler -from student.models import anonymous_id_for_user, user_by_anonymous_id -from student.roles import CourseBetaTesterRole -from xblock.core import XBlock -from xblock.fields import Scope -from xblock.runtime import KvsFieldData, KeyValueStore -from xblock.exceptions import NoSuchHandlerError, NoSuchViewError -from xblock.django.request import django_to_webob_request, webob_to_django_response -from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor -from xmodule.exceptions import NotFoundError, ProcessingError +from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey -from xmodule.contentstore.django import contentstore -from xmodule.modulestore.django import modulestore, ModuleI18nService -from xmodule.modulestore.exceptions import ItemNotFoundError from openedx.core.lib.xblock_utils import ( replace_course_urls, replace_jump_to_id_urls, replace_static_urls, add_staff_markup, wrap_xblock, - request_token + request_token as xblock_request_token, ) +from psychometrics.psychoanalyze import make_psychometrics_data_update_handler +from student.models import anonymous_id_for_user, user_by_anonymous_id +from student.roles import CourseBetaTesterRole +from xblock.core import XBlock +from xblock.django.request import django_to_webob_request, webob_to_django_response +from xblock_django.user_service import DjangoXBlockUserService +from xblock.exceptions import NoSuchHandlerError, NoSuchViewError +from xblock.reference.plugins import FSService +from xblock.runtime import KvsFieldData +from xmodule.contentstore.django import contentstore +from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor +from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.modulestore.django import modulestore, ModuleI18nService from xmodule.lti_module import LTIModule +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import XModuleDescriptor from xmodule.mixin import wrap_with_license -from xblock_django.user_service import DjangoXBlockUserService from util.json_request import JsonResponse from util.sandboxing import can_execute_unsafe_code, get_python_lib_zip from util import milestones_helpers -from util.module_utils import yield_dynamic_descriptor_descendents from verify_student.services import ReverificationService from .field_overrides import OverrideFieldData @@ -255,10 +253,12 @@ def get_xqueue_callback_url_prefix(request): def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_key, position=None, wrap_xmodule_display=True, grade_bucket_type=None, - static_asset_path=''): + static_asset_path='', disable_staff_debug_info=False): """ Implements get_module, extracting out the request-specific functionality. + disable_staff_debug_info : If this is True, exclude staff debug information in the rendering of the module. + See get_module() docstring for further details. """ track_function = make_track_function(request) @@ -278,15 +278,16 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours grade_bucket_type=grade_bucket_type, static_asset_path=static_asset_path, user_location=user_location, - request_token=request_token(request), + request_token=xblock_request_token(request), + disable_staff_debug_info=disable_staff_debug_info, ) -def get_module_system_for_user(user, field_data_cache, +def get_module_system_for_user(user, field_data_cache, # TODO # pylint: disable=too-many-statements # Arguments preceding this comment have user binding, those following don't descriptor, course_id, track_function, xqueue_callback_url_prefix, request_token, position=None, wrap_xmodule_display=True, grade_bucket_type=None, - static_asset_path='', user_location=None): + static_asset_path='', user_location=None, disable_staff_debug_info=False): """ Helper function that returns a module system and student_data bound to a user and a descriptor. @@ -309,7 +310,9 @@ def get_module_system_for_user(user, field_data_cache, student_data = KvsFieldData(DjangoKeyValueStore(field_data_cache)) def make_xqueue_callback(dispatch='score_update'): - # Fully qualified callback URL for external queueing system + """ + Returns fully qualified callback URL for external queueing system + """ relative_xqueue_callback_url = reverse( 'xqueue_callback', kwargs=dict( @@ -573,7 +576,7 @@ def get_module_system_for_user(user, field_data_cache, if settings.FEATURES.get('DISPLAY_DEBUG_INFO_TO_STAFF'): if has_access(user, 'staff', descriptor, course_id): has_instructor_access = has_access(user, 'instructor', descriptor, course_id) - block_wrappers.append(partial(add_staff_markup, user, has_instructor_access)) + block_wrappers.append(partial(add_staff_markup, user, has_instructor_access, disable_staff_debug_info)) # These modules store data using the anonymous_student_id as a key. # To prevent loss of data, we will continue to provide old modules with @@ -637,7 +640,7 @@ def get_module_system_for_user(user, field_data_cache, get_real_user=user_by_anonymous_id, services={ 'i18n': ModuleI18nService(), - 'fs': xblock.reference.plugins.FSService(), + 'fs': FSService(), 'field-data': field_data, 'user': DjangoXBlockUserService(user, user_is_staff=user_is_staff), "reverification": ReverificationService() @@ -681,7 +684,7 @@ def get_module_system_for_user(user, field_data_cache, def get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id, # pylint: disable=invalid-name track_function, xqueue_callback_url_prefix, request_token, position=None, wrap_xmodule_display=True, grade_bucket_type=None, - static_asset_path='', user_location=None): + static_asset_path='', user_location=None, disable_staff_debug_info=False): """ Actually implement get_module, without requiring a request. @@ -703,7 +706,8 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours grade_bucket_type=grade_bucket_type, static_asset_path=static_asset_path, user_location=user_location, - request_token=request_token + request_token=request_token, + disable_staff_debug_info=disable_staff_debug_info, ) descriptor.bind_for_student( @@ -836,7 +840,7 @@ def xblock_resource(request, block_type, uri): # pylint: disable=unused-argumen return HttpResponse(content, mimetype=mimetype) -def get_module_by_usage_id(request, course_id, usage_id): +def get_module_by_usage_id(request, course_id, usage_id, disable_staff_debug_info=False): """ Gets a module instance based on its `usage_id` in a course, for a given request/user @@ -880,7 +884,14 @@ def get_module_by_usage_id(request, course_id, usage_id): descriptor ) setup_masquerade(request, course_id, has_access(user, 'staff', descriptor, course_id)) - instance = get_module(user, request, usage_key, field_data_cache, grade_bucket_type='ajax') + instance = get_module_for_descriptor( + user, + request, + descriptor, + field_data_cache, + usage_key.course_key, + disable_staff_debug_info=disable_staff_debug_info + ) if instance is None: # Either permissions just changed, or someone is trying to be clever # and load something they shouldn't have access to. diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 4b8ed7fbcf..128ab35e65 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -29,12 +29,14 @@ from certificates import api as certs_api from certificates.models import CertificateStatuses, CertificateGenerationConfiguration from certificates.tests.factories import GeneratedCertificateFactory from course_modes.models import CourseMode +from courseware.testutils import RenderXBlockTestMixin from courseware.tests.factories import StudentModuleFactory from edxmako.middleware import MakoMiddleware from edxmako.tests import mako_middleware_process_request from student.models import CourseEnrollment from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory from util.tests.test_date_utils import fake_ugettext, fake_pgettext +from util.url import reload_django_url_config from util.views import ensure_valid_course_key from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -584,6 +586,7 @@ class BaseDueDateTests(ModuleStoreTestCase): course = modulestore().get_course(course.id) # pylint: disable=no-member self.assertIsNotNone(course.get_children()[0].get_children()[0].due) + CourseEnrollmentFactory(user=self.user, course_id=course.id) return course def setUp(self): @@ -752,6 +755,7 @@ class ProgressPageTests(ModuleStoreTestCase): grade_cutoffs={u'çü†øƒƒ': 0.75, 'Pass': 0.5}, ) self.course = modulestore().get_course(course.id) # pylint: disable=no-member + CourseEnrollmentFactory(user=self.user, course_id=self.course.id) self.chapter = ItemFactory.create(category='chapter', parent_location=self.course.location) # pylint: disable=no-member self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location) @@ -1087,3 +1091,22 @@ class TestIndexView(ModuleStoreTestCase): # Trigger the assertions embedded in the ViewCheckerBlocks response = views.index(request, unicode(course.id), chapter=chapter.url_name, section=section.url_name) self.assertEquals(response.content.count("ViewCheckerPassed"), 3) + + +class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): + """ + Tests for the courseware.render_xblock endpoint. + 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): + """ + 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)}) + return self.client.get(url) diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py new file mode 100644 index 0000000000..d9664b7077 --- /dev/null +++ b/lms/djangoapps/courseware/testutils.py @@ -0,0 +1,176 @@ +""" +Common test utilities for courseware functionality +""" + +from abc import ABCMeta, abstractmethod +from datetime import datetime +import ddt +from mock import patch + +from lms.djangoapps.courseware.url_helpers import get_redirect_url +from student.tests.factories import AdminFactory, UserFactory, CourseEnrollmentFactory +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls + + +@ddt.ddt +class RenderXBlockTestMixin(object): + """ + Mixin for testing the courseware.render_xblock function. + It can be used for testing any higher-level endpoint that calls this method. + """ + __metaclass__ = ABCMeta + + # DOM elements that appear in the LMS Courseware, + # but are excluded from the xBlock-only rendering. + COURSEWARE_CHROME_HTML_ELEMENTS = [ + '
[^/]*)$'.format(settings.COURSE_ID_PATTERN), + url( + r'^courses/{course_id}/{usage_id}$'.format( + course_id=settings.COURSE_ID_PATTERN, + usage_id=settings.USAGE_ID_PATTERN + ), 'lti_provider.views.lti_launch', name="lti_provider_launch"), url(r'^lti_run$', 'lti_provider.views.lti_run', name="lti_provider_run"), ) diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py index 33ac680301..6fb9e5093d 100644 --- a/lms/djangoapps/lti_provider/views.py +++ b/lms/djangoapps/lti_provider/views.py @@ -10,10 +10,6 @@ from django.http import HttpResponseBadRequest, HttpResponseForbidden, Http404 from django.views.decorators.csrf import csrf_exempt import logging -from courseware.access import has_access -from courseware.courses import get_course_with_access -from courseware.module_render import get_module_by_usage_id -from edxmako.shortcuts import render_to_response from lti_provider.outcomes import store_outcome_parameters from lti_provider.models import LtiConsumer from lti_provider.signature_validator import SignatureValidator @@ -139,7 +135,7 @@ def lti_run(request): ) store_outcome_parameters(params, request.user, lti_consumer) - return render_courseware(request, params) + return render_courseware(request, params['usage_key']) def get_required_parameters(dictionary, additional_params=None): @@ -197,40 +193,19 @@ def restore_params_from_session(request): return session_params -def render_courseware(request, lti_params): +def render_courseware(request, usage_key): """ Render the content requested for the LTI launch. TODO: This method depends on the current refactoring work on the courseware/courseware.html template. It's signature may change depending on the requirements for that template once the refactoring is complete. - :return: an HttpResponse object that contains the template and necessary + Return an HttpResponse object that contains the template and necessary context to render the courseware. """ - usage_key = lti_params['usage_key'] - course_key = lti_params['course_key'] - user = request.user - course = get_course_with_access(user, 'load', course_key) - staff = has_access(request.user, 'staff', course) - instance, _dummy = get_module_by_usage_id( - request, - unicode(course_key), - unicode(usage_key) - ) - - fragment = instance.render('student_view', context=request.GET) - - context = { - 'fragment': fragment, - 'course': course, - 'disable_accordion': True, - 'allow_iframing': True, - 'disable_header': True, - 'staff_access': staff, - 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://example.com/xqa'), - } - - return render_to_response('courseware/courseware-chromeless.html', context) + # return an HttpResponse object that contains the template and necessary context to render the courseware. + from courseware.views import render_xblock + return render_xblock(request, unicode(usage_key)) def parse_course_and_usage_keys(course_id, usage_id): diff --git a/lms/envs/common.py b/lms/envs/common.py index 03bfe9a4ea..7292ea9519 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -321,6 +321,7 @@ FEATURES = { # ENABLE_OAUTH2_PROVIDER to True 'ENABLE_MOBILE_REST_API': False, 'ENABLE_MOBILE_SOCIAL_FACEBOOK_FEATURES': False, + 'ENABLE_RENDER_XBLOCK_API': False, # Enable the combined login/registration form 'ENABLE_COMBINED_LOGIN_REGISTRATION': False, diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index c5911912e4..3efe49986f 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -21,7 +21,7 @@ def url_class(is_active): %> <% cohorted_user_partition = get_cohorted_user_partition(course.id) - show_preview_menu = staff_access and active_page in ['courseware', 'info'] + show_preview_menu = not disable_preview_menu and staff_access and active_page in ['courseware', 'info'] is_student_masquerade = masquerade and masquerade.role == 'student' masquerade_group_id = masquerade.group_id if masquerade else None %> diff --git a/lms/templates/main.html b/lms/templates/main.html index 1115dcab34..bd1f5239b6 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -129,7 +129,9 @@ from branding import api as branding_api +% if not disable_window_wrap:
+% endif #content">${_("Skip to main content")} % if not disable_header: @@ -159,7 +161,9 @@ from branding import api as branding_api % endif +% if not disable_window_wrap:
+% endif % if not disable_courseware_js: <%static:js group='application'/> diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index f6be75e07a..f146db3eaa 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -20,7 +20,8 @@ ${block_content} % endif % endif -