From b05ead864fd20f715bed31d41a9b099b5a235899 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 31 Jan 2013 12:53:49 -0500 Subject: [PATCH 01/15] Revert "Revert "Merge pull request #1374 from MITx/feature/cale/no-course-collectstatic"" This reverts commit c4f56620dfb610d9a1f105478cfe4da6017102c3. --- cms/djangoapps/contentstore/views.py | 140 ++++++++++----------- common/lib/xmodule/xmodule/capa_module.py | 4 +- lms/djangoapps/courseware/courses.py | 5 +- lms/djangoapps/courseware/module_render.py | 10 +- lms/envs/common.py | 18 --- lms/envs/dev.py | 21 ++++ 6 files changed, 103 insertions(+), 95 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index f70164138d..fbff570803 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -132,7 +132,7 @@ def has_access(user, location, role=STAFF_ROLE_NAME): Return True if user allowed to access this piece of data Note that the CMS permissions model is with respect to courses There is a super-admin permissions if user.is_staff is set - Also, since we're unifying the user database between LMS and CAS, + Also, since we're unifying the user database between LMS and CAS, I'm presuming that the course instructor (formally known as admin) will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our queries here as INSTRUCTOR has all the rights that STAFF do @@ -154,7 +154,7 @@ def course_index(request, org, course, name): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -213,7 +213,7 @@ def edit_subsection(request, location): # remove all metadata from the generic dictionary that is presented in a more normalized UI - policy_metadata = dict((key,value) for key, value in item.metadata.iteritems() + policy_metadata = dict((key,value) for key, value in item.metadata.iteritems() if key not in ['display_name', 'start', 'due', 'format'] and key not in item.system_metadata_fields) can_view_live = False @@ -291,7 +291,7 @@ def edit_unit(request, location): containing_section = modulestore().get_item(containing_section_locs[0]) # cdodge hack. We're having trouble previewing drafts via jump_to redirect - # so let's generate the link url here + # so let's generate the link url here # need to figure out where this item is in the list of children as the preview will need this index =1 @@ -302,12 +302,12 @@ def edit_unit(request, location): preview_lms_link = '//{preview}{lms_base}/courses/{org}/{course}/{course_name}/courseware/{section}/{subsection}/{index}'.format( preview='preview.', - lms_base=settings.LMS_BASE, + lms_base=settings.LMS_BASE, org=course.location.org, - course=course.location.course, - course_name=course.location.name, - section=containing_section.location.name, - subsection=containing_subsection.location.name, + course=course.location.course, + course_name=course.location.name, + section=containing_section.location.name, + subsection=containing_subsection.location.name, index=index) unit_state = compute_unit_state(item) @@ -358,14 +358,14 @@ def assignment_type_update(request, org, course, category, name): location = Location(['i4x', org, course, category, name]) if not has_access(request.user, location): raise HttpResponseForbidden() - + if request.method == 'GET': - return HttpResponse(json.dumps(CourseGradingModel.get_section_grader_type(location)), + return HttpResponse(json.dumps(CourseGradingModel.get_section_grader_type(location)), mimetype="application/json") elif request.method == 'POST': # post or put, doesn't matter. - return HttpResponse(json.dumps(CourseGradingModel.update_section_grader_type(location, request.POST)), + return HttpResponse(json.dumps(CourseGradingModel.update_section_grader_type(location, request.POST)), mimetype="application/json") - + def user_author_string(user): '''Get an author string for commits by this user. Format: @@ -510,23 +510,23 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ error_msg=exc_info_to_str(sys.exc_info()) ).xmodule_constructor(system)(None, None) - # cdodge: Special case + # cdodge: Special case if module.location.category == 'static_tab': module.get_html = wrap_xmodule( module.get_html, module, "xmodule_tab_display.html", ) - else: + else: module.get_html = wrap_xmodule( module.get_html, module, "xmodule_display.html", ) - + module.get_html = replace_static_urls( module.get_html, - module.metadata.get('data_dir', module.location.course), + '/static/' + module.metadata.get('data_dir', module.location.course), course_namespace = Location([module.location.tag, module.location.org, module.location.course, None, None]) ) save_preview_state(request, preview_id, descriptor.location.url(), @@ -554,7 +554,7 @@ def _xmodule_recurse(item, action): _xmodule_recurse(child, action) action(item) - + @login_required @expect_json @@ -589,7 +589,7 @@ def delete_item(request): # delete_item on a vertical tries to delete the draft version leaving the # requested delete to never occur if item.location.revision is None and item.location.category=='vertical' and delete_all_versions: - modulestore('direct').delete_item(item.location) + modulestore('direct').delete_item(item.location) return HttpResponse() @@ -608,7 +608,7 @@ def save_item(request): if request.POST.get('data') is not None: data = request.POST['data'] store.update_item(item_location, data) - + # cdodge: note calling request.POST.get('children') will return None if children is an empty array # so it lead to a bug whereby the last component to be deleted in the UI was not actually # deleting the children object from the children collection @@ -698,7 +698,7 @@ def unpublish_unit(request): def clone_item(request): parent_location = Location(request.POST['parent_location']) template = Location(request.POST['template']) - + display_name = request.POST.get('display_name') if not has_access(request.user, parent_location): @@ -738,9 +738,9 @@ def upload_asset(request, org, course, coursename): location = ['i4x', org, course, 'course', coursename] if not has_access(request.user, location): return HttpResponseForbidden() - + # Does the course actually exist?!? Get anything from it to prove its existance - + try: item = modulestore().get_item(location) except: @@ -774,9 +774,9 @@ def upload_asset(request, org, course, coursename): # readback the saved content - we need the database timestamp readback = contentstore().find(content.location) - - response_payload = {'displayname' : content.name, - 'uploadDate' : get_date_display(readback.last_modified_at), + + response_payload = {'displayname' : content.name, + 'uploadDate' : get_date_display(readback.last_modified_at), 'url' : StaticContent.get_url_path_from_location(content.location), 'thumb_url' : StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None, 'msg' : 'Upload completed' @@ -792,7 +792,7 @@ This view will return all CMS users who are editors for the specified course @login_required @ensure_csrf_cookie def manage_users(request, location): - + # check that logged in user has permissions to this item if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME): raise PermissionDenied() @@ -808,7 +808,7 @@ def manage_users(request, location): 'allow_actions' : has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME), 'request_user_id' : request.user.id }) - + def create_json_response(errmsg = None): if errmsg is not None: @@ -830,13 +830,13 @@ def add_user(request, location): if email=='': return create_json_response('Please specify an email address.') - + # check that logged in user has admin permissions to this course if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME): raise PermissionDenied() - + user = get_user_by_email(email) - + # user doesn't exist?!? Return error. if user is None: return create_json_response('Could not find user by email address \'{0}\'.'.format(email)) @@ -859,7 +859,7 @@ the specified course @ensure_csrf_cookie def remove_user(request, location): email = request.POST["email"] - + # check that logged in user has admin permissions on this course if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME): raise PermissionDenied() @@ -886,7 +886,7 @@ def landing(request, org, course, coursename): def static_pages(request, org, course, coursename): location = ['i4x', org, course, 'course', coursename] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -951,7 +951,7 @@ def reorder_static_tabs(request): @login_required @ensure_csrf_cookie def edit_tabs(request, org, course, coursename): - location = ['i4x', org, course, 'course', coursename] + location = ['i4x', org, course, 'course', coursename] course_item = modulestore().get_item(location) static_tabs_loc = Location('i4x', org, course, 'static_tab', None) @@ -980,7 +980,7 @@ def edit_tabs(request, org, course, coursename): return render_to_response('edit-tabs.html', { 'active_tab': 'pages', - 'context_course':course_item, + 'context_course':course_item, 'components': components }) @@ -1001,13 +1001,13 @@ def course_info(request, org, course, name, provided_id=None): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() - + course_module = modulestore().get_item(location) - + # get current updates location = ['i4x', org, course, 'course_info', "updates"] @@ -1018,7 +1018,7 @@ def course_info(request, org, course, name, provided_id=None): 'course_updates' : json.dumps(get_course_updates(location)), 'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url() }) - + @expect_json @login_required @ensure_csrf_cookie @@ -1032,7 +1032,7 @@ def course_info_updates(request, org, course, provided_id=None): # ??? No way to check for access permission afaik # get current updates location = ['i4x', org, course, 'course_info', "updates"] - + # Hmmm, provided_id is coming as empty string on create whereas I believe it used to be None :-( # Possibly due to my removing the seemingly redundant pattern in urls.py if provided_id == '': @@ -1047,7 +1047,7 @@ def course_info_updates(request, org, course, provided_id=None): real_method = request.META['HTTP_X_HTTP_METHOD_OVERRIDE'] else: real_method = request.method - + if request.method == 'GET': return HttpResponse(json.dumps(get_course_updates(location)), mimetype="application/json") elif real_method == 'DELETE': # coming as POST need to pull from Request Header X-HTTP-Method-Override DELETE @@ -1064,7 +1064,7 @@ def course_info_updates(request, org, course, provided_id=None): @ensure_csrf_cookie def module_info(request, module_location): location = Location(module_location) - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -1077,10 +1077,10 @@ def module_info(request, module_location): rewrite_static_links = request.GET.get('rewrite_url_links','True') in ['True', 'true'] logging.debug('rewrite_static_links = {0} {1}'.format(request.GET.get('rewrite_url_links','False'), rewrite_static_links)) - + # check that logged in user has permissions to this item if not has_access(request.user, location): - raise PermissionDenied() + raise PermissionDenied() if real_method == 'GET': return HttpResponse(json.dumps(get_module_info(get_modulestore(location), location, rewrite_static_links=rewrite_static_links)), mimetype="application/json") @@ -1098,20 +1098,20 @@ def get_course_settings(request, org, course, name): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() - + course_module = modulestore().get_item(location) course_details = CourseDetails.fetch(location) - + return render_to_response('settings.html', { - 'active_tab': 'settings', + 'active_tab': 'settings', 'context_course': course_module, 'course_details' : json.dumps(course_details, cls=CourseSettingsEncoder) }) - + @expect_json @login_required @ensure_csrf_cookie @@ -1134,13 +1134,13 @@ def course_settings_updates(request, org, course, name, section): elif section == 'grading': manager = CourseGradingModel else: return - + if request.method == 'GET': # Cannot just do a get w/o knowing the course name :-( - return HttpResponse(json.dumps(manager.fetch(Location(['i4x', org, course, 'course',name])), cls=CourseSettingsEncoder), + return HttpResponse(json.dumps(manager.fetch(Location(['i4x', org, course, 'course',name])), cls=CourseSettingsEncoder), mimetype="application/json") elif request.method == 'POST': # post or put, doesn't matter. - return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder), + return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder), mimetype="application/json") @expect_json @@ -1153,7 +1153,7 @@ def course_grader_updates(request, org, course, name, grader_index=None): org, course: Attributes of the Location for the item to edit """ - + location = ['i4x', org, course, 'course', name] # check that logged in user has permissions to this item @@ -1164,13 +1164,13 @@ def course_grader_updates(request, org, course, name, grader_index=None): real_method = request.META['HTTP_X_HTTP_METHOD_OVERRIDE'] else: real_method = request.method - + if real_method == 'GET': # Cannot just do a get w/o knowing the course name :-( - return HttpResponse(json.dumps(CourseGradingModel.fetch_grader(Location(['i4x', org, course, 'course',name]), grader_index)), + return HttpResponse(json.dumps(CourseGradingModel.fetch_grader(Location(['i4x', org, course, 'course',name]), grader_index)), mimetype="application/json") elif real_method == "DELETE": - # ??? Shoudl this return anything? Perhaps success fail? + # ??? Shoudl this return anything? Perhaps success fail? CourseGradingModel.delete_grader(Location(['i4x', org, course, 'course',name]), grader_index) return HttpResponse() elif request.method == 'POST': # post or put, doesn't matter. @@ -1187,7 +1187,7 @@ def asset_index(request, org, course, name): org, course, name: Attributes of the Location for the item to edit """ location = ['i4x', org, course, 'course', name] - + # check that logged in user has permissions to this item if not has_access(request.user, location): raise PermissionDenied() @@ -1200,7 +1200,7 @@ def asset_index(request, org, course, name): }) course_module = modulestore().get_item(location) - + course_reference = StaticContent.compute_location(org, course, name) assets = contentstore().get_all_content_for_course(course_reference) @@ -1214,15 +1214,15 @@ def asset_index(request, org, course, name): display_info = {} display_info['displayname'] = asset['displayname'] display_info['uploadDate'] = get_date_display(asset['uploadDate']) - + asset_location = StaticContent.compute_location(id['org'], id['course'], id['name']) display_info['url'] = StaticContent.get_url_path_from_location(asset_location) - + # note, due to the schema change we may not have a 'thumbnail_location' in the result set _thumbnail_location = asset.get('thumbnail_location', None) thumbnail_location = Location(_thumbnail_location) if _thumbnail_location is not None else None display_info['thumb_url'] = StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_location is not None else None - + asset_display.append(display_info) return render_to_response('asset_index.html', { @@ -1241,9 +1241,9 @@ def edge(request): @expect_json def create_new_course(request): template = Location(request.POST['template']) - org = request.POST.get('org') - number = request.POST.get('number') - display_name = request.POST.get('display_name') + org = request.POST.get('org') + number = request.POST.get('number') + display_name = request.POST.get('display_name') try: dest_location = Location('i4x', org, number, 'course', Location.clean(display_name)) @@ -1289,13 +1289,13 @@ def initialize_course_tabs(course): # at least a list populated with the minimal times # @TODO: I don't like the fact that the presentation tier is away of these data related constraints, let's find a better # place for this. Also rather than using a simple list of dictionaries a nice class model would be helpful here - course.tabs = [{"type": "courseware"}, - {"type": "course_info", "name": "Course Info"}, + course.tabs = [{"type": "courseware"}, + {"type": "course_info", "name": "Course Info"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"}] - modulestore('direct').update_metadata(course.location.url(), course.own_metadata) + modulestore('direct').update_metadata(course.location.url(), course.own_metadata) @ensure_csrf_cookie @login_required @@ -1388,7 +1388,7 @@ def generate_export_course(request, org, course, name): root_dir = path(mkdtemp()) # export out to a tempdir - + logging.debug('root = {0}'.format(root_dir)) export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name) @@ -1400,7 +1400,7 @@ def generate_export_course(request, org, course, name): tf.close() # remove temp dir - shutil.rmtree(root_dir/name) + shutil.rmtree(root_dir/name) wrapper = FileWrapper(export_file) response = HttpResponse(wrapper, content_type='application/x-tgz') @@ -1430,4 +1430,4 @@ def event(request): A noop to swallow the analytics call so that cms methods don't spook and poor developers looking at console logs don't get distracted :-) ''' - return HttpResponse(True) \ No newline at end of file + return HttpResponse(True) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index f4b0c32e96..d3c8786f66 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -369,7 +369,7 @@ class CapaModule(XModule): id=self.location.html_id(), ajax_url=self.system.ajax_url) + html + "" # now do the substitutions which are filesystem based, e.g. '/static/' prefixes - return self.system.replace_urls(html, self.metadata['data_dir'], course_namespace=self.location) + return self.system.replace_urls(html) def handle_ajax(self, dispatch, get): ''' @@ -490,7 +490,7 @@ class CapaModule(XModule): new_answers = dict() for answer_id in answers: try: - new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.metadata['data_dir'], course_namespace=self.location)} + new_answer = {answer_id: self.system.replace_urls(answers[answer_id])} except TypeError: log.debug('Unable to perform URL substitution on answers[%s]: %s' % (answer_id, answers[answer_id])) new_answer = {answer_id: answers[answer_id]} diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 89a1496eca..03d5a89c64 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -83,13 +83,12 @@ def get_opt_course_with_access(user, course_id, action): return None return get_course_with_access(user, course_id, action) - + def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" if isinstance(modulestore(), XMLModuleStore): - path = course.metadata['data_dir'] + "/images/course_image.jpg" - return try_staticfiles_lookup(path) + return '/static/' + course.metadata['data_dir'] + "/images/course_image.jpg" else: loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') path = StaticContent.get_url_path_from_location(loc) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 7ed32c8597..22d95ef8a2 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -3,6 +3,8 @@ import logging import pyparsing import sys +from functools import partial + from django.conf import settings from django.contrib.auth.models import User from django.core.urlresolvers import reverse @@ -244,7 +246,11 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, # TODO (cpennington): This should be removed when all html from # a module is coming through get_html and is therefore covered # by the replace_static_urls code below - replace_urls=replace_urls, + replace_urls=partial( + replace_urls, + staticfiles_prefix='/static/' + descriptor.metadata.get('data_dir', ''), + course_namespace=descriptor.location._replace(category=None, name=None), + ), node_path=settings.NODE_PATH, anonymous_student_id=unique_id_for_user(user), course_id=course_id, @@ -280,7 +286,7 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, module.get_html = replace_static_urls( _get_html, - module.metadata['data_dir'] if 'data_dir' in module.metadata else '', + '/static/' + module.metadata.get('data_dir', ''), course_namespace = module.location._replace(category=None, name=None)) # Allow URLs of the form '/course/' refer to the root of multicourse directory diff --git a/lms/envs/common.py b/lms/envs/common.py index 4b325821dd..24c757f51b 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -266,24 +266,6 @@ STATICFILES_DIRS = [ COMMON_ROOT / "static", PROJECT_ROOT / "static", ] -if os.path.isdir(DATA_DIR): - # Add the full course repo if there is no static directory - STATICFILES_DIRS += [ - # TODO (cpennington): When courses are stored in a database, this - # should no longer be added to STATICFILES - (course_dir, DATA_DIR / course_dir) - for course_dir in os.listdir(DATA_DIR) - if (os.path.isdir(DATA_DIR / course_dir) and - not os.path.isdir(DATA_DIR / course_dir / 'static')) - ] - # Otherwise, add only the static directory from the course dir - STATICFILES_DIRS += [ - # TODO (cpennington): When courses are stored in a database, this - # should no longer be added to STATICFILES - (course_dir, DATA_DIR / course_dir / 'static') - for course_dir in os.listdir(DATA_DIR) - if (os.path.isdir(DATA_DIR / course_dir / 'static')) - ] # Locale/Internationalization TIME_ZONE = 'America/New_York' # http://en.wikipedia.org/wiki/List_of_tz_zones_by_name diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 99ee9662ee..338a31f641 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -106,6 +106,27 @@ VIRTUAL_UNIVERSITIES = [] COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" +############################## Course static files ########################## +if os.path.isdir(DATA_DIR): + # Add the full course repo if there is no static directory + STATICFILES_DIRS += [ + # TODO (cpennington): When courses are stored in a database, this + # should no longer be added to STATICFILES + (course_dir, DATA_DIR / course_dir) + for course_dir in os.listdir(DATA_DIR) + if (os.path.isdir(DATA_DIR / course_dir) and + not os.path.isdir(DATA_DIR / course_dir / 'static')) + ] + # Otherwise, add only the static directory from the course dir + STATICFILES_DIRS += [ + # TODO (cpennington): When courses are stored in a database, this + # should no longer be added to STATICFILES + (course_dir, DATA_DIR / course_dir / 'static') + for course_dir in os.listdir(DATA_DIR) + if (os.path.isdir(DATA_DIR / course_dir / 'static')) + ] + + ################################# mitx revision string ##################### MITX_VERSION_STRING = os.popen('cd %s; git describe' % REPO_ROOT).read().strip() From 04879a83d3057cca5e128841809c9aee4af83e68 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 1 Feb 2013 22:15:31 -0500 Subject: [PATCH 02/15] Rejigger how /course and /static urls are replaced, to make the logic slightly more comprehensible --- .../contentstore/module_info_model.py | 16 ++- cms/djangoapps/contentstore/views.py | 12 +- common/djangoapps/static_replace.py | 109 +++++++++++------- common/djangoapps/xmodule_modifiers.py | 8 +- common/lib/xmodule/xmodule/video_module.py | 8 +- lms/djangoapps/courseware/courses.py | 9 +- lms/djangoapps/courseware/module_render.py | 6 +- lms/djangoapps/courseware/tabs.py | 3 +- 8 files changed, 106 insertions(+), 65 deletions(-) diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 0017010885..3b783c8815 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -1,5 +1,5 @@ import logging -from static_replace import replace_urls +from static_replace import replace_static_urls from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore @@ -18,7 +18,17 @@ def get_module_info(store, location, parent_location = None, rewrite_static_link data = module.definition['data'] if rewrite_static_links: - data = replace_urls(module.definition['data'], course_namespace = Location([module.location.tag, module.location.org, module.location.course, None, None])) + data = replace_static_urls( + module.definition['data'], + None, + course_namespace=Location([ + module.location.tag, + module.location.org, + module.location.course, + None, + None + ]) + ) return { 'id': module.location.url(), @@ -47,7 +57,7 @@ def set_module_info(store, location, post_data): if post_data.get('data') is not None: data = post_data['data'] store.update_item(location, data) - + # cdodge: note calling request.POST.get('children') will return None if children is an empty array # so it lead to a bug whereby the last component to be deleted in the UI was not actually # deleting the children object from the children collection diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index fbff570803..14065f2d54 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -31,7 +31,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationErr from xmodule.x_module import ModuleSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str -from static_replace import replace_urls +from static_replace import replace_static_urls from external_auth.views import ssl_login_shortcut from mitxmako.shortcuts import render_to_response, render_to_string @@ -473,7 +473,7 @@ def preview_module_system(request, preview_id, descriptor): get_module=partial(get_preview_module, request, preview_id), render_template=render_from_lms, debug=True, - replace_urls=replace_urls, + replace_urls=partial(replace_static_urls, data_directory=None, course_namespace=descriptor.location), user=request.user, ) @@ -915,7 +915,7 @@ def reorder_static_tabs(request): # get list of existing static tabs in course # make sure they are the same lengths (i.e. the number of passed in tabs equals the number # that we know about) otherwise we can drop some! - + existing_static_tabs = [t for t in course.tabs if t['type'] == 'static_tab'] if len(existing_static_tabs) != len(tabs): return HttpResponseBadRequest() @@ -934,15 +934,15 @@ def reorder_static_tabs(request): static_tab_idx = 0 for tab in course.tabs: if tab['type'] == 'static_tab': - reordered_tabs.append({'type': 'static_tab', - 'name' : tab_items[static_tab_idx].metadata.get('display_name'), + reordered_tabs.append({'type': 'static_tab', + 'name' : tab_items[static_tab_idx].metadata.get('display_name'), 'url_slug' : tab_items[static_tab_idx].location.name}) static_tab_idx += 1 else: reordered_tabs.append(tab) - # OK, re-assemble the static tabs in the new order + # OK, re-assemble the static tabs in the new order course.tabs = reordered_tabs modulestore('direct').update_metadata(course.location, course.metadata) return HttpResponse() diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py index e75362d784..12db870f1a 100644 --- a/common/djangoapps/static_replace.py +++ b/common/djangoapps/static_replace.py @@ -11,6 +11,16 @@ from xmodule.contentstore.content import StaticContent log = logging.getLogger(__name__) + +def _url_replace_regex(prefix): + return r""" + (?x) # flags=re.VERBOSE + (?P\\?['"]) # the opening quotes + (?P{prefix}) # theeprefix + (?P.*?) # everything else in the url + (?P=quote) # the first matching closing quote + """.format(prefix=prefix) + def try_staticfiles_lookup(path): """ Try to lookup a path in staticfiles_storage. If it fails, return @@ -26,48 +36,67 @@ def try_staticfiles_lookup(path): return url -def replace(static_url, prefix=None, course_namespace=None): - if prefix is None: - prefix = '' - else: - prefix = prefix + '/' +def replace_course_urls(text, course_id): + """ + Replace /course/$stuff urls with /courses/$course_id/$stuff urls - quote = static_url.group('quote') + text: The text to replace + course_module: A CourseDescriptor - servable = ( - # If in debug mode, we'll serve up anything that the finders can find - (settings.DEBUG and finders.find(static_url.group('rest'), True)) or - # Otherwise, we'll only serve up stuff that the storages can find - staticfiles_storage.exists(static_url.group('rest')) - ) + returns: text with the links replaced + """ - if servable: - return static_url.group(0) - else: - # don't error if file can't be found - # cdodge: to support the change over to Mongo backed content stores, lets - # use the utility functions in StaticContent.py - if static_url.group('prefix') == '/static/' and not isinstance(modulestore(), XMLModuleStore): - if course_namespace is None: - raise BaseException('You must pass in course_namespace when remapping static content urls with MongoDB stores') - url = StaticContent.convert_legacy_static_url(static_url.group('rest'), course_namespace) + + def replace_course_url(match): + log.warning("Course match: %s", match.groupdict()) + quote = match.group('quote') + rest = match.group('rest') + return "".join([quote, '/courses/' + course_id + '/', rest, quote]) + + return re.sub(_url_replace_regex('/courses/'), replace_course_url, text) + + +def replace_static_urls(text, data_directory, course_namespace=None): + """ + Replace /static/$stuff urls either with their correct url as generated by collectstatic, + (/static/$md5_hashed_stuff) or by the course-specific content static url + /static/$course_data_dir/$stuff, or, if course_namespace is not None, by the + correct url in the contentstore (c4x://) + + text: The source text to do the substitution in + data_directory: The directory in which course data is stored + course_namespace: The course identifier used to distinguish static content for this course in studio + """ + + def replace_static_url(match): + log.warning(match.groupdict()) + quote = match.group('quote') + rest = match.group('rest') + + # course_namespace is not None, then use studio style urls + if course_namespace is not None and not isinstance(modulestore(), XMLModuleStore): + url = StaticContent.convert_legacy_static_url(rest, course_namespace) + log.warning("From modulestore: %s", url) + # If we're in debug mode, and the file as requested exists, then don't change the links + elif (settings.DEBUG and finders.find(rest, True)): + url = match.group('prefix') + rest + log.warning("From finder: %s", url) + # Otherwise, look the file up in staticfiles_storage else: - url = try_staticfiles_lookup(prefix + static_url.group('rest')) + try: + url = staticfiles_storage.url(data_directory + '/' + rest) + log.warning("From staticfiles_storage: %s", url) + # And if that fails, return the path unmodified + except Exception as err: + log.warning("staticfiles_storage couldn't find path {0}: {1}".format( + path, str(err))) + url = path + log.warning("Fallback: %s", url) + log.warning("".join([quote, url, quote])) + return "".join([quote, url, quote]) - new_link = "".join([quote, url, quote]) - return new_link - - - -def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', course_namespace=None): - - def replace_url(static_url): - return replace(static_url, staticfiles_prefix, course_namespace = course_namespace) - - return re.sub(r""" - (?x) # flags=re.VERBOSE - (?P\\?['"]) # the opening quotes - (?P{prefix}) # the prefix - (?P.*?) # everything else in the url - (?P=quote) # the first matching closing quote - """.format(prefix=replace_prefix), replace_url, text) + return re.sub( + _url_replace_regex('/static/(?!{data_dir}'.format(data_dir=data_directory)), + replace_static_url, + text + ) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 5c19a2f1d7..e0fda01eef 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -2,10 +2,10 @@ import re import json import logging import time +import static_replace from django.conf import settings from functools import wraps -from static_replace import replace_urls from mitxmako.shortcuts import render_to_string from xmodule.seq_module import SequenceModule from xmodule.vertical_module import VerticalModule @@ -49,10 +49,10 @@ def replace_course_urls(get_html, course_id): """ @wraps(get_html) def _get_html(): - return replace_urls(get_html(), staticfiles_prefix='/courses/'+course_id, replace_prefix='/course/') + return static_replace.replace_course_urls(get_html(), course_id) return _get_html -def replace_static_urls(get_html, prefix, course_namespace=None): +def replace_static_urls(get_html, data_dir, course_namespace=None): """ Updates the supplied module with a new get_html function that wraps the old get_html function and substitutes urls of the form /static/... @@ -61,7 +61,7 @@ def replace_static_urls(get_html, prefix, course_namespace=None): @wraps(get_html) def _get_html(): - return replace_urls(get_html(), staticfiles_prefix=prefix, course_namespace = course_namespace) + return static_replace.replace_static_urls(get_html(), data_dir, course_namespace) return _get_html diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index bb3af745ae..f21cd37a37 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -6,7 +6,7 @@ from pkg_resources import resource_string, resource_listdir from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor -from xmodule.modulestore.mongo import MongoModuleStore +from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent @@ -121,12 +121,12 @@ class VideoModule(XModule): return self.youtube def get_html(self): - if isinstance(modulestore(), MongoModuleStore) : - caption_asset_path = StaticContent.get_base_url_path_for_course_assets(self.location) + '/subs_' - else: + if isinstance(modulestore(), XMLModuleStore) : # VS[compat] # cdodge: filesystem static content support. caption_asset_path = "/static/{0}/subs/".format(self.metadata['data_dir']) + else: + caption_asset_path = StaticContent.get_base_url_path_for_course_assets(self.location) + '/subs_' return self.system.render_template('video.html', { 'streams': self.video_list(), diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 03d5a89c64..ce29d69784 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -19,7 +19,7 @@ from xmodule.contentstore.content import StaticContent from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import XModule -from static_replace import replace_urls, try_staticfiles_lookup +from static_replace import replace_static_urls from courseware.access import has_access import branding from courseware.models import StudentModuleCache @@ -223,8 +223,11 @@ def get_course_syllabus_section(course, section_key): dirs = [path("syllabus") / course.url_name, path("syllabus")] filepath = find_file(fs, dirs, section_key + ".html") with fs.open(filepath) as htmlFile: - return replace_urls(htmlFile.read().decode('utf-8'), - course.metadata['data_dir'], course_namespace=course.location) + return replace_static_urls( + htmlFile.read().decode('utf-8'), + course.metadata['data_dir'], + course_namespace=course.location + ) except ResourceNotFoundError: log.exception("Missing syllabus section {key} in course {url}".format( key=section_key, url=course.location.url())) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 22d95ef8a2..8a791785cd 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -2,6 +2,7 @@ import json import logging import pyparsing import sys +import static_replace from functools import partial @@ -20,7 +21,6 @@ from courseware.access import has_access from mitxmako.shortcuts import render_to_string from models import StudentModule, StudentModuleCache from psychometrics.psychoanalyze import make_psychometrics_data_update_handler -from static_replace import replace_urls from student.models import unique_id_for_user from xmodule.errortracker import exc_info_to_str from xmodule.exceptions import NotFoundError @@ -247,8 +247,8 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, # a module is coming through get_html and is therefore covered # by the replace_static_urls code below replace_urls=partial( - replace_urls, - staticfiles_prefix='/static/' + descriptor.metadata.get('data_dir', ''), + static_replace.replace_static_urls, + data_directory=descriptor.metadata.get('data_dir', ''), course_namespace=descriptor.location._replace(category=None, name=None), ), node_path=settings.NODE_PATH, diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 0a7c723cb5..64d488d845 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -24,7 +24,6 @@ from static_replace import replace_urls from lxml.html import rewrite_links from module_render import get_module from courseware.access import has_access -from static_replace import replace_urls from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.xml import XMLModuleStore @@ -322,4 +321,4 @@ def get_static_tab_contents(request, cache, course, tab): if tab_module is not None: html = tab_module.get_html() - return html \ No newline at end of file + return html From 88fc2e3756073740441c514d112f9570b0adb251 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 11:17:25 -0500 Subject: [PATCH 03/15] Fix unbalanced paren in regex --- common/djangoapps/static_replace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py index 12db870f1a..0301f02ddf 100644 --- a/common/djangoapps/static_replace.py +++ b/common/djangoapps/static_replace.py @@ -96,7 +96,7 @@ def replace_static_urls(text, data_directory, course_namespace=None): return "".join([quote, url, quote]) return re.sub( - _url_replace_regex('/static/(?!{data_dir}'.format(data_dir=data_directory)), + _url_replace_regex('/static/(?!{data_dir})'.format(data_dir=data_directory)), replace_static_url, text ) From 0da1467a904bad9b97b6a876355041ccc0a32292 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 11:20:43 -0500 Subject: [PATCH 04/15] Pull out missing variable --- common/djangoapps/static_replace.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py index 0301f02ddf..c99d55794e 100644 --- a/common/djangoapps/static_replace.py +++ b/common/djangoapps/static_replace.py @@ -83,8 +83,9 @@ def replace_static_urls(text, data_directory, course_namespace=None): log.warning("From finder: %s", url) # Otherwise, look the file up in staticfiles_storage else: + path = data_directory + '/' + rest try: - url = staticfiles_storage.url(data_directory + '/' + rest) + url = staticfiles_storage.url(path) log.warning("From staticfiles_storage: %s", url) # And if that fails, return the path unmodified except Exception as err: From ba7bd9022b5d0c82a2baac673b4c5769bff6cdf9 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 11:24:11 -0500 Subject: [PATCH 05/15] Replace the correct /course/ urls, rather than /courses/ --- common/djangoapps/static_replace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py index c99d55794e..2932c0357a 100644 --- a/common/djangoapps/static_replace.py +++ b/common/djangoapps/static_replace.py @@ -53,7 +53,7 @@ def replace_course_urls(text, course_id): rest = match.group('rest') return "".join([quote, '/courses/' + course_id + '/', rest, quote]) - return re.sub(_url_replace_regex('/courses/'), replace_course_url, text) + return re.sub(_url_replace_regex('/course/'), replace_course_url, text) def replace_static_urls(text, data_directory, course_namespace=None): From 401f564e6a0a6ac33a9f05e6dffc6f9c9b7e3800 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 12:29:21 -0500 Subject: [PATCH 06/15] Fix data directory passed to replace_static_urls --- cms/djangoapps/contentstore/views.py | 2 +- lms/djangoapps/courseware/module_render.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 14065f2d54..9328b7fdb1 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -526,7 +526,7 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ module.get_html = replace_static_urls( module.get_html, - '/static/' + module.metadata.get('data_dir', module.location.course), + module.metadata.get('data_dir', module.location.course), course_namespace = Location([module.location.tag, module.location.org, module.location.course, None, None]) ) save_preview_state(request, preview_id, descriptor.location.url(), diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 8a791785cd..b19796b357 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -286,7 +286,7 @@ def _get_module(user, request, descriptor, student_module_cache, course_id, module.get_html = replace_static_urls( _get_html, - '/static/' + module.metadata.get('data_dir', ''), + module.metadata.get('data_dir', ''), course_namespace = module.location._replace(category=None, name=None)) # Allow URLs of the form '/course/' refer to the root of multicourse directory From f1f2bd8fd25b75fbc3dd62f7f0fa2aaccbc2eaba Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 12:29:35 -0500 Subject: [PATCH 07/15] Start adding tests for static replacement --- .../__init__.py} | 0 .../test/test_static_replace.py | 20 +++++++++++++++++++ 2 files changed, 20 insertions(+) rename common/djangoapps/{static_replace.py => static_replace/__init__.py} (100%) create mode 100644 common/djangoapps/static_replace/test/test_static_replace.py diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace/__init__.py similarity index 100% rename from common/djangoapps/static_replace.py rename to common/djangoapps/static_replace/__init__.py diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py new file mode 100644 index 0000000000..c451f11ec6 --- /dev/null +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -0,0 +1,20 @@ +from nose.tools import assert_equals +from static_replace import replace_static_urls, replace_course_urls + +DATA_DIRECTORY = 'data_dir' +COURSE_ID = 'org/course/run' + + +def test_multi_replace(): + static_source = '"/static/file.png"' + course_source = '"/course/file.png"' + + assert_equals( + replace_static_urls(static_source, DATA_DIRECTORY), + replace_static_urls(replace_static_urls(static_source, DATA_DIRECTORY), DATA_DIRECTORY) + ) + assert_equals( + replace_course_urls(course_source, COURSE_ID), + replace_course_urls(replace_course_urls(course_source, COURSE_ID), COURSE_ID) + ) + assert False From 5eae9c3a40ef147b56f545ef2b47d3078a47e73b Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 13:12:44 -0500 Subject: [PATCH 08/15] Make lms and cms coverage reports cover common/djangoapps --- cms/.coveragerc | 2 +- lms/.coveragerc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/.coveragerc b/cms/.coveragerc index 9b1e59d670..b7ae181e99 100644 --- a/cms/.coveragerc +++ b/cms/.coveragerc @@ -1,7 +1,7 @@ # .coveragerc for cms [run] data_file = reports/cms/.coverage -source = cms +source = cms,common/djangoapps omit = cms/envs/*, cms/manage.py [report] diff --git a/lms/.coveragerc b/lms/.coveragerc index 7e18a37492..35aa7a3851 100644 --- a/lms/.coveragerc +++ b/lms/.coveragerc @@ -1,7 +1,7 @@ # .coveragerc for lms [run] data_file = reports/lms/.coverage -source = lms +source = lms,common/djangoapps omit = lms/envs/* [report] From 8592db92bfec46670336a27394b16b87ce843f74 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 13:19:23 -0500 Subject: [PATCH 09/15] Add more tests of static_replace --- common/djangoapps/static_replace/__init__.py | 12 ++---- .../test/test_static_replace.py | 38 ++++++++++++++++++- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 2932c0357a..7167456ed1 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -48,7 +48,6 @@ def replace_course_urls(text, course_id): def replace_course_url(match): - log.warning("Course match: %s", match.groupdict()) quote = match.group('quote') rest = match.group('rest') return "".join([quote, '/courses/' + course_id + '/', rest, quote]) @@ -69,31 +68,26 @@ def replace_static_urls(text, data_directory, course_namespace=None): """ def replace_static_url(match): - log.warning(match.groupdict()) + original = match.group(0) quote = match.group('quote') rest = match.group('rest') # course_namespace is not None, then use studio style urls if course_namespace is not None and not isinstance(modulestore(), XMLModuleStore): url = StaticContent.convert_legacy_static_url(rest, course_namespace) - log.warning("From modulestore: %s", url) # If we're in debug mode, and the file as requested exists, then don't change the links elif (settings.DEBUG and finders.find(rest, True)): - url = match.group('prefix') + rest - log.warning("From finder: %s", url) + return original # Otherwise, look the file up in staticfiles_storage else: path = data_directory + '/' + rest try: url = staticfiles_storage.url(path) - log.warning("From staticfiles_storage: %s", url) # And if that fails, return the path unmodified except Exception as err: log.warning("staticfiles_storage couldn't find path {0}: {1}".format( path, str(err))) - url = path - log.warning("Fallback: %s", url) - log.warning("".join([quote, url, quote])) + return original return "".join([quote, url, quote]) return re.sub( diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index c451f11ec6..cfc025b964 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -1,8 +1,12 @@ from nose.tools import assert_equals from static_replace import replace_static_urls, replace_course_urls +from mock import patch, Mock +from xmodule.modulestore import Location +from xmodule.modulestore.mongo import MongoModuleStore DATA_DIRECTORY = 'data_dir' COURSE_ID = 'org/course/run' +NAMESPACE = Location('org', 'course', 'run', None, None) def test_multi_replace(): @@ -17,4 +21,36 @@ def test_multi_replace(): replace_course_urls(course_source, COURSE_ID), replace_course_urls(replace_course_urls(course_source, COURSE_ID), COURSE_ID) ) - assert False + + +@patch('static_replace.finders') +@patch('static_replace.settings') +def test_debug_no_modify(mock_settings, mock_finders): + mock_settings.DEBUG = True + mock_finders.find.return_value = True + + static_source = '"/static/file.png"' + assert_equals(static_source, replace_static_urls(static_source, DATA_DIRECTORY)) + + mock_finders.find.assert_called_once_with('file.png', True) + + +@patch('static_replace.StaticContent') +@patch('static_replace.modulestore') +def test_mongo_filestore(mock_modulestore, mock_static_content): + + mock_modulestore.return_value = Mock(MongoModuleStore) + mock_static_content.convert_legacy_static_url.return_value = "c4x://mock_url" + + static_source = '"/static/file.png"' + + # No namespace => no change to path + assert_equals(static_source, replace_static_urls(static_source, DATA_DIRECTORY)) + + # Namespace => content url + assert_equals( + '"' + mock_static_content.convert_legacy_static_url.return_value + '"', + replace_static_urls(static_source, DATA_DIRECTORY, NAMESPACE) + ) + + mock_static_content.convert_legacy_static_url.assert_called_once_with('file.png', NAMESPACE) From ae5d3cbc578afce9e27b1094053c4cd3f16c6713 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 13:46:17 -0500 Subject: [PATCH 10/15] Add admin command to flush the staticfiles cache --- .../static_replace/management/__init__.py | 0 .../static_replace/management/commands/__init__.py | 0 .../commands/clear_collectstatic_cache.py | 13 +++++++++++++ 3 files changed, 13 insertions(+) create mode 100644 common/djangoapps/static_replace/management/__init__.py create mode 100644 common/djangoapps/static_replace/management/commands/__init__.py create mode 100644 common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py diff --git a/common/djangoapps/static_replace/management/__init__.py b/common/djangoapps/static_replace/management/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/static_replace/management/commands/__init__.py b/common/djangoapps/static_replace/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py b/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py new file mode 100644 index 0000000000..1cea81b0af --- /dev/null +++ b/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py @@ -0,0 +1,13 @@ +### +### Script for importing courseware from XML format +### + +from django.core.management.base import NoArgsCommand + +class Command(NoArgsCommand): + help = \ +'''Import the specified data directory into the default ModuleStore''' + + def handle_noargs(self): + staticfiles_cache = get_cache('staticfiles') + staticfiles_cache.clear() From 2551243dfa1a2a20ad5641e42e44004effecaf4b Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 13:50:12 -0500 Subject: [PATCH 11/15] Install the static_replace app for the management command --- cms/envs/common.py | 1 + lms/envs/common.py | 1 + 2 files changed, 2 insertions(+) diff --git a/cms/envs/common.py b/cms/envs/common.py index f2d47dfdc6..3ea532d70d 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -285,4 +285,5 @@ INSTALLED_APPS = ( # For asset pipelining 'pipeline', 'staticfiles', + 'static_replace', ) diff --git a/lms/envs/common.py b/lms/envs/common.py index 24c757f51b..bf85d1692a 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -548,6 +548,7 @@ INSTALLED_APPS = ( # For asset pipelining 'pipeline', 'staticfiles', + 'static_replace', # Our courseware 'circuit', From 10f8dbfa466b8bdbf579f1a42dcc2c8d936f68c0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 13:52:52 -0500 Subject: [PATCH 12/15] Handle options passed to clear_collectstatic_cache --- .../management/commands/clear_collectstatic_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py b/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py index 1cea81b0af..9e8e401991 100644 --- a/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py +++ b/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py @@ -8,6 +8,6 @@ class Command(NoArgsCommand): help = \ '''Import the specified data directory into the default ModuleStore''' - def handle_noargs(self): + def handle_noargs(self, **options): staticfiles_cache = get_cache('staticfiles') staticfiles_cache.clear() From 90107fa88fd40f420a985a74355c646ec1b5bb75 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 13:56:50 -0500 Subject: [PATCH 13/15] Import cache function correctly --- .../management/commands/clear_collectstatic_cache.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py b/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py index 9e8e401991..60b7c58047 100644 --- a/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py +++ b/common/djangoapps/static_replace/management/commands/clear_collectstatic_cache.py @@ -3,6 +3,8 @@ ### from django.core.management.base import NoArgsCommand +from django.core.cache import get_cache + class Command(NoArgsCommand): help = \ From 0350d15e29dc621374839b8869af7695bcd9b398 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 14:20:21 -0500 Subject: [PATCH 14/15] Make static_replace fall back to data_directory prefix, if it doesn't find the file anywhere else --- common/djangoapps/static_replace/__init__.py | 13 +++++----- .../test/test_static_replace.py | 26 ++++++++++++------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 7167456ed1..cfef798bdf 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -69,6 +69,7 @@ def replace_static_urls(text, data_directory, course_namespace=None): def replace_static_url(match): original = match.group(0) + prefix = match.group('prefix') quote = match.group('quote') rest = match.group('rest') @@ -78,16 +79,16 @@ def replace_static_urls(text, data_directory, course_namespace=None): # If we're in debug mode, and the file as requested exists, then don't change the links elif (settings.DEBUG and finders.find(rest, True)): return original - # Otherwise, look the file up in staticfiles_storage + # Otherwise, look the file up in staticfiles_storage without the data directory else: - path = data_directory + '/' + rest try: - url = staticfiles_storage.url(path) - # And if that fails, return the path unmodified + url = staticfiles_storage.url(rest) + # And if that fails, assume that it's course content, and add manually data directory except Exception as err: log.warning("staticfiles_storage couldn't find path {0}: {1}".format( - path, str(err))) - return original + rest, str(err))) + url = "".join([prefix, data_directory, '/', rest]) + return "".join([quote, url, quote]) return re.sub( diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index cfc025b964..e08c66c59f 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -3,19 +3,20 @@ from static_replace import replace_static_urls, replace_course_urls from mock import patch, Mock from xmodule.modulestore import Location from xmodule.modulestore.mongo import MongoModuleStore +from xmodule.modulestore.xml import XMLModuleStore DATA_DIRECTORY = 'data_dir' COURSE_ID = 'org/course/run' NAMESPACE = Location('org', 'course', 'run', None, None) +STATIC_SOURCE = '"/static/file.png"' def test_multi_replace(): - static_source = '"/static/file.png"' course_source = '"/course/file.png"' assert_equals( - replace_static_urls(static_source, DATA_DIRECTORY), - replace_static_urls(replace_static_urls(static_source, DATA_DIRECTORY), DATA_DIRECTORY) + replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY), + replace_static_urls(replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY), DATA_DIRECTORY) ) assert_equals( replace_course_urls(course_source, COURSE_ID), @@ -29,8 +30,7 @@ def test_debug_no_modify(mock_settings, mock_finders): mock_settings.DEBUG = True mock_finders.find.return_value = True - static_source = '"/static/file.png"' - assert_equals(static_source, replace_static_urls(static_source, DATA_DIRECTORY)) + assert_equals(STATIC_SOURCE, replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY)) mock_finders.find.assert_called_once_with('file.png', True) @@ -42,15 +42,23 @@ def test_mongo_filestore(mock_modulestore, mock_static_content): mock_modulestore.return_value = Mock(MongoModuleStore) mock_static_content.convert_legacy_static_url.return_value = "c4x://mock_url" - static_source = '"/static/file.png"' - # No namespace => no change to path - assert_equals(static_source, replace_static_urls(static_source, DATA_DIRECTORY)) + assert_equals('"/static/data_dir/file.png"', replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY)) # Namespace => content url assert_equals( '"' + mock_static_content.convert_legacy_static_url.return_value + '"', - replace_static_urls(static_source, DATA_DIRECTORY, NAMESPACE) + replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, NAMESPACE) ) mock_static_content.convert_legacy_static_url.assert_called_once_with('file.png', NAMESPACE) + +@patch('static_replace.settings') +@patch('static_replace.modulestore') +@patch('static_replace.staticfiles_storage') +def test_data_dir_fallback(mock_storage, mock_modulestore, mock_settings): + mock_modulestore.return_value = Mock(XMLModuleStore) + mock_settings.DEBUG = False + mock_storage.url.side_effect = Exception + + assert_equals('"/static/data_dir/file.png"', replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY)) From 4d8165b2b672335f88002a83cfa2be57872fcaaa Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 4 Feb 2013 15:08:30 -0500 Subject: [PATCH 15/15] Remove bad import of replace_urls --- lms/djangoapps/courseware/tabs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 64d488d845..4f5a881d97 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -19,7 +19,6 @@ from django.core.urlresolvers import reverse from fs.errors import ResourceNotFoundError from courseware.access import has_access -from static_replace import replace_urls from lxml.html import rewrite_links from module_render import get_module