From 987b9c11a94372f6d4c888755a1ce5ff5a036e30 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 2 Aug 2012 14:05:42 -0400 Subject: [PATCH 1/5] Use url_name for chapters and sections in lms views * got rid of the hackish conversions between ' ' and '_' * use url_name and display_name where appropriate * update templates to match. --- common/lib/xmodule/xmodule/x_module.py | 2 ++ lms/djangoapps/courseware/grades.py | 33 ++++++++++++++-------- lms/djangoapps/courseware/module_render.py | 28 ++++++++++-------- lms/djangoapps/courseware/views.py | 29 ++++--------------- lms/templates/accordion.html | 6 ++-- lms/templates/profile.html | 28 +++++++++--------- 6 files changed, 62 insertions(+), 64 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index ac6b5db5a4..1d16849d67 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -339,6 +339,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 diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 5a817e3d6c..717cbde140 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.metadata.get('display_name'), + 'url_name': s.metadata.get('url_name'), 'scores': scores, 'section_total': section_total, 'format': format, @@ -74,7 +84,8 @@ def grade_sheet(student, course, grader, student_module_cache): }) chapters.append({'course': course.metadata.get('display_name'), - 'chapter': c.metadata.get('display_name'), + 'display_name': c.metadata.get('display_name'), + 'url_name': c.metadata.get('url_name'), 'sections': sections}) grade_summary = grader.grade(totaled_scores) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 91d4efa651..4699ed50a4 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 "". @@ -59,12 +61,14 @@ def toc_for_course(user, request, course, active_chapter, 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.metadata.get('display_name'), + 'url_name': section.metadata.get('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.metadata.get('display_name'), + 'url_name': chapter.metadata.get('url_name'), 'sections': sections, 'active': chapter.metadata.get('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.metadata.get('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.metadata.get('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']}