From 808c70caf525587b088b7e7c67478eb4f6b6688e Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 09:39:13 -0400 Subject: [PATCH 1/5] Quick workaround courses hierarchy --- lms/djangoapps/courseware/views.py | 17 ++++++++++++++++- lms/urls.py | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 00ab45e605..1ea0a8407e 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1,5 +1,6 @@ import json import logging +import re import urllib import itertools @@ -8,7 +9,7 @@ from django.core.context_processors import csrf from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required -from django.http import Http404, HttpResponse +from django.http import Http404, HttpResponse, HttpResponseRedirect from django.shortcuts import redirect from mitxmako.shortcuts import render_to_response, render_to_string #from django.views.decorators.csrf import ensure_csrf_cookie @@ -35,6 +36,20 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} + +def course_redirect(request, course_path): + ''' + Allows course XML to refer to its own directory structure without knowing the multicourse hierarchy. + Prepends the multicourse path to the requested course-internal path. + If the course_id cannot be determined, redirect to multicourse page. (NOTE: is Http404 more appropriate?) + ''' + referer = request.META['HTTP_REFERER'] + match = re.search(r'/courses/(?P[^/]+/[^/]+/[^/]+)/', referer) + courses_path = '/courses/' + if match is not None: + courses_path += match.group('course_id') + '/' + course_path + return HttpResponseRedirect(courses_path) + def user_groups(user): """ TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately. diff --git a/lms/urls.py b/lms/urls.py index aaeba1b51e..af49aa2f3c 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -124,6 +124,7 @@ if settings.COURSEWARE_ENABLED: 'courseware.views.course_about', name="about_course"), #Inside the course + url(r'^course/(?P.*)$','courseware.views.course_redirect'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book$', From 0b4b8ecfdc91be3e2cee9eac801cee08b45af08b Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 14:20:35 -0400 Subject: [PATCH 2/5] courseware.get_module passes down course_id triplet (org, course, run) --- lms/djangoapps/courseware/module_render.py | 24 ++++++++++++++-------- lms/djangoapps/courseware/views.py | 23 +++++---------------- lms/urls.py | 1 - 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 29b73dda96..33f131e576 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -48,7 +48,7 @@ def make_track_function(request): return f -def toc_for_course(user, request, course, active_chapter, active_section): +def toc_for_course(user, request, course, active_chapter, active_section, course_id=None): ''' Create a table of contents from the module store @@ -71,7 +71,7 @@ def toc_for_course(user, request, course, active_chapter, active_section): ''' student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2) - course = get_module(user, request, course.location, student_module_cache) + course = get_module(user, request, course.location, student_module_cache, course_id=course_id) chapters = list() for chapter in course.get_display_items(): @@ -127,7 +127,7 @@ def get_section(course_module, chapter, section): return section_module -def get_module(user, request, location, student_module_cache, position=None): +def get_module(user, request, location, student_module_cache, position=None, course_id=None): ''' Get an instance of the xmodule class identified by location, setting the state based on an existing StudentModule, or creating one if none exists. @@ -144,6 +144,14 @@ def get_module(user, request, location, student_module_cache, position=None): ''' descriptor = modulestore().get_item(location) + + # NOTE: + # A 'course_id' is understood to be the triplet (org, course, run), for example + # (MITx, 6.002x, 2012_Spring). + # At the moment generic XModule does not contain enough information to replicate + # the triplet (it is missing 'run'), so we must pass down course_id + if course_id is None: + course_id = descriptor.location.course_id # Will NOT produce (org, course, run) for non-CourseModule's # Short circuit--if the user shouldn't have access, bail without doing any work if not has_access(user, descriptor, 'load'): @@ -167,7 +175,7 @@ def get_module(user, request, location, student_module_cache, position=None): # Setup system context for module instance ajax_url = reverse('modx_dispatch', - kwargs=dict(course_id=descriptor.location.course_id, + kwargs=dict(course_id=course_id, id=descriptor.location.url(), dispatch=''), ) @@ -175,7 +183,7 @@ def get_module(user, request, location, student_module_cache, position=None): # Fully qualified callback URL for external queueing system xqueue_callback_url = request.build_absolute_uri('/')[:-1] # Trailing slash provided by reverse xqueue_callback_url += reverse('xqueue_callback', - kwargs=dict(course_id=descriptor.location.course_id, + kwargs=dict(course_id=course_id, userid=str(user.id), id=descriptor.location.url(), dispatch='score_update'), @@ -195,7 +203,7 @@ def get_module(user, request, location, student_module_cache, position=None): Delegate to get_module. It does an access check, so may return None """ return get_module(user, request, location, - student_module_cache, position) + student_module_cache, position, course_id=course_id) # TODO (cpennington): When modules are shared between courses, the static # prefix is going to have to be specific to the module, not the directory @@ -309,7 +317,7 @@ def xqueue_callback(request, course_id, userid, id, dispatch): student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( user, modulestore().get_item(id), depth=0, acquire_lock=True) - instance = get_module(user, request, id, student_module_cache) + instance = get_module(user, request, id, student_module_cache, course_id=course_id) if instance is None: log.debug("No module {} for user {}--access denied?".format(id, user)) raise Http404 @@ -370,7 +378,7 @@ def modx_dispatch(request, dispatch=None, id=None, course_id=None): p[inputfile_id] = inputfile student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, modulestore().get_item(id)) - instance = get_module(request.user, request, id, student_module_cache) + instance = get_module(request.user, request, id, student_module_cache, course_id=course_id) 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/views.py b/lms/djangoapps/courseware/views.py index dd22327f24..cbd706e155 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -35,19 +35,6 @@ log = logging.getLogger("mitx.courseware") template_imports = {'urllib': urllib} -def course_redirect(request, course_path): - ''' - Allows course XML to refer to its own directory structure without knowing the multicourse hierarchy. - Prepends the multicourse path to the requested course-internal path. - If the course_id cannot be determined, redirect to multicourse page. (NOTE: is Http404 more appropriate?) - ''' - referer = request.META['HTTP_REFERER'] - match = re.search(r'/courses/(?P[^/]+/[^/]+/[^/]+)/', referer) - courses_path = '/courses/' - if match is not None: - courses_path += match.group('course_id') + '/' + course_path - return HttpResponseRedirect(courses_path) - def user_groups(user): """ TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately. @@ -82,7 +69,7 @@ def courses(request): return render_to_response("courses.html", {'universities': universities}) -def render_accordion(request, course, chapter, section): +def render_accordion(request, course, chapter, section, course_id=None): ''' Draws navigation bar. Takes current position in accordion as parameter. @@ -93,7 +80,7 @@ def render_accordion(request, course, chapter, section): Returns the html string''' # grab the table of contents - toc = toc_for_course(request.user, request, course, chapter, section) + toc = toc_for_course(request.user, request, course, chapter, section, course_id=course_id) context = dict([('toc', toc), ('course_id', course.id), @@ -134,7 +121,7 @@ def index(request, course_id, chapter=None, section=None, try: context = { 'csrf': csrf(request)['csrf_token'], - 'accordion': render_accordion(request, course, chapter, section), + 'accordion': render_accordion(request, course, chapter, section, course_id=course_id), 'COURSE_TITLE': course.title, 'course': course, 'init': '', @@ -150,7 +137,7 @@ def index(request, course_id, chapter=None, section=None, section_descriptor) module = get_module(request.user, request, section_descriptor.location, - student_module_cache) + student_module_cache, course_id=course_id) if module is None: # User is probably being clever and trying to access something # they don't have access to. @@ -285,7 +272,7 @@ def profile(request, course_id, student_id=None): user_info = UserProfile.objects.get(user=student) student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course) - course_module = get_module(request.user, request, course.location, student_module_cache) + course_module = get_module(request.user, request, course.location, student_module_cache, course_id=course_id) courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache) grade_summary = grades.grade(request.user, request, course, student_module_cache) diff --git a/lms/urls.py b/lms/urls.py index af49aa2f3c..aaeba1b51e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -124,7 +124,6 @@ if settings.COURSEWARE_ENABLED: 'courseware.views.course_about', name="about_course"), #Inside the course - url(r'^course/(?P.*)$','courseware.views.course_redirect'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/info$', 'courseware.views.course_info', name="info"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book$', From f1f80cb1f4bb2394ec8335d5e93094881b277b81 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 14:26:41 -0400 Subject: [PATCH 3/5] Drop HttpResponseRedirect import --- lms/djangoapps/courseware/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index cbd706e155..6da6720622 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -9,7 +9,7 @@ from django.core.context_processors import csrf from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required -from django.http import Http404, HttpResponse, HttpResponseRedirect +from django.http import Http404, HttpResponse from django.shortcuts import redirect from mitxmako.shortcuts import render_to_response, render_to_string #from django.views.decorators.csrf import ensure_csrf_cookie From 02d970c31e8def30d5b116e95f42f078ad9c1cd0 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 14:45:12 -0400 Subject: [PATCH 4/5] Map '/course/' urls to the course's root in the multicourse hierarchy --- common/djangoapps/xmodule_modifiers.py | 11 +++++++++++ lms/djangoapps/courseware/module_render.py | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 0aeaa59d69..4b3050e227 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -34,6 +34,17 @@ def wrap_xmodule(get_html, module, template): return _get_html +def replace_course_urls(get_html, course_id, module): + """ + Updates the supplied module with a new get_html function that wraps + the old get_html function and substitutes urls of the form /course/... + with urls that are /courses//... + """ + @wraps(get_html) + def _get_html(): + return replace_urls(get_html(), staticfiles_prefix='/courses/'+course_id, replace_prefix='/course/') + return _get_html + def replace_static_urls(get_html, prefix, module): """ Updates the supplied module with a new get_html function that wraps diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 33f131e576..b84bdb2310 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -19,7 +19,7 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.x_module import ModuleSystem -from xmodule_modifiers import replace_static_urls, add_histogram, wrap_xmodule +from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule log = logging.getLogger("mitx.courseware") @@ -233,6 +233,10 @@ def get_module(user, request, location, student_module_cache, position=None, cou module.metadata['data_dir'], module ) + # Allow URLs of the form '/course/' refer to the root of multicourse directory + # hierarchy of this course + module.get_html = replace_course_urls(module.get_html, course_id, module) + if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'): if has_access(user, module, 'staff'): module.get_html = add_histogram(module.get_html, module, user) From 58932bb2f7a37ee91939b7c482aa77c34696cc71 Mon Sep 17 00:00:00 2001 From: kimth Date: Wed, 15 Aug 2012 15:23:14 -0400 Subject: [PATCH 5/5] Drop import of re --- lms/djangoapps/courseware/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 20a1443f53..f5a93475b9 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1,6 +1,5 @@ import json import logging -import re import urllib import itertools