[LTI Provider] Addressing review comments

lms/djangoapps/lti_provider/tests/test_views.py

@@ -29,7 +29,7 @@ COURSE_PARAMS = {
 }
 
 
-ALL_PARAMS = dict(list(LTI_DEFAULT_PARAMS.items()) + list(COURSE_PARAMS.items()))
+ALL_PARAMS = dict(LTI_DEFAULT_PARAMS.items() + COURSE_PARAMS.items())
 
 
 def build_launch_request(authenticated=True):
@@ -192,21 +192,35 @@ class LtiRunTest(TestCase):
         self.assertNotIn(views.LTI_SESSION_KEY, request.session)
 
 
-MODULE_INSTANCE = MagicMock()
-MODULE_INSTANCE.render = MagicMock(return_value='Fragment')
-
-
-@patch('lti_provider.views.render_to_response', return_value='Rendered page')
-@patch('lti_provider.views.get_module_by_usage_id', return_value=(MODULE_INSTANCE, None))
-@patch('lti_provider.views.has_access', return_value='StaffAccess')
-@patch('lti_provider.views.get_course_with_access', return_value='CourseWithAccess')
-@patch('lti_provider.views.CourseKey.from_string', return_value='CourseKey')
 class RenderCoursewareTest(TestCase):
     """
     Tests for the render_courseware method
     """
 
-    def test_valid_launch(self, _key_mock, _course_mock, _access_mock, _module_mock, _render_mock):
+    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
         """
@@ -214,47 +228,47 @@ class RenderCoursewareTest(TestCase):
         response = views.render_courseware(request, ALL_PARAMS.copy())
         self.assertEqual(response, 'Rendered page')
 
-    def test_course_key(self, key_mock, _course_mock, _access_mock, _module_mock, _render_mock):
+    def test_course_key(self):
         """
         Verify that the correct course key is requested
         """
         request = build_run_request()
         views.render_courseware(request, ALL_PARAMS.copy())
-        key_mock.assert_called_with(ALL_PARAMS['course_id'])
+        self.key_mock.assert_called_with(ALL_PARAMS['course_id'])
 
-    def test_course_with_access(self, _key_mock, course_mock, _access_mock, _module_mock, _render_mock):
+    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())
-        course_mock.assert_called_with(request.user, 'load', 'CourseKey')
+        self.course_mock.assert_called_with(request.user, 'load', 'CourseKey')
 
-    def test_has_access(self, _key_mock, _course_mock, access_mock, _module_mock, _render_mock):
+    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())
-        access_mock.assert_called_with(request.user, 'staff', 'CourseWithAccess')
+        self.access_mock.assert_called_with(request.user, 'staff', 'CourseWithAccess')
 
-    def test_get_module(self, _key_mock, _course_mock, _access_mock, module_mock, _render_mock):
+    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())
-        module_mock.assert_called_with(request, ALL_PARAMS['course_id'], ALL_PARAMS['usage_id'])
+        self.module_mock.assert_called_with(request, ALL_PARAMS['course_id'], ALL_PARAMS['usage_id'])
 
-    def test_render(self, _key_mock, _course_mock, _access_mock, _module_mock, _render_mock):
+    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())
-        MODULE_INSTANCE.render.assert_called_with('student_view', context={})
+        self.module_instance.render.assert_called_with('student_view', context={})
 
-    def test_context(self, _key_mock, _course_mock, _access_mock, _module_mock, render_mock):
+    def test_context(self):
         expected_context = {
             'fragment': 'Fragment',
             'course': 'CourseWithAccess',
@@ -264,8 +278,8 @@ class RenderCoursewareTest(TestCase):
             'disable_footer': True,
             'disable_tabs': True,
             'staff_access': 'StaffAccess',
-            'xqa_server': 'http://xqa:server@content-qa.mitx.mit.edu/xqa',
+            'xqa_server': 'http://example.com/xqa',
         }
         request = build_run_request()
         views.render_courseware(request, ALL_PARAMS.copy())
-        render_mock.assert_called_with('courseware/courseware.html', expected_context)
+        self.render_mock.assert_called_with('courseware/courseware.html', expected_context)

lms/djangoapps/lti_provider/views.py

@@ -173,7 +173,7 @@ def render_courseware(request, lti_params):
         'disable_footer': True,
         'disable_tabs': True,
         'staff_access': staff,
-        'xqa_server': settings.FEATURES.get('USE_XQA_SERVER', 'http://xqa:server@content-qa.mitx.mit.edu/xqa'),
+        'xqa_server': settings.FEATURES.get('USE_XQA_SERVER', 'http://example.com/xqa'),
     }
 
     return render_to_response('courseware/courseware.html', context)

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.');
         }
     });