From 345d2af5406cbd76541e3082f5a461f767715e1b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 22 Jul 2013 20:44:39 -0400 Subject: [PATCH] address review feedback --- lms/djangoapps/courseware/module_render.py | 8 +++++--- lms/djangoapps/courseware/tests/test_module_render.py | 8 ++++++-- lms/djangoapps/courseware/tests/test_views.py | 8 ++++---- lms/djangoapps/courseware/views.py | 7 ++++--- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6d8b244f27..102aac651b 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -400,9 +400,11 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours # know the hierarchy # NOTE: module_id is empty string here. The 'module_id' will get assigned in the replacement # function, we just need to specify something to get the reverse() to work - module.get_html = replace_jump_to_id_urls(module.get_html, course_id, - reverse('jump_to_id', - kwargs={'course_id': course_id, 'module_id': ''})) + module.get_html = replace_jump_to_id_urls( + module.get_html, + course_id, + reverse('jump_to_id', kwargs={'course_id': course_id, 'module_id': ''}) + ) if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'): if has_access(user, module, 'staff', course_id): diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index b4b1bcdfd1..fd5fe91672 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -66,8 +66,12 @@ class ModuleRenderTestCase(LoginEnrollmentTestCase): model_data_cache = ModelDataCache.cache_for_descriptor_descendents( self.course_id, self.mock_user, course, depth=2) - module = render.get_module(self.mock_user, mock_request, ['i4x', 'edX', 'toy', 'html', 'toyjumpto'], - model_data_cache, self.course_id) + module = render.get_module( + self.mock_user, + mock_request, + ['i4x', 'edX', 'toy', 'html', 'toyjumpto'], + model_data_cache, self.course_id + ) # get the rendered HTML output which should have the rewritten link html = module.get_html() diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 081d922114..875b62eefb 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -33,27 +33,27 @@ class TestJumpTo(TestCase): def test_jumpto_invalid_location(self): location = Location('i4x', 'edX', 'toy', 'NoSuchPlace', None) - jumpto_url = '%s/%s/jump_to/%s' % ('/courses', self.course_name, location) + jumpto_url = '{0}/{1}/jump_to/{2}'.format('/courses', self.course_name, location) response = self.client.get(jumpto_url) self.assertEqual(response.status_code, 404) def test_jumpto_from_chapter(self): location = Location('i4x', 'edX', 'toy', 'chapter', 'Overview') - jumpto_url = '%s/%s/jump_to/%s' % ('/courses', self.course_name, location) + jumpto_url = '{0}/{1}/jump_to/{2}'.format('/courses', self.course_name, location) expected = 'courses/edX/toy/2012_Fall/courseware/Overview/' response = self.client.get(jumpto_url) self.assertRedirects(response, expected, status_code=302, target_status_code=302) def test_jumpto_id(self): location = Location('i4x', 'edX', 'toy', 'chapter', 'Overview') - jumpto_url = '%s/%s/jump_to_id/%s' % ('/courses', self.course_name, location.name) + jumpto_url = '{0}/{1}/jump_to_id/{2}'.format('/courses', self.course_name, location.name) expected = 'courses/edX/toy/2012_Fall/courseware/Overview/' response = self.client.get(jumpto_url) self.assertRedirects(response, expected, status_code=302, target_status_code=302) def test_jumpto_id_invalid_location(self): location = Location('i4x', 'edX', 'toy', 'NoSuchPlace', None) - jumpto_url = '%s/%s/jump_to_id/%s' % ('/courses', self.course_name, location.name) + jumpto_url = '{0}/{1}/jump_to_id/{2)'.format('/courses', self.course_name, location.name) response = self.client.get(jumpto_url) self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 81e1ad2427..dc6c33bddf 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -459,10 +459,11 @@ def jump_to_id(request, course_id, module_id): items = modulestore().get_items(['i4x', course_location.org, course_location.course, None, module_id]) if len(items) == 0: - raise Http404("Could not find id = {0} in course_id = {1}".format(module_id, course_id)) + raise Http404("Could not find id = {0} in course_id = {1}. Referer = {2}". + format(module_id, course_id, request.META.get("HTTP_REFERER", ""))) if len(items) > 1: - logging.warning("Multiple items found with id = {0} in course_id = {1}. Using first found {2}...". - format(module_id, course_id, items[0].location.url())) + log.warning("Multiple items found with id = {0} in course_id = {1}. Referer = {2}. Using first found {3}...". + format(module_id, course_id, request.META.get("HTTP_REFERER", ""), items[0].location.url())) return jump_to(request, course_id, items[0].location.url())