From c3a6ad2f8c45788bc97e48b77a720b1bea282b39 Mon Sep 17 00:00:00 2001 From: Bridger Maxwell Date: Fri, 27 Jan 2012 16:12:56 -0500 Subject: [PATCH] Module ids are unique even across module types. Fix bugs where other fields were being used for an id. --HG-- branch : profiledev --- courseware/content_parser.py | 53 +++++++++++++++++-------------- courseware/models.py | 2 +- courseware/module_render.py | 9 ++++-- courseware/modules/capa_module.py | 6 ++-- courseware/views.py | 23 +++++++++----- 5 files changed, 55 insertions(+), 38 deletions(-) diff --git a/courseware/content_parser.py b/courseware/content_parser.py index ca70282caa..e577324568 100644 --- a/courseware/content_parser.py +++ b/courseware/content_parser.py @@ -82,40 +82,47 @@ def id_tag(course): if elem.get('id'): pass elif elem.get(default_ids[elem.tag]): - new_id = elem.get(default_ids[elem.tag]) # Convert to alphanumeric - new_id = "".join([a for a in new_id if a.isalnum()]) + new_id = elem.get(default_ids[elem.tag]) + new_id = "".join([a for a in new_id if a.isalnum()]) # Convert to alphanumeric + # Without this, a conflict may occur between an hmtl or youtube id + new_id = default_ids[elem.tag] + new_id elem.set('id', new_id) else: elem.set('id', fasthash(etree.tostring(elem))) -def due_tag(element, parent_due_date=None): - ''' This call is to pass down due dates. If an element has a due date, - all of the elements children will inherit this due date (unless the element - has a due date of its own). This is called recursively''' - - if (parent_due_date == None): #This is the entry call. Select all due elements - all_due_elements = element.xpath("//*[@due]") - for due_element in all_due_elements: - due_date = due_element.get('due') - for child_element in due_element: - due_tag(child_element, due_date) +def propogate_downward_tag(element, attribute_name, parent_attribute = None): + ''' This call is to pass down an attribute to all children. If an element + has this attribute, it will be "inherited" by all of its children. If a + child (A) already has that attribute, A will keep the same attribute and + all of A's children will inherit A's attribute. This is a recursive call.''' + + if (parent_attribute == None): #This is the entry call. Select all due elements + all_attributed_elements = element.xpath("//*[@" + attribute_name +"]") + for attributed_element in all_attributed_elements: + attribute_value = attributed_element.get(attribute_name) + for child_element in attributed_element: + propogate_downward_tag(child_element, attribute_name, attribute_value) else: - #The hack below is because we would get _ContentOnlyELements from the - #iterator that can't have due dates set. We can't find API for it - if not element.get('due') and type(element) == etree._Element: - element.set('due', parent_due_date) - due_date = parent_due_date - else: - due_date = element.get('due') + '''The hack below is because we would get _ContentOnlyELements from the + iterator that can't have due dates set. We can't find API for it. If we + ever have an element which subclasses BaseElement, we will not tag it''' + if not element.get(attribute_name) and type(element) == etree._Element: + element.set(attribute_name, parent_attribute) - for child_element in element: - due_tag(child_element, due_date) + for child_element in element: + propogate_downward_tag(child_element, attribute_name, parent_attribute) + else: + #This element would have already been found by Xpath, so we return + #for now and trust that this element will get its turn to propogate + #to its children later. + return def course_file(user): # TODO: Cache. tree = etree.parse(settings.DATA_DIR+UserProfile.objects.get(user=user).courseware) id_tag(tree) - due_tag(tree) + propogate_downward_tag(tree, "due") + propogate_downward_tag(tree, "graded") return tree def module_xml(coursefile, module, id_tag, module_id): diff --git a/courseware/models.py b/courseware/models.py index cee61f336b..f334208597 100644 --- a/courseware/models.py +++ b/courseware/models.py @@ -50,7 +50,7 @@ class StudentModule(models.Model): module_id = models.CharField(max_length=255) # Filename for homeworks, etc. student = models.ForeignKey(User) class Meta: - unique_together = (('student', 'module_id', 'module_type'),) + unique_together = (('student', 'module_id'),) ## Internal state of the object state = models.TextField(null=True, blank=True) diff --git a/courseware/module_render.py b/courseware/module_render.py index 4c969447a1..b4e08ab17b 100644 --- a/courseware/module_render.py +++ b/courseware/module_render.py @@ -33,10 +33,14 @@ from django.conf import settings import courseware.content_parser as content_parser import sys +import logging from lxml import etree import uuid + +log = logging.getLogger("mitx.courseware") + ## TODO: Add registration mechanism modx_modules={'problem':courseware.modules.capa_module.LoncapaModule, 'video':courseware.modules.video_module.VideoModule, @@ -64,11 +68,10 @@ def make_track_function(request): def modx_dispatch(request, module=None, dispatch=None, id=None): ''' Generic view for extensions. ''' # Grab the student information for the module from the database - s = StudentModule.objects.filter(module_type=module, - student=request.user, + s = StudentModule.objects.filter(student=request.user, module_id=id) if len(s) == 0: - print "ls404", module, request.user, id + log.debug("Couldnt find module for user and id " + str(module) + " " + str(request.user) + " "+ str(id)) raise Http404 s=s[0] diff --git a/courseware/modules/capa_module.py b/courseware/modules/capa_module.py index 591f9c7516..51cf6f823a 100644 --- a/courseware/modules/capa_module.py +++ b/courseware/modules/capa_module.py @@ -40,13 +40,13 @@ class LoncapaModule(XModule): def get_html(self): return render_to_string('problem_ajax.html', - {'id':self.filename, + {'id':self.item_id, 'ajax_url':self.ajax_url, }) def get_init_js(self): return render_to_string('problem.js', - {'id':self.filename, + {'id':self.item_id, 'ajax_url':self.ajax_url, }) @@ -94,7 +94,7 @@ class LoncapaModule(XModule): html=render_to_string('problem.html', {'problem' : content, - 'id' : self.filename, + 'id' : self.item_id, 'check_button' : check_button, 'reset_button' : reset_button, 'save_button' : save_button, diff --git a/courseware/views.py b/courseware/views.py index 5f5680b663..ccfda0f678 100644 --- a/courseware/views.py +++ b/courseware/views.py @@ -20,6 +20,7 @@ from lxml import etree from auth.models import UserProfile from models import StudentModule from module_render import * # TODO: Clean up +from module_render import modx_dispatch import courseware.content_parser as content_parser log = logging.getLogger("mitx.courseware") @@ -45,6 +46,11 @@ def profile(request): chapters = dom.xpath('//course[@name=$course]/chapter', course=course) responses=StudentModule.objects.filter(student=request.user) + response_by_id = {} + for response in responses: + response_by_id[response.module_id] = response + + print response_by_id for c in chapters: chname=c.get('name') @@ -55,14 +61,15 @@ def profile(request): scores=[] if len(problems)>0: for p in problems: - id = p.get('filename') + id = p.get('id') correct = 0 - for response in responses: - if response.module_id == id: - if response.grade!=None: - correct=response.grade - else: - correct=0 + if id in response_by_id: + response = response_by_id[id] + if response.grade!=None: + correct=response.grade + else: + print "Couldn't find id " + id + total=courseware.modules.capa_module.LoncapaModule(etree.tostring(p), "id").max_score() # TODO: Add state. Not useful now, but maybe someday problems will have randomized max scores? scores.append((int(correct),total)) score={'course':course, @@ -71,7 +78,7 @@ def profile(request): 'scores':scores, } hw.append(score) - + user_info=UserProfile.objects.get(user=request.user) context={'name':user_info.name,