diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 0447254cfb..0305795e52 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -108,7 +108,7 @@ def edit_item(request): 'contents': item.get_html(), 'js_module': item.js_module_name, 'category': item.category, - 'name': item.name, + 'url_name': item.url_name, 'previews': get_module_previews(request, item), }) diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 6aa780d42a..828f95ed47 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -1,7 +1,7 @@
-

${name}

+

${url_name}

${category}

diff --git a/cms/templates/widgets/navigation.html b/cms/templates/widgets/navigation.html index 9f9b37d571..ce18e867bd 100644 --- a/cms/templates/widgets/navigation.html +++ b/cms/templates/widgets/navigation.html @@ -41,7 +41,7 @@ % for week in weeks:
  • -

    ${week.name}

    +

    ${week.url_name}

      % if 'goals' in week.metadata: % for goal in week.metadata['goals']: @@ -60,7 +60,7 @@ data-type="${module.js_module_name}" data-preview-type="${module.module_class.js_module_name}"> - ${module.name} + ${module.url_name} handle % endfor diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index f7108e366e..c623eb4ec2 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -39,7 +39,7 @@ ${child.name} + data-preview-type="${child.module_class.js_module_name}">${child.url_name} handle %endfor diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 93b6a085c1..cb3c19487d 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -288,20 +288,30 @@ class LoncapaProblem(object): try: ifp = self.system.filestore.open(file) # open using ModuleSystem OSFS filestore except Exception as err: - log.error('Error %s in problem xml include: %s' % (err, etree.tostring(inc, pretty_print=True))) - log.error('Cannot find file %s in %s' % (file, self.system.filestore)) - if not self.system.get('DEBUG'): # if debugging, don't fail - just log error + log.error('Error %s in problem xml include: %s' % ( + err, etree.tostring(inc, pretty_print=True))) + log.error('Cannot find file %s in %s' % ( + file, self.system.filestore)) + # if debugging, don't fail - just log error + # TODO (vshnayder): need real error handling, display to users + if not self.system.get('DEBUG'): raise - else: continue + else: + continue try: incxml = etree.XML(ifp.read()) # read in and convert to XML except Exception as err: - log.error('Error %s in problem xml include: %s' % (err, etree.tostring(inc, pretty_print=True))) + log.error('Error %s in problem xml include: %s' % ( + err, etree.tostring(inc, pretty_print=True))) log.error('Cannot parse XML in %s' % (file)) - if not self.system.get('DEBUG'): # if debugging, don't fail - just log error + # if debugging, don't fail - just log error + # TODO (vshnayder): same as above + if not self.system.get('DEBUG'): raise - else: continue - parent = inc.getparent() # insert new XML into tree in place of inlcude + else: + continue + # insert new XML into tree in place of inlcude + parent = inc.getparent() parent.insert(parent.index(inc), incxml) parent.remove(inc) log.debug('Included %s into %s' % (file, self.problem_id)) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 7c3456e5ad..7a09004e33 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -121,13 +121,13 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): # Not proper format. Write html to file, return an empty tag filepath = u'{category}/{name}.html'.format(category=self.category, - name=self.name) + name=self.url_name) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: file.write(self.definition['data']) elt = etree.Element('html') - elt.set("filename", self.name) + elt.set("filename", self.url_name) return elt diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py index a383b3f8ec..baf3d46b57 100644 --- a/common/lib/xmodule/xmodule/modulestore/search.py +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -87,6 +87,7 @@ def path_to_location(modulestore, location, course_name=None): n = len(path) course_id = CourseDescriptor.location_to_id(path[0]) + # pull out the location names chapter = path[1].name if n > 1 else None section = path[2].name if n > 2 else None diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index da10e4bc91..fb68ba982b 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -23,11 +23,12 @@ class VideoModule(XModule): css = {'scss': [resource_string(__name__, 'css/video/display.scss')]} js_module_name = "Video" - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, + instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, + instance_state, shared_state, **kwargs) xmltree = etree.fromstring(self.definition['data']) self.youtube = xmltree.get('youtube') - self.name = xmltree.get('name') self.position = 0 if instance_state is not None: @@ -71,7 +72,7 @@ class VideoModule(XModule): 'streams': self.video_list(), 'id': self.location.html_id(), 'position': self.position, - 'name': self.name, + 'display_name': self.display_name, # TODO (cpennington): This won't work when we move to data that isn't on the filesystem 'data_dir': self.metadata['data_dir'], }) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index ac6b5db5a4..f6a43f2612 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -191,11 +191,20 @@ class XModule(HTMLSnippet): self.instance_state = instance_state self.shared_state = shared_state self.id = self.location.url() - self.name = self.location.name + self.url_name = self.location.name self.category = self.location.category self.metadata = kwargs.get('metadata', {}) self._loaded_children = None + @property + def display_name(self): + ''' + Return a display name for the module: use display_name if defined in + metadata, otherwise convert the url name. + ''' + return self.metadata.get('display_name', + self.url_name.replace('_', ' ')) + def get_children(self): ''' Return module instances for all the children of this module. @@ -339,6 +348,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet): module display_name: The name to use for displaying this module to the user + url_name: The name to use for this module in urls and other places + where a unique name is needed. format: The format of this module ('Homework', 'Lab', etc) graded (bool): Whether this module is should be graded or not start (string): The date for which this module will be available @@ -353,13 +364,22 @@ class XModuleDescriptor(Plugin, HTMLSnippet): self.metadata = kwargs.get('metadata', {}) self.definition = definition if definition is not None else {} self.location = Location(kwargs.get('location')) - self.name = self.location.name + self.url_name = self.location.name self.category = self.location.category self.shared_state_key = kwargs.get('shared_state_key') self._child_instances = None self._inherited_metadata = set() + @property + def display_name(self): + ''' + Return a display name for the module: use display_name if defined in + metadata, otherwise convert the url name. + ''' + return self.metadata.get('display_name', + self.url_name.replace('_', ' ')) + @property def own_metadata(self): """ diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index ea13bc2640..b0a289d149 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -225,7 +225,7 @@ class XmlDescriptor(XModuleDescriptor): # Write it to a file if necessary if self.split_to_file(xml_object): # Put this object in its own file - filepath = self.__class__._format_filepath(self.category, self.name) + filepath = self.__class__._format_filepath(self.category, self.url_name) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: file.write(etree.tostring(xml_object, pretty_print=True)) @@ -238,10 +238,10 @@ class XmlDescriptor(XModuleDescriptor): xml_object.tail = '' - xml_object.set('filename', self.name) + xml_object.set('filename', self.url_name) # Add the metadata - xml_object.set('url_name', self.name) + xml_object.set('url_name', self.url_name) for attr in self.metadata_attributes: attr_map = self.xml_attribute_map.get(attr, AttrMap(attr)) metadata_key = attr_map.metadata_key diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index cfda6497d5..19eef3ee80 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -83,7 +83,7 @@ def get_course_about_section(course, section_key): log.warning("Missing about section {key} in course {url}".format(key=section_key, url=course.location.url())) return None elif section_key == "title": - return course.metadata.get('display_name', course.name) + return course.metadata.get('display_name', course.url_name) elif section_key == "university": return course.location.org elif section_key == "number": diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 5a817e3d6c..6e7a0ce102 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -12,18 +12,23 @@ _log = logging.getLogger("mitx.courseware") def grade_sheet(student, course, grader, student_module_cache): """ - This pulls a summary of all problems in the course. It returns a dictionary with two datastructures: + This pulls a summary of all problems in the course. It returns a dictionary + with two datastructures: - - courseware_summary is a summary of all sections with problems in the course. It is organized as an array of chapters, - each containing an array of sections, each containing an array of scores. This contains information for graded and ungraded - problems, and is good for displaying a course summary with due dates, etc. + - courseware_summary is a summary of all sections with problems in the + course. It is organized as an array of chapters, each containing an array of + sections, each containing an array of scores. This contains information for + graded and ungraded problems, and is good for displaying a course summary + with due dates, etc. - - grade_summary is the output from the course grader. More information on the format is in the docstring for CourseGrader. + - grade_summary is the output from the course grader. More information on + the format is in the docstring for CourseGrader. Arguments: student: A User object for the student to grade course: An XModule containing the course to grade - student_module_cache: A StudentModuleCache initialized with all instance_modules for the student + student_module_cache: A StudentModuleCache initialized with all + instance_modules for the student """ totaled_scores = {} chapters = [] @@ -51,12 +56,16 @@ def grade_sheet(student, course, grader, student_module_cache): correct = total if not total > 0: - #We simply cannot grade a problem that is 12/0, because we might need it as a percentage + #We simply cannot grade a problem that is 12/0, because we + #might need it as a percentage graded = False - scores.append(Score(correct, total, graded, module.metadata.get('display_name'))) + scores.append(Score(correct, total, graded, + module.metadata.get('display_name'))) + + section_total, graded_total = graders.aggregate_scores( + scores, s.metadata.get('display_name')) - section_total, graded_total = graders.aggregate_scores(scores, s.metadata.get('display_name')) #Add the graded total to totaled_scores format = s.metadata.get('format', "") if format and graded_total.possible > 0: @@ -65,7 +74,8 @@ def grade_sheet(student, course, grader, student_module_cache): totaled_scores[format] = format_scores sections.append({ - 'section': s.metadata.get('display_name'), + 'display_name': s.display_name, + 'url_name': s.url_name, 'scores': scores, 'section_total': section_total, 'format': format, @@ -73,8 +83,9 @@ def grade_sheet(student, course, grader, student_module_cache): 'graded': graded, }) - chapters.append({'course': course.metadata.get('display_name'), - 'chapter': c.metadata.get('display_name'), + chapters.append({'course': course.display_name, + 'display_name': c.display_name, + 'url_name': c.url_name, 'sections': sections}) grade_summary = grader.grade(totaled_scores) diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py index 1773c3d21f..de845df572 100644 --- a/lms/djangoapps/courseware/management/commands/clean_xml.py +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -61,11 +61,7 @@ def import_with_checks(course_dir, verbose=True): course_dirs=course_dirs) def str_of_err(tpl): - (msg, exc_info) = tpl - if exc_info is None: - return msg - - exc_str = '\n'.join(traceback.format_exception(*exc_info)) + (msg, exc_str) = tpl return '{msg}\n{exc}'.format(msg=msg, exc=exc_str) courses = modulestore.get_courses() @@ -83,7 +79,7 @@ def import_with_checks(course_dir, verbose=True): print '\n' print "=" * 40 print 'ERRORs during import:' - print '\n'.join(map(str_of_err,errors)) + print '\n'.join(map(str_of_err, errors)) print "=" * 40 print '\n' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 91d4efa651..9260e15c61 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -35,10 +35,12 @@ def toc_for_course(user, request, course, active_chapter, active_section): Create a table of contents from the module store Return format: - [ {'name': name, 'sections': SECTIONS, 'active': bool}, ... ] + [ {'display_name': name, 'url_name': url_name, + 'sections': SECTIONS, 'active': bool}, ... ] where SECTIONS is a list - [ {'name': name, 'format': format, 'due': due, 'active' : bool}, ...] + [ {'display_name': name, 'url_name': url_name, + 'format': format, 'due': due, 'active' : bool}, ...] active is set for the section and chapter corresponding to the passed parameters. Everything else comes from the xml, or defaults to "". @@ -54,19 +56,21 @@ def toc_for_course(user, request, course, active_chapter, active_section): sections = list() for section in chapter.get_display_items(): - active = (chapter.metadata.get('display_name') == active_chapter and - section.metadata.get('display_name') == active_section) + active = (chapter.display_name == active_chapter and + section.display_name == active_section) hide_from_toc = section.metadata.get('hide_from_toc', 'false').lower() == 'true' if not hide_from_toc: - sections.append({'name': section.metadata.get('display_name'), + sections.append({'display_name': section.display_name, + 'url_name': section.url_name, 'format': section.metadata.get('format', ''), 'due': section.metadata.get('due', ''), 'active': active}) - chapters.append({'name': chapter.metadata.get('display_name'), + chapters.append({'display_name': chapter.display_name, + 'url_name': chapter.url_name, 'sections': sections, - 'active': chapter.metadata.get('display_name') == active_chapter}) + 'active': chapter.display_name == active_chapter}) return chapters @@ -76,8 +80,8 @@ def get_section(course_module, chapter, section): or None if this doesn't specify a valid section course: Course url - chapter: Chapter name - section: Section name + chapter: Chapter url_name + section: Section url_name """ if course_module is None: @@ -85,7 +89,7 @@ def get_section(course_module, chapter, section): chapter_module = None for _chapter in course_module.get_children(): - if _chapter.metadata.get('display_name') == chapter: + if _chapter.url_name == chapter: chapter_module = _chapter break @@ -94,7 +98,7 @@ def get_section(course_module, chapter, section): section_module = None for _section in chapter_module.get_children(): - if _section.metadata.get('display_name') == section: + if _section.url_name == section: section_module = _section break @@ -141,12 +145,12 @@ def get_module(user, request, location, student_module_cache, position=None): # Setup system context for module instance ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' - # Fully qualified callback URL for external queueing system - xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL + - 'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' + + # Fully qualified callback URL for external queueing system + xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL + + 'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' + 'score_update') - # Default queuename is course-specific and is derived from the course that + # Default queuename is course-specific and is derived from the course that # contains the current module. # TODO: Queuename should be derived from 'course_settings.json' of each course xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 2b55b48caf..18b710e108 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -54,11 +54,6 @@ def user_groups(user): return group_names -def format_url_params(params): - return [urllib.quote(string.replace(' ', '_')) - if string is not None else None - for string in params] - @ensure_csrf_cookie @cache_if_anonymous @@ -124,7 +119,6 @@ def profile(request, course_id, student_id=None): 'language': user_info.language, 'email': student.email, 'course': course, - 'format_url_params': format_url_params, 'csrf': csrf(request)['csrf_token'] } context.update(grades.grade_sheet(student, course_module, course.grader, student_module_cache)) @@ -138,9 +132,9 @@ def render_accordion(request, course, chapter, section): If chapter and section are '' or None, renders a default accordion. - Returns (initialization_javascript, content)''' + Returns the html string''' - # TODO (cpennington): do the right thing with courses + # grab the table of contents toc = toc_for_course(request.user, request, course, chapter, section) active_chapter = 1 @@ -152,7 +146,6 @@ def render_accordion(request, course, chapter, section): ('toc', toc), ('course_name', course.title), ('course_id', course.id), - ('format_url_params', format_url_params), ('csrf', csrf(request)['csrf_token'])] + template_imports.items()) return render_to_string('accordion.html', context) @@ -169,9 +162,9 @@ def index(request, course_id, chapter=None, section=None, Arguments: - request : HTTP request - - course : coursename (str) - - chapter : chapter name (str) - - section : section name (str) + - course_id : course id (str: ORG/course/URL_NAME) + - chapter : chapter url_name (str) + - section : section url_name (str) - position : position in module, eg of module (str) Returns: @@ -180,16 +173,6 @@ def index(request, course_id, chapter=None, section=None, ''' course = check_course(course_id) - def clean(s): - ''' Fixes URLs -- we convert spaces to _ in URLs to prevent - funny encoding characters and keep the URLs readable. This undoes - that transformation. - ''' - return s.replace('_', ' ') if s is not None else None - - chapter = clean(chapter) - section = clean(section) - try: context = { 'csrf': csrf(request)['csrf_token'], @@ -202,8 +185,6 @@ def index(request, course_id, chapter=None, section=None, look_for_module = chapter is not None and section is not None if look_for_module: - # TODO (cpennington): Pass the right course in here - section_descriptor = get_section(course, chapter, section) if section_descriptor is not None: student_module_cache = StudentModuleCache(request.user, diff --git a/lms/templates/accordion.html b/lms/templates/accordion.html index defb424a29..353b83db70 100644 --- a/lms/templates/accordion.html +++ b/lms/templates/accordion.html @@ -1,13 +1,13 @@ <%! from django.core.urlresolvers import reverse %> <%def name="make_chapter(chapter)"> -

      ${chapter['name']}

      +

      ${chapter['display_name']}