diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 920d2f1545..c25fe55dc9 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -815,7 +815,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): """ Gets a module instance based on its `usage_id` in a course, for a given request/user @@ -887,7 +887,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix): if error_msg: return JsonResponse(object={'success': error_msg}, status=413) - instance, tracking_context = _get_module_by_usage_id(request, course_id, usage_id) + instance, tracking_context = get_module_by_usage_id(request, course_id, usage_id) tracking_context_name = 'module_callback_handler' req = django_to_webob_request(request) @@ -945,7 +945,7 @@ def xblock_view(request, course_id, usage_id, view_name): if not request.user.is_authenticated(): raise PermissionDenied - instance, _ = _get_module_by_usage_id(request, course_id, usage_id) + instance, _ = get_module_by_usage_id(request, course_id, usage_id) try: fragment = instance.render(view_name, context=request.GET) diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py index fdee142fdb..f5447916b0 100644 --- a/lms/djangoapps/lti_provider/tests/test_views.py +++ b/lms/djangoapps/lti_provider/tests/test_views.py @@ -23,6 +23,38 @@ LTI_DEFAULT_PARAMS = { } +COURSE_PARAMS = { + 'course_id': 'CourseID', + 'usage_id': 'UsageID' +} + + +ALL_PARAMS = dict(LTI_DEFAULT_PARAMS.items() + COURSE_PARAMS.items()) + + +def build_launch_request(authenticated=True): + """ + Helper method to create a new request object for the LTI launch. + """ + request = RequestFactory().post('/') + request.user = UserFactory.create() + request.user.is_authenticated = MagicMock(return_value=authenticated) + request.session = {} + request.POST.update(LTI_DEFAULT_PARAMS) + return request + + +def build_run_request(authenticated=True): + """ + Helper method to create a new request object + """ + request = RequestFactory().get('/') + request.user = UserFactory.create() + request.user.is_authenticated = MagicMock(return_value=authenticated) + request.session = {views.LTI_SESSION_KEY: ALL_PARAMS.copy()} + return request + + class LtiLaunchTest(TestCase): """ Tests for the lti_launch view @@ -34,30 +66,20 @@ class LtiLaunchTest(TestCase): # Always accept the OAuth signature SignatureValidator.verify = MagicMock(return_value=True) - def build_request(self, authenticated=True): - """ - Helper method to create a new request object for the LTI launch. - """ - request = RequestFactory().post('/') - request.user = UserFactory.create() - request.user.is_authenticated = MagicMock(return_value=authenticated) - request.session = {} - request.POST.update(LTI_DEFAULT_PARAMS) - return request - - def test_valid_launch(self): + @patch('lti_provider.views.render_courseware') + def test_valid_launch(self, render): """ Verifies that the LTI launch succeeds when passed a valid request. """ - request = self.build_request() - response = views.lti_launch(request, None, None) - self.assertEqual(response.status_code, 200) + request = build_launch_request() + views.lti_launch(request, COURSE_PARAMS['course_id'], COURSE_PARAMS['usage_id']) + render.assert_called_with(request, ALL_PARAMS) def launch_with_missing_parameter(self, missing_param): """ Helper method to remove a parameter from the LTI launch and call the view """ - request = self.build_request() + request = build_launch_request() del request.POST[missing_param] return views.lti_launch(request, None, None) @@ -79,7 +101,7 @@ class LtiLaunchTest(TestCase): is not set """ with patch.dict('django.conf.settings.FEATURES', {'ENABLE_LTI_PROVIDER': False}): - request = self.build_request() + request = build_launch_request() response = views.lti_launch(request, None, None) self.assertEqual(response.status_code, 403) @@ -89,8 +111,8 @@ class LtiLaunchTest(TestCase): Verifies that the LTI parameters and the course and usage IDs are properly stored in the session """ - request = self.build_request() - views.lti_launch(request, 'CourseID', 'UsageID') + request = build_launch_request() + views.lti_launch(request, COURSE_PARAMS['course_id'], COURSE_PARAMS['usage_id']) session = request.session[views.LTI_SESSION_KEY] self.assertEqual(session['course_id'], 'CourseID', 'Course ID not set in the session') self.assertEqual(session['usage_id'], 'UsageID', 'Usage ID not set in the session') @@ -103,7 +125,7 @@ class LtiLaunchTest(TestCase): user, the response will redirect to the login page with the correct URL """ - request = self.build_request(False) + request = build_launch_request(False) response = views.lti_launch(request, None, None) self.assertEqual(response.status_code, 302) self.assertEqual(response['Location'], '/accounts/login?next=/lti_provider/lti_run') @@ -114,7 +136,7 @@ class LtiLaunchTest(TestCase): incorrect. """ SignatureValidator.verify = MagicMock(return_value=False) - request = self.build_request() + request = build_launch_request() response = views.lti_launch(request, None, None) self.assertEqual(response.status_code, 403) @@ -124,32 +146,21 @@ class LtiRunTest(TestCase): Tests for the lti_run view """ - def build_request(self, authenticated=True): - """ - Helper method to create a new request object - """ - request = RequestFactory().get('/') - request.user = UserFactory.create() - request.user.is_authenticated = MagicMock(return_value=authenticated) - params = {'course_id': 'CourseID', 'usage_id': 'UsageID'} - params.update(LTI_DEFAULT_PARAMS) - request.session = {views.LTI_SESSION_KEY: params} - return request - - def test_valid_launch(self): + @patch('lti_provider.views.render_courseware') + def test_valid_launch(self, render): """ Verifies that the view returns OK if called with the correct context """ - request = self.build_request() + request = build_run_request() response = views.lti_run(request) - self.assertEqual(response.status_code, 200) + render.assert_called_with(request, ALL_PARAMS) def test_forbidden_if_session_key_missing(self): """ Verifies that the lti_run view returns a Forbidden status if the session doesn't have an entry for the LTI parameters. """ - request = self.build_request() + request = build_run_request() del request.session[views.LTI_SESSION_KEY] response = views.lti_run(request) self.assertEqual(response.status_code, 403) @@ -161,7 +172,7 @@ class LtiRunTest(TestCase): """ extra_keys = ['course_id', 'usage_id'] for key in views.REQUIRED_PARAMETERS + extra_keys: - request = self.build_request() + request = build_run_request() del request.session[views.LTI_SESSION_KEY][key] response = views.lti_run(request) self.assertEqual( @@ -170,11 +181,105 @@ class LtiRunTest(TestCase): 'Expected Forbidden response when session is missing ' + key ) - def test_session_cleared_in_view(self): + @patch('lti_provider.views.render_courseware') + def test_session_cleared_in_view(self, _render): """ Verifies that the LTI parameters are cleaned out of the session after launching the view to prevent a launch being replayed. """ - request = self.build_request() + request = build_run_request() views.lti_run(request) self.assertNotIn(views.LTI_SESSION_KEY, request.session) + + +class RenderCoursewareTest(TestCase): + """ + Tests for the render_courseware method + """ + + def setUp(self): + """ + Configure mocks for all the dependencies of the render method + """ + super(RenderCoursewareTest, self).setUp() + self.module_instance = MagicMock() + self.module_instance.render.return_value = "Fragment" + self.render_mock = self.setup_patch('lti_provider.views.render_to_response', 'Rendered page') + self.module_mock = self.setup_patch('lti_provider.views.get_module_by_usage_id', (self.module_instance, None)) + self.access_mock = self.setup_patch('lti_provider.views.has_access', 'StaffAccess') + self.course_mock = self.setup_patch('lti_provider.views.get_course_with_access', 'CourseWithAccess') + self.key_mock = self.setup_patch('lti_provider.views.CourseKey.from_string', 'CourseKey') + + def setup_patch(self, function_name, return_value): + """ + Patch a method with a given return value, and return the mock + """ + mock = MagicMock(return_value=return_value) + new_patch = patch(function_name, new=mock) + new_patch.start() + self.addCleanup(new_patch.stop) + return mock + + def test_valid_launch(self): + """ + Verify that the method renders a response when launched correctly + """ + request = build_run_request() + response = views.render_courseware(request, ALL_PARAMS.copy()) + self.assertEqual(response, 'Rendered page') + + def test_course_key(self): + """ + Verify that the correct course key is requested + """ + request = build_run_request() + views.render_courseware(request, ALL_PARAMS.copy()) + self.key_mock.assert_called_with(ALL_PARAMS['course_id']) + + def test_course_with_access(self): + """ + Verify that get_course_with_access is called with the right parameters + """ + request = build_run_request() + views.render_courseware(request, ALL_PARAMS.copy()) + self.course_mock.assert_called_with(request.user, 'load', 'CourseKey') + + def test_has_access(self): + """ + Verify that has_access is called with the right parameters + """ + request = build_run_request() + views.render_courseware(request, ALL_PARAMS.copy()) + self.access_mock.assert_called_with(request.user, 'staff', 'CourseWithAccess') + + def test_get_module(self): + """ + Verify that get_module_by_usage_id is called with the right parameters + """ + request = build_run_request() + views.render_courseware(request, ALL_PARAMS.copy()) + self.module_mock.assert_called_with(request, ALL_PARAMS['course_id'], ALL_PARAMS['usage_id']) + + def test_render(self): + """ + Verify that render is called on the right object with the right parameters + """ + request = build_run_request() + views.render_courseware(request, ALL_PARAMS.copy()) + self.module_instance.render.assert_called_with('student_view', context={}) + + def test_context(self): + expected_context = { + 'fragment': 'Fragment', + 'course': 'CourseWithAccess', + 'disable_accordion': True, + 'allow_iframing': True, + 'disable_header': True, + 'disable_footer': True, + 'disable_tabs': True, + 'staff_access': 'StaffAccess', + 'xqa_server': 'http://example.com/xqa', + } + request = build_run_request() + views.render_courseware(request, ALL_PARAMS.copy()) + self.render_mock.assert_called_with('courseware/courseware.html', expected_context) diff --git a/lms/djangoapps/lti_provider/views.py b/lms/djangoapps/lti_provider/views.py index b74b8d193d..491b200d07 100644 --- a/lms/djangoapps/lti_provider/views.py +++ b/lms/djangoapps/lti_provider/views.py @@ -6,10 +6,15 @@ from django.conf import settings from django.contrib.auth.decorators import login_required from django.contrib.auth.views import redirect_to_login from django.core.urlresolvers import reverse -from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden +from django.http import HttpResponseBadRequest, HttpResponseForbidden from django.views.decorators.csrf import csrf_exempt +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.signature_validator import SignatureValidator +from opaque_keys.edx.keys import CourseKey # LTI launch parameters that must be present for a successful launch REQUIRED_PARAMETERS = [ @@ -97,7 +102,7 @@ def lti_run(request): # Remove the parameters from the session to prevent replay del request.session[LTI_SESSION_KEY] - return render_courseware() + return render_courseware(request, params) def get_required_parameters(dictionary, additional_params=None): @@ -139,7 +144,7 @@ def restore_params_from_session(request): return get_required_parameters(session_params, additional_params) -def render_courseware(): +def render_courseware(request, lti_params): """ Render the content requested for the LTI launch. TODO: This method depends on the current refactoring work on the @@ -149,4 +154,26 @@ def render_courseware(): :return: an HttpResponse object that contains the template and necessary context to render the courseware. """ - return HttpResponse('TODO: Render refactored courseware view.') + usage_id = lti_params['usage_id'] + course_id = lti_params['course_id'] + course_key = CourseKey.from_string(course_id) + user = request.user + course = get_course_with_access(user, 'load', course_key) + staff = has_access(request.user, 'staff', course) + instance, _ = get_module_by_usage_id(request, course_id, usage_id) + + fragment = instance.render('student_view', context=request.GET) + + context = { + 'fragment': fragment, + 'course': course, + 'disable_accordion': True, + 'allow_iframing': True, + 'disable_header': True, + 'disable_footer': True, + 'disable_tabs': True, + 'staff_access': staff, + 'xqa_server': settings.FEATURES.get('USE_XQA_SERVER', 'http://example.com/xqa'), + } + + return render_to_response('courseware/courseware.html', context) diff --git a/lms/templates/courseware/xqa_interface.html b/lms/templates/courseware/xqa_interface.html index 1bc88878fb..9322c4393a 100644 --- a/lms/templates/courseware/xqa_interface.html +++ b/lms/templates/courseware/xqa_interface.html @@ -51,7 +51,7 @@ function sendlog(element_id, edit_link, staff_context){ $('#' + element_id + '_xqa_log_data').html(result); }, error: function() { - alert('Error: cannot connect to XQA server'); + alert('Error: cannot connect to XQA server. Check the value of the USE_XQA_SERVER feature.'); console.log('error!'); } }); @@ -78,7 +78,7 @@ function getlog(element_id, staff_context){ $('#' + element_id + '_xqa_log_data').html(result); }, error: function() { - alert('Error: cannot connect to XQA server'); + alert('Error: cannot connect to XQA server. Check the value of the USE_XQA_SERVER feature.'); } });