diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index fe6ebdd..7c4de7d 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -26,8 +26,29 @@ class I4xSystem(object): This is an abstraction such that x_modules can function independent of the courseware (e.g. import into other types of courseware, LMS, or if we want to have a sandbox server for user-contributed content) + + I4xSystem objects are passed to x_modules to provide access to system + functionality. ''' - def __init__(self, ajax_url, track_function, render_function, render_template, filestore=None): + def __init__(self, ajax_url, track_function, render_function, + render_template, filestore=None): + ''' + Create a closure around the system environment. + + ajax_url - the url where ajax calls to the encapsulating module go. + track_function - function of (event_type, event), intended for logging + or otherwise tracking the event. + TODO: Not used, and has inconsistent args in different + files. Update or remove. + render_function - function that takes (module_xml) and renders it, + returning a dictionary with a context for rendering the + module to html. Dictionary will contain keys 'content' + and 'type'. + render_template - a function that takes (template_file, context), and returns + rendered html. + filestore - A filestore ojbect. Defaults to an instance of OSFS based at + settings.DATA_DIR. + ''' self.ajax_url = ajax_url self.track_function = track_function if not filestore: @@ -35,37 +56,47 @@ class I4xSystem(object): else: self.filestore = filestore if settings.DEBUG: - log.info("[courseware.module_render.I4xSystem] filestore path = %s" % filestore) + log.info("[courseware.module_render.I4xSystem] filestore path = %s", + filestore) self.render_function = render_function self.render_template = render_template self.exception404 = Http404 self.DEBUG = settings.DEBUG - def get(self,attr): # uniform access to attributes (like etree) + def get(self, attr): + ''' provide uniform access to attributes (like etree).''' return self.__dict__.get(attr) - def set(self,attr,val): # uniform access to attributes (like etree) + + def set(self,attr,val): + '''provide uniform access to attributes (like etree)''' self.__dict__[attr] = val + def __repr__(self): return repr(self.__dict__) + def __str__(self): return str(self.__dict__) -def object_cache(cache, user, module_type, module_id): - # We don't look up on user -- all queries include user - # Additional lookup would require a DB hit the way Django - # is broken. +def smod_cache_lookup(cache, module_type, module_id): + ''' + Look for a student module with the given type and id in the cache. + + cache -- list of student modules + + returns first found object, or None + ''' for o in cache: - if o.module_type == module_type and \ - o.module_id == module_id: + if o.module_type == module_type and o.module_id == module_id: return o return None def make_track_function(request): ''' We want the capa problem (and other modules) to be able to track/log what happens inside them without adding dependencies on - Django or the rest of the codebase. We do this by passing a - tracking function to them. This generates a closure for each request - that gives a clean interface on both sides. + Django or the rest of the codebase. + + To do this in a clean way, we pass a tracking function to the module, + which calls it to log events. ''' import track.views @@ -75,85 +106,91 @@ def make_track_function(request): def grade_histogram(module_id): ''' Print out a histogram of grades on a given problem. - Part of staff member debug info. + Part of staff member debug info. ''' from django.db import connection cursor = connection.cursor() - cursor.execute("select courseware_studentmodule.grade,COUNT(courseware_studentmodule.student_id) from courseware_studentmodule where courseware_studentmodule.module_id=%s group by courseware_studentmodule.grade", [module_id]) + q = """SELECT courseware_studentmodule.grade, + COUNT(courseware_studentmodule.student_id) + FROM courseware_studentmodule + WHERE courseware_studentmodule.module_id=%s + GROUP BY courseware_studentmodule.grade""" + # Passing module_id this way prevents sql-injection. + cursor.execute(q, [module_id]) grades = list(cursor.fetchall()) - grades.sort(key=lambda x:x[0]) # Probably not necessary - if (len(grades) == 1 and grades[0][0] is None): + grades.sort(key=lambda x: x[0]) # Add ORDER BY to sql query? + if len(grades) == 1 and grades[0][0] is None: return [] return grades -def get_module(user, request, xml_module, module_object_preload, position=None): - ''' Get the appropriate xmodule and StudentModule. +def get_module(user, request, module_xml, student_module_cache, position=None): + ''' Get an instance of the xmodule class corresponding to module_xml, + setting the state based on an existing StudentModule, or creating one if none + exists. Arguments: - user : current django User - request : current django HTTPrequest - - xml_module : lxml etree of xml subtree for the current module - - module_object_preload : list of StudentModule objects, one of which may match this module type and id - - position : extra information from URL for user-specified position within module + - module_xml : lxml etree of xml subtree for the requested module + - student_module_cache : list of StudentModule objects, one of which may + match this module type and id + - position : extra information from URL for user-specified + position within module Returns: - a tuple (xmodule instance, student module, module type). - ''' - module_type=xml_module.tag - module_class=xmodule.get_module_class(module_type) - module_id=xml_module.get('id') #module_class.id_attribute) or "" - - # Grab state from database - smod = object_cache(module_object_preload, - user, - module_type, - module_id) - - if not smod: # If nothing in the database... - state=None - else: - state = smod.state + module_type = module_xml.tag + module_class = xmodule.get_module_class(module_type) + module_id = module_xml.get('id') + + # Grab xmodule state from StudentModule cache + smod = smod_cache_lookup(student_module_cache, module_type, module_id) + state = smod.state if smod else None - # get coursename if stored + # get coursename if present in request coursename = multicourse_settings.get_coursename_from_request(request) if coursename and settings.ENABLE_MULTICOURSE: - xp = multicourse_settings.get_course_xmlpath(coursename) # path to XML for the course + # path to XML for the course + xp = multicourse_settings.get_course_xmlpath(coursename) data_root = settings.DATA_DIR + xp else: data_root = settings.DATA_DIR - # Create a new instance - ajax_url = settings.MITX_ROOT_URL + '/modx/'+module_type+'/'+module_id+'/' + # Setup system context for module instance + ajax_url = settings.MITX_ROOT_URL + '/modx/' + module_type + '/' + module_id + '/' + def render_function(module_xml): + return render_x_module(user, request, module_xml, student_module_cache, position) system = I4xSystem(track_function = make_track_function(request), - render_function = lambda x: render_x_module(user, request, x, module_object_preload, position), + render_function = render_function, render_template = render_to_string, ajax_url = ajax_url, filestore = OSFS(data_root), ) - system.set('position',position) # pass URL specified position along to module, through I4xSystem - instance=module_class(system, - etree.tostring(xml_module), - module_id, - state=state) + # pass position specified in URL to module through I4xSystem + system.set('position', position) + instance = module_class(system, + etree.tostring(module_xml), + module_id, + state=state) - # If instance wasn't already in the database, and this - # isn't a guest user, create it + # If StudentModule for this instance wasn't already in the database, + # and this isn't a guest user, create it. if not smod and user.is_authenticated(): - smod=StudentModule(student=user, - module_type = module_type, - module_id=module_id, - state=instance.get_state()) + smod = StudentModule(student=user, module_type = module_type, + module_id=module_id, state=instance.get_state()) smod.save() - module_object_preload.append(smod) + # Add to cache. The caller and the system context have references + # to it, so the change persists past the return + student_module_cache.append(smod) return (instance, smod, module_type) -def render_x_module(user, request, xml_module, module_object_preload, position=None): +def render_x_module(user, request, module_xml, student_module_cache, position=None): ''' Generic module for extensions. This renders to HTML. modules include sequential, vertical, problem, video, html @@ -164,37 +201,36 @@ def render_x_module(user, request, xml_module, module_object_preload, position=N - user : current django User - request : current django HTTPrequest - - xml_module : lxml etree of xml subtree for the current module - - module_object_preload : list of StudentModule objects, one of which may match this module type and id + - module_xml : lxml etree of xml subtree for the current module + - student_module_cache : list of StudentModule objects, one of which may match this module type and id - position : extra information from URL for user-specified position within module Returns: - - dict which is context for HTML rendering of the specified module - + - dict which is context for HTML rendering of the specified module. Will have + key 'content', and will have 'type' key if passed a valid module. ''' - if xml_module==None : - return {"content":""} + if module_xml is None : + return {"content": ""} - (instance, smod, module_type) = get_module(user, request, xml_module, module_object_preload, position) + (instance, smod, module_type) = get_module( + user, request, module_xml, student_module_cache, position) - # Grab content content = instance.get_html() # special extra information about each problem, only for users who are staff if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: - module_id = xml_module.get('id') + module_id = module_xml.get('id') histogram = grade_histogram(module_id) render_histogram = len(histogram) > 0 - content=content+render_to_string("staff_problem_info.html", {'xml':etree.tostring(xml_module), - 'module_id' : module_id, - 'histogram': json.dumps(histogram), - 'render_histogram' : render_histogram}) + staff_context = {'xml': etree.tostring(module_xml), + 'module_id': module_id, + 'histogram': json.dumps(histogram), + 'render_histogram': render_histogram} + content += render_to_string("staff_problem_info.html", staff_context) - content = {'content':content, - 'type':module_type} - - return content + context = {'content': content, 'type': module_type} + return context def modx_dispatch(request, module=None, dispatch=None, id=None): ''' Generic view for extensions. This is where AJAX calls go. @@ -210,32 +246,38 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): - id -- the module id. Used to look up the student module. e.g. filenamexformularesponse ''' + # ''' (fix emacs broken parsing) if not request.user.is_authenticated(): return redirect('/') + # python concats adjacent strings + error_msg = ("We're sorry, this module is temporarily unavailable." + "Our staff is working to fix it as soon as possible") + + # Grab the student information for the module from the database s = StudentModule.objects.filter(student=request.user, module_id=id) - #s = StudentModule.get_with_caching(request.user, id) - if len(s) == 0 or s is None: - log.debug("Couldnt find module for user and id " + str(module) + " " + str(request.user) + " "+ str(id)) + + # s = StudentModule.get_with_caching(request.user, id) + if s is None or len(s) == 0: + log.debug("Couldn't find module '%s' for user '%s' and id '%s'", + module, request.user, id) raise Http404 s = s[0] oldgrade = s.grade oldstate = s.state - # TODO: if dispatch is left at default value None, this will go boom. What's the correct - # behavior? - dispatch=dispatch.split('?')[0] - - ajax_url = settings.MITX_ROOT_URL + '/modx/'+module+'/'+id+'/' + # If there are arguments, get rid of them + if '?' in dispatch: + dispatch = dispatch.split('?')[0] - # get coursename if stored + ajax_url = '{root}/modx/{module}/{id}'.format(root = settings.MITX_ROOT_URL, + module=module, id=id) coursename = multicourse_settings.get_coursename_from_request(request) - if coursename and settings.ENABLE_MULTICOURSE: - xp = multicourse_settings.get_course_xmlpath(coursename) # path to XML for the course + xp = multicourse_settings.get_course_xmlpath(coursename) data_root = settings.DATA_DIR + xp else: data_root = settings.DATA_DIR @@ -244,11 +286,13 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): try: xml = content_parser.module_xml(request.user, module, 'id', id, coursename) except: - log.exception("Unable to load module during ajax call. module=%s, dispatch=%s, id=%s" % (module, dispatch, id)) + log.exception( + "Unable to load module during ajax call. module=%s, dispatch=%s, id=%s", + module, dispatch, id) if accepts(request, 'text/html'): return render_to_response("module-error.html", {}) else: - response = HttpResponse(json.dumps({'success': "We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible"})) + response = HttpResponse(json.dumps({'success': error_msg})) return response # Create the module @@ -260,24 +304,23 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): ) try: - instance=xmodule.get_module_class(module)(system, - xml, - id, - state=oldstate) + module_class = xmodule.get_module_class(module) + instance = module_class(system, xml, id, state=oldstate) except: log.exception("Unable to load module instance during ajax call") if accepts(request, 'text/html'): return render_to_response("module-error.html", {}) else: - response = HttpResponse(json.dumps({'success': "We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible"})) + response = HttpResponse(json.dumps({'success': error_msg})) return response # Let the module handle the AJAX - ajax_return=instance.handle_ajax(dispatch, request.POST) + ajax_return = instance.handle_ajax(dispatch, request.POST) + # Save the state back to the database - s.state=instance.get_state() + s.state = instance.get_state() if instance.get_score(): - s.grade=instance.get_score()['score'] + s.grade = instance.get_score()['score'] if s.grade != oldgrade or s.state != oldstate: s.save() # Return whatever the module wanted to return to the client/caller diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index fcd0104..7bc8b32 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -41,66 +41,71 @@ def gradebook(request): coursename = multicourse_settings.get_coursename_from_request(request) student_objects = User.objects.all()[:100] - student_info = [{'username' :s.username, - 'id' : s.id, + student_info = [{'username': s.username, + 'id': s.id, 'email': s.email, - 'grade_info' : grades.grade_sheet(s,coursename), - 'realname' : UserProfile.objects.get(user = s).name + 'grade_info': grades.grade_sheet(s, coursename), + 'realname': UserProfile.objects.get(user = s).name } for s in student_objects] - return render_to_response('gradebook.html',{'students':student_info}) + return render_to_response('gradebook.html', {'students': student_info}) @login_required @cache_control(no_cache=True, no_store=True, must_revalidate=True) -def profile(request, student_id = None): +def profile(request, student_id=None): ''' User profile. Show username, location, etc, as well as grades . We need to allow the user to change some of these settings .''' if student_id is None: student = request.user - else: + else: if 'course_admin' not in content_parser.user_groups(request.user): raise Http404 student = User.objects.get( id = int(student_id)) - user_info = UserProfile.objects.get(user=student) # request.user.profile_cache # + user_info = UserProfile.objects.get(user=student) # request.user.profile_cache # coursename = multicourse_settings.get_coursename_from_request(request) - context={'name':user_info.name, - 'username':student.username, - 'location':user_info.location, - 'language':user_info.language, - 'email':student.email, - 'format_url_params' : content_parser.format_url_params, - 'csrf':csrf(request)['csrf_token'] - } - context.update(grades.grade_sheet(student,coursename)) + context = {'name': user_info.name, + 'username': student.username, + 'location': user_info.location, + 'language': user_info.language, + 'email': student.email, + 'format_url_params': content_parser.format_url_params, + 'csrf': csrf(request)['csrf_token'] + } + context.update(grades.grade_sheet(student, coursename)) return render_to_response('profile.html', context) -def render_accordion(request,course,chapter,section): + +def render_accordion(request, course, chapter, section): ''' Draws navigation bar. Takes current position in accordion as parameter. Returns (initialization_javascript, content)''' if not course: course = "6.002 Spring 2012" - toc=content_parser.toc_from_xml(content_parser.course_file(request.user,course), chapter, section) - active_chapter=1 + toc = content_parser.toc_from_xml( + content_parser.course_file(request.user, course), chapter, section) + + active_chapter = 1 for i in range(len(toc)): if toc[i]['active']: - active_chapter=i - context=dict([['active_chapter',active_chapter], - ['toc',toc], - ['course_name',course], - ['format_url_params',content_parser.format_url_params], - ['csrf',csrf(request)['csrf_token']]] + \ + active_chapter = i + + context=dict([('active_chapter', active_chapter), + ('toc', toc), + ('course_name', course), + ('format_url_params', content_parser.format_url_params), + ('csrf', csrf(request)['csrf_token'])] + template_imports.items()) - return render_to_string('accordion.html',context) + return render_to_string('accordion.html', context) + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def render_section(request, section): - ''' TODO: Consolidate with index + ''' TODO: Consolidate with index ''' user = request.user if not settings.COURSEWARE_ENABLED: @@ -120,15 +125,15 @@ def render_section(request, section): } module_ids = dom.xpath("//@id") - + if user.is_authenticated(): - module_object_preload = list(StudentModule.objects.filter(student=user, + student_module_cache = list(StudentModule.objects.filter(student=user, module_id__in=module_ids)) else: - module_object_preload = [] - + student_module_cache = [] + try: - module = render_x_module(user, request, dom, module_object_preload) + module = render_x_module(user, request, dom, student_module_cache) except: log.exception("Unable to load module") context.update({ @@ -138,35 +143,19 @@ def render_section(request, section): return render_to_response('courseware.html', context) context.update({ - 'init':module.get('init_js', ''), - 'content':module['content'], + 'init': module.get('init_js', ''), + 'content': module['content'], }) result = render_to_response('courseware.html', context) return result +def get_course(request, course): + ''' Figure out what the correct course is. -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -def index(request, course=None, chapter="Using the System", section="Hints",position=None): - ''' Displays courseware accordion, and any associated content. - - Arguments: - - - request : HTTP request - - course : coursename (str) - - chapter : chapter name (str) - - section : section name (str) - - position : position in module, eg of <sequential> module (str) - - Returns: - - - HTTPresponse - - ''' - user = request.user - if not settings.COURSEWARE_ENABLED: - return redirect('/') + Needed to preserve backwards compatibility with non-multi-course version. + TODO: Can this go away once multicourse becomes standard? + ''' if course==None: if not settings.ENABLE_MULTICOURSE: @@ -175,97 +164,165 @@ def index(request, course=None, chapter="Using the System", section="Hints",posi course = request.session['coursename'] else: course = settings.COURSE_DEFAULT + return course - # Fixes URLs -- we don't get funny encoding characters from spaces - # so they remain readable - ## TODO: Properly replace underscores - course=course.replace("_"," ") - chapter=chapter.replace("_"," ") - section=section.replace("_"," ") - - # use multicourse module to determine if "course" is valid - #if course!=settings.COURSE_NAME.replace('_',' '): - if not multicourse_settings.is_valid_course(course): - return redirect('/') +def get_module_xml(user, course, chapter, section): + ''' Look up the module xml for the given course/chapter/section path. - request.session['coursename'] = course # keep track of current course being viewed in django's request.session + Takes the user to look up the course file. + Returns None if there was a problem, or the lxml etree for the module. + ''' try: - # this is the course.xml etree - dom = content_parser.course_file(user,course) # also pass course to it, for course-specific XML path + # this is the course.xml etree + dom = content_parser.course_file(user, course) except: log.exception("Unable to parse courseware xml") - return render_to_response('courseware-error.html', {}) + return None # this is the module's parent's etree - dom_module = dom.xpath("//course[@name=$course]/chapter[@name=$chapter]//section[@name=$section]", - course=course, chapter=chapter, section=section) - - #print "DM", dom_module - - if len(dom_module) == 0: - module_wrapper = None - else: - module_wrapper = dom_module[0] + path = "//course[@name=$course]/chapter[@name=$chapter]//section[@name=$section]" + dom_module = dom.xpath(path, course=course, chapter=chapter, section=section) + module_wrapper = dom_module[0] if len(dom_module) > 0 else None if module_wrapper is None: module = None elif module_wrapper.get("src"): - module = content_parser.section_file(user=user, section=module_wrapper.get("src"), coursename=course) + module = content_parser.section_file( + user=user, section=module_wrapper.get("src"), coursename=course) else: - # this is the module's etree - module = etree.XML(etree.tostring(module_wrapper[0])) # Copy the element out of the tree + # Copy the element out of the module's etree + module = etree.XML(etree.tostring(module_wrapper[0])) + return module - module_ids = [] - if module is not None: - module_ids = module.xpath("//@id", - course=course, chapter=chapter, section=section) - if user.is_authenticated(): - module_object_preload = list(StudentModule.objects.filter(student=user, - module_id__in=module_ids)) - else: - module_object_preload = [] +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +def index(request, course=None, chapter=None, section=None, + position=None): + ''' Displays courseware accordion, and any associated content. + If course, chapter, and section aren't all specified, just returns + the accordion. If they are specified, returns an error if they don't + point to a valid module. + + Arguments: + + - request : HTTP request + - course : coursename (str) + - chapter : chapter name (str) + - section : section name (str) + - position : position in module, eg of <sequential> module (str) + + Returns: + + - HTTPresponse + ''' + 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. + + TODO: Properly replace underscores. (Q: what is properly?) + ''' + return s.replace('_', ' ') + + def get_submodule_ids(module_xml): + ''' + Get a list with ids of the modules within this module. + ''' + return module_xml.xpath("//@id") + + def preload_student_modules(module_xml): + ''' + Find any StudentModule objects for this user that match + one of the given module_ids. Used as a cache to avoid having + each rendered module hit the db separately. + + Returns the list, or None on error. + ''' + if request.user.is_authenticated(): + module_ids = get_submodule_ids(module_xml) + return list(StudentModule.objects.filter(student=request.user, + module_id__in=module_ids)) + else: + return [] + + def get_module_context(): + ''' + Look up the module object and render it. If all goes well, returns + {'init': module-init-js, 'content': module-rendered-content} + + If there's an error, returns + {'content': module-error message} + ''' + # Can't modify variables of outer scope, so need new ones + chapter_ = clean(chapter) + section_ = clean(section) + + user = request.user + + module_xml = get_module_xml(user, course, chapter_, section_) + if module_xml is None: + log.exception("couldn't get module_xml: course/chapter/section: '%s/%s/%s'", + course, chapter_, section_) + return {'content' : render_to_string("module-error.html", {})} + + student_module_cache = preload_student_modules(module_xml) + + try: + module_context = render_x_module(user, request, module_xml, + student_module_cache, position) + except: + log.exception("Unable to load module") + return {'content' : render_to_string("module-error.html", {})} + + return {'init': module_context.get('init_js', ''), + 'content': module_context['content']} + + if not settings.COURSEWARE_ENABLED: + return redirect('/') + + course = clean(get_course(request, course)) + if not multicourse_settings.is_valid_course(course): + return redirect('/') + + # keep track of current course being viewed in django's request.session + request.session['coursename'] = course context = { 'csrf': csrf(request)['csrf_token'], 'accordion': render_accordion(request, course, chapter, section), - 'COURSE_TITLE':multicourse_settings.get_course_title(course), + 'COURSE_TITLE': multicourse_settings.get_course_title(course), + 'init': '', + 'content': '' } - try: - module_context = render_x_module(user, request, module, module_object_preload, position) - except: - log.exception("Unable to load module") - context.update({ - 'init': '', - 'content': render_to_string("module-error.html", {}), - }) - return render_to_response('courseware.html', context) - - context.update({ - 'init': module_context.get('init_js', ''), - 'content': module_context['content'], - }) + look_for_module = chapter is not None and section is not None + if look_for_module: + context.update(get_module_context()) result = render_to_response('courseware.html', context) return result def jump_to(request, probname=None): ''' - Jump to viewing a specific problem. The problem is specified by a problem name - currently the filename (minus .xml) - of the problem. Maybe this should change to a more generic tag, eg "name" given as an attribute in <problem>. + Jump to viewing a specific problem. The problem is specified by a + problem name - currently the filename (minus .xml) of the problem. + Maybe this should change to a more generic tag, eg "name" given as + an attribute in <problem>. - We do the jump by (1) reading course.xml to find the first instance of <problem> with the given filename, then - (2) finding the parent element of the problem, then (3) rendering that parent element with a specific computed position - value (if it is <sequential>). + We do the jump by (1) reading course.xml to find the first + instance of <problem> with the given filename, then (2) finding + the parent element of the problem, then (3) rendering that parent + element with a specific computed position value (if it is + <sequential>). ''' # get coursename if stored coursename = multicourse_settings.get_coursename_from_request(request) # begin by getting course.xml tree - xml = content_parser.course_file(request.user,coursename) + xml = content_parser.course_file(request.user, coursename) # look for problem of given name pxml = xml.xpath('//problem[@filename="%s"]' % probname) @@ -279,12 +336,16 @@ def jump_to(request, probname=None): section = None branch = parent for k in range(4): # max depth of recursion - if branch.tag=='section': section = branch.get('name') - if branch.tag=='chapter': chapter = branch.get('name') + if branch.tag == 'section': + section = branch.get('name') + if branch.tag == 'chapter': + chapter = branch.get('name') branch = branch.getparent() position = None - if parent.tag=='sequential': - position = parent.index(pxml)+1 # position in sequence - - return index(request,course=coursename,chapter=chapter,section=section,position=position) + if parent.tag == 'sequential': + position = parent.index(pxml) + 1 # position in sequence + + return index(request, + course=coursename, chapter=chapter, + section=section, position=position)