Make user profiles work again after the switch to an XMLModuleStore. Staff user…

Make user profiles work again after the switch to an XMLModuleStore. Staff user histograms are still broken

common/lib/xmodule/abtest_module.py

@@ -88,8 +88,5 @@ class Module(XModule):
     def get_children(self):
         return [self.module_from_xml(child) for child in self._xml_children()]
 
-    def rendered_children(self):
-        return [self.render_function(child) for child in self._xml_children()]
-
     def get_html(self):
         return '\n'.join(child.get_html() for child in self.get_children())

common/lib/xmodule/capa_module.py

@@ -229,7 +229,13 @@ class CapaModule(XModule):
         # TODO: Should be: self.filename=only_one(dom2.xpath('/problem/@filename'))
         self.filename = "problems/" + only_one(dom2.xpath('/problem/@filename')) + ".xml"
         self.name = only_one(dom2.xpath('/problem/@name'))
-        self.weight = only_one(dom2.xpath('/problem/@weight'))
+
+        weight_string = only_one(dom2.xpath('/problem/@weight'))
+        if weight_string:
+            self.weight = float(weight_string)
+        else:
+            self.weight = 1
+
         if self.rerandomize == 'never':
             seed = 1
         elif self.rerandomize == "per_student" and hasattr(system, 'id'):

common/lib/xmodule/video_module.py

@@ -50,7 +50,6 @@ class VideoModule(XModule):
             'id': self.location.html_id(),
             'position': self.position,
             'name': self.name,
-            'annotations': self.annotations,
         })
 
     def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs):
@@ -65,9 +64,6 @@ class VideoModule(XModule):
             if 'position' in state:
                 self.position = int(float(state['position']))
 
-        self.annotations = [(e.get("name"), self.render_function(e)) \
-                      for e in xmltree]
-
 
 class VideoDescriptor(RawDescriptor):
     module_class = VideoModule

common/lib/xmodule/x_module.py

@@ -83,6 +83,7 @@ class XModule(object):
         self.id = self.location.url()
         self.name = self.location.name
         self.display_name = kwargs.get('display_name', '')
+        self.type = self.location.category
         self._loaded_children = None
 
     def get_name(self):

lms/djangoapps/courseware/grades.py

@@ -15,11 +15,9 @@ from courseware.course_settings import GRADER  # This won't work.
 
 """
 
-from lxml import etree
 import random
 import imp
 import logging
-import sys
 import types
 
 from django.conf import settings
@@ -28,11 +26,10 @@ from courseware import global_course_settings
 from xmodule import graders
 from xmodule.graders import Score
 from models import StudentModule
-import courseware.content_parser as content_parser
-import xmodule
 
 _log = logging.getLogger("mitx.courseware")
 
+
 class Settings(object):
     def __init__(self):
         # update this dict from global settings (but only for ALL_CAPS settings)
@@ -40,7 +37,6 @@ class Settings(object):
             if setting == setting.upper():
                 setattr(self, setting, getattr(global_course_settings, setting))
 
-        
         data_dir = settings.DATA_DIR
 
         fp = None
@@ -65,9 +61,7 @@ class Settings(object):
 course_settings = Settings()
 
 
-
-
-def grade_sheet(student,coursename=None):
+def grade_sheet(student, course, student_module_cache):
     """
     This pulls a summary of all problems in the course. It returns a dictionary with two datastructures:
 
@@ -76,130 +70,105 @@ def grade_sheet(student,coursename=None):
     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.
-    """
-    dom=content_parser.course_file(student,coursename)
-    course = dom.xpath('//course/@name')[0]
-    xmlChapters = dom.xpath('//course[@name=$course]/chapter', course=course)
-
-    responses = StudentModule.objects.filter(student=student)
-    response_by_id = {}
-    for response in responses:
-        response_by_id[response.module_state_key] = response
-
 
+    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
+    """
     totaled_scores = {}
-    chapters=[]
-    for c in xmlChapters:
+    chapters = []
+    for c in course.get_children():
         sections = []
-        chname=c.get('name')
-        
-        
-        for s in dom.xpath('//course[@name=$course]/chapter[@name=$chname]/section', 
-                           course=course, chname=chname):
-            problems=dom.xpath('//course[@name=$course]/chapter[@name=$chname]/section[@name=$section]//problem', 
-                           course=course, chname=chname, section=s.get('name'))
-
-            graded = True if s.get('graded') == "true" else False
-            scores=[]
-            if len(problems)>0:
-                for p in problems:
-                    (correct,total) = get_score(student, p, response_by_id, coursename=coursename)
+        for s in c.get_children():
+            def yield_descendents(module):
+                yield module
+                for child in module.get_display_items():
+                    for module in yield_descendents(child):
+                        yield module
+
+            graded = getattr(s, 'graded', False)
+            scores = []
+            for module in yield_descendents(s):
+                (correct, total) = get_score(student, module, student_module_cache)
 
                 if settings.GENERATE_PROFILE_SCORES:
                     if total > 1:
-                            correct = random.randrange( max(total-2, 1) , total + 1 )
+                        correct = random.randrange(max(total - 2, 1), total + 1)
                     else:
                         correct = total
 
                 if not total > 0:
                     #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, p.get("name")) )
 
-                section_total, graded_total = graders.aggregate_scores(scores, s.get("name"))
+                if correct is not None and total is not None:
+                    scores.append(Score(correct, total, graded, module.display_name))
+
+            section_total, graded_total = graders.aggregate_scores(scores, s.display_name)
             #Add the graded total to totaled_scores
-                format = s.get('format', "")
-                subtitle = s.get('subtitle', format)
+            format = getattr(s, 'format', "")
+            subtitle = getattr(s, 'subtitle', format)
             if format and graded_total[1] > 0:
                 format_scores = totaled_scores.get(format, [])
-                    format_scores.append( graded_total )
-                    totaled_scores[ format ] = format_scores
-
-                section_score={'section':s.get("name"),
-                       'scores':scores,
-                       'section_total' : section_total,
-                       'format' : format,
-                       'subtitle' : subtitle,
-                       'due' : s.get("due") or "",
-                       'graded' : graded,
-                       }
-                sections.append(section_score)
-
-        chapters.append({'course':course,
-                         'chapter' : c.get("name"),
-                         'sections' : sections,})
-    
+                format_scores.append(graded_total)
+                totaled_scores[format] = format_scores
+
+            sections.append({
+                'section': s.display_name,
+                'scores': scores,
+                'section_total': section_total,
+                'format': format,
+                'subtitle': subtitle,
+                'due': getattr(s, "due", ""),
+                'graded': graded,
+            })
+
+        chapters.append({'course': course.display_name,
+                         'chapter': c.display_name,
+                         'sections': sections})
 
     grader = course_settings.GRADER
     grade_summary = grader.grade(totaled_scores)
 
-    return {'courseware_summary' : chapters,
-            'grade_summary' : grade_summary}
+    return {'courseware_summary': chapters,
+            'grade_summary': grade_summary}
 
-def get_score(user, problem, cache, coursename=None):
+
+def get_score(user, problem, cache):
     """
     Return the score for a user on a problem
 
     user: a Student object
-    problem: the xml for the problem
-    cache: a dictionary mapping module_state_key tuples to instantiated StudentModules
-           module_state_key is either the problem_id, or a key used by the problem
-           to share state across instances
+    problem: an XModule
+    cache: A StudentModuleCache
     """
-    ## HACK: assumes max score is fixed per problem
-    module_type = problem.tag
-    module_class = xmodule.get_module_class(module_type)
-    module_id = problem.get('id')
-    module_state_key = problem.get(module_class.state_key, module_id)
     correct = 0.0
 
     # If the ID is not in the cache, add the item
-    if module_state_key not in cache:
-        module = StudentModule(module_type='problem',  # TODO: Move into StudentModule.__init__?
-                               module_state_key=id,
+    instance_module = cache.lookup(problem.type, problem.id)
+    if instance_module is None:
+        instance_module = StudentModule(module_type=problem.type,
+                                        module_state_key=problem.id,
                                         student=user,
                                         state=None,
                                         grade=0,
-                               max_grade=None,
+                                        max_grade=problem.max_score(),
                                         done='i')
-        cache[module_id] = module
+        cache.append(instance_module)
+        instance_module.save()
 
-    # Grab the # correct from cache
-    if id in cache:
-        response = cache[id]
-        if response.grade != None:
-            correct = float(response.grade)
+    # If this problem is ungraded/ungradable, bail
+    if instance_module.max_grade is None:
+        return (None, None)
 
-    # Grab max grade from cache, or if it doesn't exist, compute and save to DB
-    if id in cache and response.max_grade is not None:
-        total = response.max_grade
-    else:
-        ## HACK 1: We shouldn't specifically reference capa_module
-        ## HACK 2: Backwards-compatibility: This should be written when a grade is saved, and removed from the system
-        # TODO: These are no longer correct params for I4xSystem -- figure out what this code
-        # does, clean it up.
-        # from module_render import I4xSystem
-        # system = I4xSystem(None, None, None, coursename=coursename)
-        # total=float(xmodule.capa_module.Module(system, etree.tostring(problem), "id").max_score())
-        # response.max_grade = total
-        # response.save()
-        total = 1
-        # For a temporary fix, we just assume a problem is worth 1 point if we haven't seen it before. This is totally incorrect
+    correct = instance_module.grade if instance_module.grade is not None else 0
+    total = instance_module.max_grade
 
+    if correct is not None and total is not None:
         #Now we re-weight the problem, if specified
-    weight = problem.get("weight", None)
-    if weight:
-        weight = float(weight) 
+        weight = getattr(problem, 'weight', 1)
+        if weight != 1:
             correct = correct * weight / total
             total = weight
 

lms/djangoapps/courseware/models.py

@@ -75,8 +75,16 @@ class StudentModuleCache(object):
         '''
         if user.is_authenticated():
             module_ids = self._get_module_state_keys(descriptor, depth)
-            self.cache = list(StudentModule.objects.filter(student=user,
-                                                           module_state_key__in=module_ids))
+
+            # This works around a limitation in sqlite3 on the number of parameters
+            # that can be put into a single query
+            self.cache = []
+            chunk_size = 500
+            for id_chunk in [module_ids[i:i+chunk_size] for i in xrange(0, len(module_ids), chunk_size)]:
+                self.cache.extend(StudentModule.objects.filter(
+                    student=user,
+                    module_state_key__in=id_chunk)
+                )
         else:
             self.cache = []
 

lms/djangoapps/courseware/module_render.py

@@ -29,7 +29,7 @@ class I4xSystem(object):
     and user, or other environment-specific info.
     '''
     def __init__(self, ajax_url, track_function, render_function,
-                 get_module, render_template, request=None,
+                 get_module, render_template, user=None,
                  filestore=None):
         '''
         Create a closure around the system environment.
@@ -47,7 +47,7 @@ class I4xSystem(object):
                           and 'type'.
         render_template - a function that takes (template_file, context), and returns
                           rendered html.
-        request - the request in progress
+        user - The user to base the seed off of for this request
         filestore - A filestore ojbect.  Defaults to an instance of OSFS based at
                     settings.DATA_DIR.
         '''
@@ -59,7 +59,7 @@ class I4xSystem(object):
         self.render_template = render_template
         self.exception404 = Http404
         self.DEBUG = settings.DEBUG
-        self.id = request.user.id if request is not None else 0
+        self.seed = user.id if user is not None else 0
 
     def get(self, attr):
         '''	provide uniform access to attributes (like etree).'''
@@ -234,13 +234,13 @@ def get_module(user, request, location, student_module_cache, position=None):
 
     system = I4xSystem(track_function=make_track_function(request),
                        render_function=lambda xml: render_x_module(
-                           user, request, xml, student_module_cache, position),
+                           user, xml, student_module_cache, position),
                        render_template=render_to_string,
                        ajax_url=ajax_url,
-                       request=request,
                        # TODO (cpennington): Figure out how to share info between systems
                        filestore=descriptor.system.resources_fs,
                        get_module=_get_module,
+                       user=user,
                        )
     # pass position specified in URL to module through I4xSystem
     system.set('position', position)
@@ -272,7 +272,7 @@ def get_module(user, request, location, student_module_cache, position=None):
     return (module, instance_module, shared_module, descriptor.type)
 
 
-def render_x_module(user, request, module_xml, student_module_cache, position=None):
+def render_x_module(user, module, student_module_cache, position=None):
     ''' Generic module for extensions. This renders to HTML.
 
     modules include sequential, vertical, problem, video, html
@@ -282,8 +282,7 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No
     Arguments:
 
       - user                  : current django User
-      - request               : current django HTTPrequest
-      - module_xml            : lxml etree of xml subtree for the current module
+      - module                : 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
 
@@ -296,7 +295,7 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No
         return {"content": ""}
 
     (instance, _, _, module_type) = get_module(
-        user, request, module_xml, student_module_cache, position)
+        user, module_xml, student_module_cache, position)
 
     content = instance.get_html()
 

lms/djangoapps/courseware/views.py

@@ -20,17 +20,16 @@ from module_render import render_x_module, toc_for_course, get_module, get_secti
 from models import StudentModuleCache
 from student.models import UserProfile
 from multicourse import multicourse_settings
+from keystore.django import keystore
 
-import courseware.content_parser as content_parser
-
-import courseware.grades as grades
+from courseware import grades, content_parser
 
 log = logging.getLogger("mitx.courseware")
 
 etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False,
-                                         remove_comments = True))
+                                         remove_comments=True))
 
-template_imports={'urllib':urllib}
+template_imports = {'urllib': urllib}
 
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 def gradebook(request):
@@ -49,6 +48,7 @@ def gradebook(request):
 
     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):
@@ -60,11 +60,14 @@ def profile(request, student_id=None):
     else:
         if 'course_admin' not in content_parser.user_groups(request.user):
             raise Http404
-        student = User.objects.get( id = int(student_id))
+        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)
 
     coursename = multicourse_settings.get_coursename_from_request(request)
+    course_location = multicourse_settings.get_course_location(coursename)
+    student_module_cache = StudentModuleCache(request.user, keystore().get_item(course_location))
+    course, _, _, _ = get_module(request.user, request, course_location, student_module_cache)
 
     context = {'name': user_info.name,
                'username': student.username,
@@ -74,7 +77,7 @@ def profile(request, student_id=None):
                'format_url_params': content_parser.format_url_params,
                'csrf': csrf(request)['csrf_token']
                }
-    context.update(grades.grade_sheet(student, coursename))
+    context.update(grades.grade_sheet(student, course, student_module_cache))
 
     return render_to_response('profile.html', context)
 
@@ -127,7 +130,7 @@ def render_section(request, section):
     student_module_cache = StudentModuleCache(request.user, dom)
 
     try:
-        module = render_x_module(user, request, dom, student_module_cache)
+        module = render_x_module(user, dom, student_module_cache)
     except:
         log.exception("Unable to load module")
         context.update({

lms/envs/common.py

@@ -132,7 +132,7 @@ COURSE_DEFAULT = '6.002_Spring_2012'
 COURSE_SETTINGS =  {'6.002_Spring_2012': {'number' : '6.002x',
                                           'title'  :  'Circuits and Electronics',
                                           'xmlpath': '6002x/',
-                                          'location': 'i4x://edx/6002xs12/course/6_002_Spring_2012',
+                                          'location': 'i4x://edx/6002xs12/course/6.002_Spring_2012',
                                           }
                     }
 

lms/templates/video.html

@@ -13,11 +13,3 @@
     </article>
   </div>
 </div>
-
-<ol class="video-mod">
-% for t in annotations:
-  <li id="video-${annotations.index(t)}">
-    ${t[1]['content']}
-  </li>
-% endfor
-</ol>