Commit 987b9c11 by Victor Shnayder

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.
parent 3b6f33f5
...@@ -339,6 +339,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet): ...@@ -339,6 +339,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
module module
display_name: The name to use for displaying this module to the display_name: The name to use for displaying this module to the
user 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) format: The format of this module ('Homework', 'Lab', etc)
graded (bool): Whether this module is should be graded or not graded (bool): Whether this module is should be graded or not
start (string): The date for which this module will be available start (string): The date for which this module will be available
......
...@@ -12,18 +12,23 @@ _log = logging.getLogger("mitx.courseware") ...@@ -12,18 +12,23 @@ _log = logging.getLogger("mitx.courseware")
def grade_sheet(student, course, grader, student_module_cache): 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, - courseware_summary is a summary of all sections with problems in the
each containing an array of sections, each containing an array of scores. This contains information for graded and ungraded course. It is organized as an array of chapters, each containing an array of
problems, and is good for displaying a course summary with due dates, etc. 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: Arguments:
student: A User object for the student to grade student: A User object for the student to grade
course: An XModule containing the course 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 = {} totaled_scores = {}
chapters = [] chapters = []
...@@ -51,12 +56,16 @@ def grade_sheet(student, course, grader, student_module_cache): ...@@ -51,12 +56,16 @@ def grade_sheet(student, course, grader, student_module_cache):
correct = total correct = total
if not total > 0: 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 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 #Add the graded total to totaled_scores
format = s.metadata.get('format', "") format = s.metadata.get('format', "")
if format and graded_total.possible > 0: if format and graded_total.possible > 0:
...@@ -65,7 +74,8 @@ def grade_sheet(student, course, grader, student_module_cache): ...@@ -65,7 +74,8 @@ def grade_sheet(student, course, grader, student_module_cache):
totaled_scores[format] = format_scores totaled_scores[format] = format_scores
sections.append({ sections.append({
'section': s.metadata.get('display_name'), 'display_name': s.metadata.get('display_name'),
'url_name': s.metadata.get('url_name'),
'scores': scores, 'scores': scores,
'section_total': section_total, 'section_total': section_total,
'format': format, 'format': format,
...@@ -74,7 +84,8 @@ def grade_sheet(student, course, grader, student_module_cache): ...@@ -74,7 +84,8 @@ def grade_sheet(student, course, grader, student_module_cache):
}) })
chapters.append({'course': course.metadata.get('display_name'), 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}) 'sections': sections})
grade_summary = grader.grade(totaled_scores) grade_summary = grader.grade(totaled_scores)
......
...@@ -35,10 +35,12 @@ def toc_for_course(user, request, course, active_chapter, active_section): ...@@ -35,10 +35,12 @@ def toc_for_course(user, request, course, active_chapter, active_section):
Create a table of contents from the module store Create a table of contents from the module store
Return format: Return format:
[ {'name': name, 'sections': SECTIONS, 'active': bool}, ... ] [ {'display_name': name, 'url_name': url_name,
'sections': SECTIONS, 'active': bool}, ... ]
where SECTIONS is a list 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 active is set for the section and chapter corresponding to the passed
parameters. Everything else comes from the xml, or defaults to "". 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): ...@@ -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' hide_from_toc = section.metadata.get('hide_from_toc', 'false').lower() == 'true'
if not hide_from_toc: 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', ''), 'format': section.metadata.get('format', ''),
'due': section.metadata.get('due', ''), 'due': section.metadata.get('due', ''),
'active': active}) '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, 'sections': sections,
'active': chapter.metadata.get('display_name') == active_chapter}) 'active': chapter.metadata.get('display_name') == active_chapter})
return chapters return chapters
...@@ -76,8 +80,8 @@ def get_section(course_module, chapter, section): ...@@ -76,8 +80,8 @@ def get_section(course_module, chapter, section):
or None if this doesn't specify a valid section or None if this doesn't specify a valid section
course: Course url course: Course url
chapter: Chapter name chapter: Chapter url_name
section: Section name section: Section url_name
""" """
if course_module is None: if course_module is None:
...@@ -85,7 +89,7 @@ def get_section(course_module, chapter, section): ...@@ -85,7 +89,7 @@ def get_section(course_module, chapter, section):
chapter_module = None chapter_module = None
for _chapter in course_module.get_children(): for _chapter in course_module.get_children():
if _chapter.metadata.get('display_name') == chapter: if _chapter.metadata.get('url_name') == chapter:
chapter_module = _chapter chapter_module = _chapter
break break
...@@ -94,7 +98,7 @@ def get_section(course_module, chapter, section): ...@@ -94,7 +98,7 @@ def get_section(course_module, chapter, section):
section_module = None section_module = None
for _section in chapter_module.get_children(): for _section in chapter_module.get_children():
if _section.metadata.get('display_name') == section: if _section.metadata.get('url_name') == section:
section_module = _section section_module = _section
break break
...@@ -141,12 +145,12 @@ def get_module(user, request, location, student_module_cache, position=None): ...@@ -141,12 +145,12 @@ def get_module(user, request, location, student_module_cache, position=None):
# Setup system context for module instance # Setup system context for module instance
ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/'
# Fully qualified callback URL for external queueing system # Fully qualified callback URL for external queueing system
xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL + xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL +
'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' + 'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' +
'score_update') '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. # contains the current module.
# TODO: Queuename should be derived from 'course_settings.json' of each course # TODO: Queuename should be derived from 'course_settings.json' of each course
xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course xqueue_default_queuename = descriptor.location.org + '-' + descriptor.location.course
......
...@@ -54,11 +54,6 @@ def user_groups(user): ...@@ -54,11 +54,6 @@ def user_groups(user):
return group_names 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 @ensure_csrf_cookie
@cache_if_anonymous @cache_if_anonymous
...@@ -124,7 +119,6 @@ def profile(request, course_id, student_id=None): ...@@ -124,7 +119,6 @@ def profile(request, course_id, student_id=None):
'language': user_info.language, 'language': user_info.language,
'email': student.email, 'email': student.email,
'course': course, 'course': course,
'format_url_params': format_url_params,
'csrf': csrf(request)['csrf_token'] 'csrf': csrf(request)['csrf_token']
} }
context.update(grades.grade_sheet(student, course_module, course.grader, student_module_cache)) context.update(grades.grade_sheet(student, course_module, course.grader, student_module_cache))
...@@ -138,9 +132,9 @@ def render_accordion(request, course, chapter, section): ...@@ -138,9 +132,9 @@ def render_accordion(request, course, chapter, section):
If chapter and section are '' or None, renders a default accordion. 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) toc = toc_for_course(request.user, request, course, chapter, section)
active_chapter = 1 active_chapter = 1
...@@ -152,7 +146,6 @@ def render_accordion(request, course, chapter, section): ...@@ -152,7 +146,6 @@ def render_accordion(request, course, chapter, section):
('toc', toc), ('toc', toc),
('course_name', course.title), ('course_name', course.title),
('course_id', course.id), ('course_id', course.id),
('format_url_params', format_url_params),
('csrf', csrf(request)['csrf_token'])] + template_imports.items()) ('csrf', csrf(request)['csrf_token'])] + template_imports.items())
return render_to_string('accordion.html', context) return render_to_string('accordion.html', context)
...@@ -169,9 +162,9 @@ def index(request, course_id, chapter=None, section=None, ...@@ -169,9 +162,9 @@ def index(request, course_id, chapter=None, section=None,
Arguments: Arguments:
- request : HTTP request - request : HTTP request
- course : coursename (str) - course_id : course id (str: ORG/course/URL_NAME)
- chapter : chapter name (str) - chapter : chapter url_name (str)
- section : section name (str) - section : section url_name (str)
- position : position in module, eg of <sequential> module (str) - position : position in module, eg of <sequential> module (str)
Returns: Returns:
...@@ -180,16 +173,6 @@ def index(request, course_id, chapter=None, section=None, ...@@ -180,16 +173,6 @@ def index(request, course_id, chapter=None, section=None,
''' '''
course = check_course(course_id) 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: try:
context = { context = {
'csrf': csrf(request)['csrf_token'], 'csrf': csrf(request)['csrf_token'],
...@@ -202,8 +185,6 @@ def index(request, course_id, chapter=None, section=None, ...@@ -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 look_for_module = chapter is not None and section is not None
if look_for_module: if look_for_module:
# TODO (cpennington): Pass the right course in here
section_descriptor = get_section(course, chapter, section) section_descriptor = get_section(course, chapter, section)
if section_descriptor is not None: if section_descriptor is not None:
student_module_cache = StudentModuleCache(request.user, student_module_cache = StudentModuleCache(request.user,
......
<%! from django.core.urlresolvers import reverse %> <%! from django.core.urlresolvers import reverse %>
<%def name="make_chapter(chapter)"> <%def name="make_chapter(chapter)">
<h3><a href="#">${chapter['name']}</a></h3> <h3><a href="#">${chapter['display_name']}</a></h3>
<ul> <ul>
% for section in chapter['sections']: % for section in chapter['sections']:
<li${' class="active"' if 'active' in section and section['active'] else ''}> <li${' class="active"' if 'active' in section and section['active'] else ''}>
<a href="${reverse('courseware_section', args=[course_id] + format_url_params([chapter['name'], section['name']]))}"> <a href="${reverse('courseware_section', args=[course_id, chapter['url_name'], section['url_name']])}">
<p>${section['name']} <p>${section['display_name']}
<span class="subtitle"> <span class="subtitle">
${section['format']} ${"due " + section['due'] if 'due' in section and section['due'] != '' else ''} ${section['format']} ${"due " + section['due'] if 'due' in section and section['due'] != '' else ''}
</span> </span>
......
...@@ -72,7 +72,7 @@ $(function() { ...@@ -72,7 +72,7 @@ $(function() {
var new_email = $('#new_email_field').val(); var new_email = $('#new_email_field').val();
var new_password = $('#new_email_password').val(); var new_password = $('#new_email_password').val();
postJSON('/change_email',{"new_email":new_email, postJSON('/change_email',{"new_email":new_email,
"password":new_password}, "password":new_password},
function(data){ function(data){
if(data.success){ if(data.success){
...@@ -81,7 +81,7 @@ $(function() { ...@@ -81,7 +81,7 @@ $(function() {
$("#change_email_error").html(data.error); $("#change_email_error").html(data.error);
} }
}); });
log_event("profile", {"type":"email_change_request", log_event("profile", {"type":"email_change_request",
"old_email":"${email}", "old_email":"${email}",
"new_email":new_email}); "new_email":new_email});
return false; return false;
...@@ -91,7 +91,7 @@ $(function() { ...@@ -91,7 +91,7 @@ $(function() {
var new_name = $('#new_name_field').val(); var new_name = $('#new_name_field').val();
var rationale = $('#name_rationale_field').val(); var rationale = $('#name_rationale_field').val();
postJSON('/change_name',{"new_name":new_name, postJSON('/change_name',{"new_name":new_name,
"rationale":rationale}, "rationale":rationale},
function(data){ function(data){
if(data.success){ if(data.success){
...@@ -100,7 +100,7 @@ $(function() { ...@@ -100,7 +100,7 @@ $(function() {
$("#change_name_error").html(data.error); $("#change_name_error").html(data.error);
} }
}); });
log_event("profile", {"type":"name_change_request", log_event("profile", {"type":"name_change_request",
"new_name":new_name, "new_name":new_name,
"rationale":rationale}); "rationale":rationale});
return false; return false;
...@@ -125,9 +125,9 @@ $(function() { ...@@ -125,9 +125,9 @@ $(function() {
<ol class="chapters"> <ol class="chapters">
%for chapter in courseware_summary: %for chapter in courseware_summary:
%if not chapter['chapter'] == "hidden": %if not chapter['display_name'] == "hidden":
<li> <li>
<h2>${ chapter['chapter'] }</h2> <h2>${ chapter['display_name'] }</h2>
<ol class="sections"> <ol class="sections">
%for section in chapter['sections']: %for section in chapter['sections']:
...@@ -137,14 +137,14 @@ $(function() { ...@@ -137,14 +137,14 @@ $(function() {
total = section['section_total'].possible total = section['section_total'].possible
percentageString = "{0:.0%}".format( float(earned)/total) if earned > 0 and total > 0 else "" percentageString = "{0:.0%}".format( float(earned)/total) if earned > 0 and total > 0 else ""
%> %>
<h3><a href="${reverse('courseware_section', kwargs={'course_id' : course.id, 'chapter' : chapter['chapter'], 'section' : section['section']})}"> <h3><a href="${reverse('courseware_section', kwargs={'course_id' : course.id, 'chapter' : chapter['url_name'], 'section' : section['url_name']})}">
${ section['section'] }</a> ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}</h3> ${ section['display_name'] }</a> ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}</h3>
${section['format']} ${section['format']}
%if 'due' in section and section['due']!="": %if 'due' in section and section['due']!="":
due ${section['due']} due ${section['due']}
%endif %endif
%if len(section['scores']) > 0: %if len(section['scores']) > 0:
<ol class="scores"> <ol class="scores">
${ "Problem Scores: " if section['graded'] else "Practice Scores: "} ${ "Problem Scores: " if section['graded'] else "Practice Scores: "}
...@@ -153,7 +153,7 @@ $(function() { ...@@ -153,7 +153,7 @@ $(function() {
%endfor %endfor
</ol> </ol>
%endif %endif
</li> <!--End section--> </li> <!--End section-->
%endfor %endfor
</ol> <!--End sections--> </ol> <!--End sections-->
...@@ -181,7 +181,7 @@ $(function() { ...@@ -181,7 +181,7 @@ $(function() {
</li> </li>
<li> <li>
Forum name: <strong>${username}</strong> Forum name: <strong>${username}</strong>
</li> </li>
<li> <li>
...@@ -215,7 +215,7 @@ $(function() { ...@@ -215,7 +215,7 @@ $(function() {
<form id="change_name_form"> <form id="change_name_form">
<div id="change_name_error"> </div> <div id="change_name_error"> </div>
<fieldset> <fieldset>
<p>To uphold the credibility of <span class="edx">edX</span> certificates, name changes must go through an approval process. A member of the course staff will review your request, and if approved, update your information. Please allow up to a week for your request to be processed. Thank you.</p> <p>To uphold the credibility of <span class="edx">edX</span> certificates, name changes must go through an approval process. A member of the course staff will review your request, and if approved, update your information. Please allow up to a week for your request to be processed. Thank you.</p>
<ul> <ul>
<li> <li>
<label>Enter your desired full name, as it will appear on the <span class="edx">edX</span> Certificate: </label> <label>Enter your desired full name, as it will appear on the <span class="edx">edX</span> Certificate: </label>
...@@ -234,7 +234,7 @@ $(function() { ...@@ -234,7 +234,7 @@ $(function() {
</div> </div>
<div id="change_email" class="leanModal_box"> <div id="change_email" class="leanModal_box">
<h1>Change e-mail</h1> <h1>Change e-mail</h1>
<div id="apply_name_change_error"></div> <div id="apply_name_change_error"></div>
<form id="change_email_form"> <form id="change_email_form">
<div id="change_email_error"> </div> <div id="change_email_error"> </div>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment