From 1c79b9c8747abc0513e0adc78c0b0876718e4a0f Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sat, 20 Jul 2013 21:02:24 -0400 Subject: [PATCH 01/10] add a /jump_to_id/ shortcut for producing more durable links between courseware in Studio --- cms/templates/unit.html | 2 +- common/djangoapps/static_replace/__init__.py | 21 ++++++++++++++++++- common/djangoapps/xmodule_modifiers.py | 13 ++++++++++++ .../test/data/toy/html/secret/toyjumpto.html | 1 + .../test/data/toy/html/secret/toyjumpto.xml | 1 + lms/djangoapps/courseware/module_render.py | 11 +++++++++- lms/djangoapps/courseware/tests/test_views.py | 13 ++++++++++++ lms/djangoapps/courseware/views.py | 21 +++++++++++++++++++ lms/urls.py | 2 ++ 9 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 common/test/data/toy/html/secret/toyjumpto.html create mode 100644 common/test/data/toy/html/secret/toyjumpto.xml diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 300d631421..22333fd85f 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -171,7 +171,7 @@

${_("Unit Location")}

-
+
${_("Unit Identifier:")} 
  1. ${section.display_name_with_default} diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index b73a658c5f..4a4a3fe576 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -43,6 +43,26 @@ def try_staticfiles_lookup(path): return url +def replace_jump_to_id_urls(text, course_id, jump_to_id_base_url): + """ + This will replace a link to another piece of courseware to a 'jump_to' + URL that will redirect to the right place in the courseware + + NOTE: This is similar to replace_course_urls in terms of functionality + but it is intended to be used when we only have a 'id' that the + course author provides. This is much more helpful when using + Studio authored courses since they don't need to know the path. This + is also durable with respect to item moves. + """ + + def replace_jump_to_id_url(match): + quote = match.group('quote') + rest = match.group('rest') + return "".join([quote, jump_to_id_base_url + rest, quote]) + + return re.sub(_url_replace_regex('/jump_to_id/'), replace_jump_to_id_url, text) + + def replace_course_urls(text, course_id): """ Replace /course/$stuff urls with /courses/$course_id/$stuff urls @@ -53,7 +73,6 @@ def replace_course_urls(text, course_id): returns: text with the links replaced """ - def replace_course_url(match): quote = match.group('quote') rest = match.group('rest') diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 3efc04789e..b0fa557c5d 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -42,6 +42,19 @@ def wrap_xmodule(get_html, module, template, context=None): return _get_html +def replace_jump_to_id_urls(get_html, course_id, jump_to_id_base_url): + """ + This will replace a link between courseware in the format + /jump_to/ with a URL for a page that will correctly redirect + This is similar to replace_course_urls, but much more flexible and + durable for Studio authored courses. See more comments in static_replace.replace_jump_to_urls + """ + @wraps(get_html) + def _get_html(): + return static_replace.replace_jump_to_id_urls(get_html(), course_id, jump_to_id_base_url) + return _get_html + + def replace_course_urls(get_html, course_id): """ Updates the supplied module with a new get_html function that wraps diff --git a/common/test/data/toy/html/secret/toyjumpto.html b/common/test/data/toy/html/secret/toyjumpto.html new file mode 100644 index 0000000000..779cd78d7c --- /dev/null +++ b/common/test/data/toy/html/secret/toyjumpto.html @@ -0,0 +1 @@ +This is a link to another page diff --git a/common/test/data/toy/html/secret/toyjumpto.xml b/common/test/data/toy/html/secret/toyjumpto.xml new file mode 100644 index 0000000000..af9cdf4f74 --- /dev/null +++ b/common/test/data/toy/html/secret/toyjumpto.xml @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index de709f7652..01b2451d3a 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -27,7 +27,7 @@ from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import ModuleSystem -from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule, save_module # pylint: disable=F0401 +from xmodule_modifiers import replace_course_urls, replace_jump_to_id_urls, replace_static_urls, add_histogram, wrap_xmodule, save_module # pylint: disable=F0401 import static_replace from psychometrics.psychoanalyze import make_psychometrics_data_update_handler @@ -393,6 +393,15 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours # hierarchy of this course module.get_html = replace_course_urls(module.get_html, course_id) + # this will rewrite intra-courseware links + # that use the shorthand /jump_to_id/. This is very helpful + # for studio authored courses (compared to the /course/... format) since it is + # is durable with respect to moves and the author doesn't need to + # know the hierarchy + 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): module.get_html = add_histogram(module.get_html, module, user) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 07be74c98e..c80e314cec 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -44,6 +44,19 @@ class TestJumpTo(TestCase): 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) + 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/%s' % ('/courses', self.course_name, location.name) + response = self.client.get(jumpto_url) + self.assertEqual(response.status_code, 404) + class ViewsTestCase(TestCase): def setUp(self): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index f1e1f7660c..81e1ad2427 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -33,6 +33,7 @@ from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem from xmodule.modulestore.search import path_to_location +from xmodule.course_module import CourseDescriptor import comment_client @@ -446,6 +447,26 @@ def index(request, course_id, chapter=None, section=None, return result +@ensure_csrf_cookie +def jump_to_id(request, course_id, module_id): + """ + This entry point allows for a shorter version of a jump to where just the id of the element is + passed in. This assumes that id is unique within the course_id namespace + """ + + course_location = CourseDescriptor.id_to_location(course_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)) + 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())) + + return jump_to(request, course_id, items[0].location.url()) + + @ensure_csrf_cookie def jump_to(request, course_id, location): """ diff --git a/lms/urls.py b/lms/urls.py index 6c32face81..9670a67a4a 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -177,6 +177,8 @@ if settings.COURSEWARE_ENABLED: urlpatterns += ( url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/jump_to/(?P.*)$', 'courseware.views.jump_to', name="jump_to"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/jump_to_id/(?P.*)$', + 'courseware.views.jump_to_id', name="jump_to_id"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch', name='modx_dispatch'), From 63c5cfda94fcf4e7b27cbddae0941135905501e2 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sat, 20 Jul 2013 21:51:11 -0400 Subject: [PATCH 02/10] add comment and fix mistake in test --- lms/djangoapps/courseware/module_render.py | 2 ++ lms/djangoapps/courseware/tests/test_views.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 01b2451d3a..6d8b244f27 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -398,6 +398,8 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours # for studio authored courses (compared to the /course/... format) since it is # is durable with respect to moves and the author doesn't need to # 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': ''})) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index c80e314cec..081d922114 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -53,7 +53,7 @@ class TestJumpTo(TestCase): def test_jumpto_id_invalid_location(self): location = Location('i4x', 'edX', 'toy', 'NoSuchPlace', None) - jumpto_url = '%s/%s/jump_to/%s' % ('/courses', self.course_name, location.name) + jumpto_url = '%s/%s/jump_to_id/%s' % ('/courses', self.course_name, location.name) response = self.client.get(jumpto_url) self.assertEqual(response.status_code, 404) From 059450c0de0013e87ea92ac599b9d996e15dbf7c Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Mon, 22 Jul 2013 09:28:10 -0400 Subject: [PATCH 03/10] Studio: revises HTML and styling around new unit ID on unit view --- cms/static/sass/views/_unit.scss | 19 +++++++++++++++++++ cms/templates/unit.html | 7 ++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/cms/static/sass/views/_unit.scss b/cms/static/sass/views/_unit.scss index 45e9823c2e..06685ad96b 100644 --- a/cms/static/sass/views/_unit.scss +++ b/cms/static/sass/views/_unit.scss @@ -747,6 +747,7 @@ body.unit { // Unit Page Sidebar .unit-settings { + .window-contents { padding: $baseline/2 $baseline; } @@ -854,6 +855,24 @@ body.unit { } .unit-location { + + // unit id + .wrapper-unit-id { + + .unit-id { + + .label { + @extend .t-title7; + margin-bottom: ($baseline/4); + color: $gray-d1; + } + + .value { + margin-bottom: 0; + } + } + } + .url { box-shadow: none; width: 100%; diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 22333fd85f..c31d5254e4 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -171,7 +171,12 @@

    ${_("Unit Location")}

    -
    ${_("Unit Identifier:")} 
    +
    +

    + ${_("Unit Identifier:")} + +

    +
    1. ${section.display_name_with_default} From 80e0b99342d68a3bae0ad5106e123f1da125656d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 22 Jul 2013 12:14:52 -0400 Subject: [PATCH 04/10] add a reference in the toy course to the test jump_to_id HTML module --- common/test/data/toy/course/2012_Fall.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index 8f0125ef2d..4bd311c328 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -3,6 +3,7 @@ +