Removed the unnecessary creation of StudentModules when calling module_render's get_module.

lms/djangoapps/courseware/grades.py

@@ -3,17 +3,13 @@ import logging
 
 from django.conf import settings
 
-from module_render import get_module
+from module_render import get_module, get_instance_module
 from xmodule import graders
 from xmodule.graders import Score
 from models import StudentModule
 
 _log = logging.getLogger("mitx.courseware")
 
-
-
-
-
 def get_graded_sections(course_descriptor):
     """
     Arguments:
@@ -71,7 +67,7 @@ def fast_grade(student, request, course_graded_sections, grader, student_module_
             
             if should_grade_section:
                 scores = []
-                (section_module, _, _, _) = get_module(student, request, section_descriptor.location, student_module_cache)
+                section_module = get_module(student, request, section_descriptor.location, student_module_cache)
                 
                 for module in yield_descriptor_descendents(section_module):
                     (correct, total) = get_score(student, module, student_module_cache)
@@ -175,7 +171,7 @@ def grade_sheet(student, course, grader, student_module_cache):
             'grade_summary': grade_summary}
 
 
-def get_score(user, problem, cache):
+def get_score(user, problem, student_module_cache):
     """
     Return the score for a user on a problem
 
@@ -186,17 +182,18 @@ def get_score(user, problem, cache):
     correct = 0.0
 
     # If the ID is not in the cache, add the item
-    instance_module = cache.lookup(problem.category, problem.id)
-    if instance_module is None:
-        instance_module = StudentModule(module_type=problem.category,
-                                        module_state_key=problem.id,
-                                        student=user,
-                                        state=None,
-                                        grade=0,
-                                        max_grade=problem.max_score(),
-                                        done='i')
-        cache.append(instance_module)
-        instance_module.save()
+    instance_module = get_instance_module(user, problem, student_module_cache)
+    # instance_module = student_module_cache.lookup(problem.category, problem.id)
+    # if instance_module is None:
+    #     instance_module = StudentModule(module_type=problem.category,
+    #                                     module_state_key=problem.id,
+    #                                     student=user,
+    #                                     state=None,
+    #                                     grade=0,
+    #                                     max_grade=problem.max_score(),
+    #                                     done='i')
+    #     cache.append(instance_module)
+    #     instance_module.save()
 
     # If this problem is ungraded/ungradable, bail
     if instance_module.max_grade is None:

lms/djangoapps/courseware/management/commands/check_course.py

@@ -79,7 +79,7 @@ class Command(BaseCommand):
         # TODO (cpennington): Get coursename in a legitimate way
         course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012'
         student_module_cache = StudentModuleCache(sample_user, modulestore().get_item(course_location))
-        (course, _, _, _) = get_module(sample_user, None, course_location, student_module_cache)
+        course = get_module(sample_user, None, course_location, student_module_cache)
 
         to_run = [
             #TODO (vshnayder) : make check_rendering work (use module_render.py),

lms/djangoapps/courseware/module_render.py

@@ -45,9 +45,9 @@ def toc_for_course(user, request, course, active_chapter, active_section):
 
     chapters with name 'hidden' are skipped.
     '''
-
+    
     student_module_cache = StudentModuleCache(user, course, depth=2)
-    (course, _, _, _) = get_module(user, request, course.location, student_module_cache)
+    course = get_module(user, request, course.location, student_module_cache)
 
     chapters = list()
     for chapter in course.get_display_items():
@@ -114,25 +114,26 @@ def get_module(user, request, location, student_module_cache, position=None):
       - position              : extra information from URL for user-specified
                                 position within module
 
-    Returns:
-      - a tuple (xmodule instance, instance_module, shared_module, module category).
-        instance_module is a StudentModule specific to this module for this student,
-            or None if this is an anonymous user
-        shared_module is a StudentModule specific to all modules with the same
-            'shared_state_key' attribute, or None if the module does not elect to
-            share state
+    Returns: xmodule instance
+    
     '''
     descriptor = modulestore().get_item(location)
-
-    instance_module = student_module_cache.lookup(descriptor.category,
-                                                  descriptor.location.url())
-
-    shared_state_key = getattr(descriptor, 'shared_state_key', None)
-    if shared_state_key is not None:
-        shared_module = student_module_cache.lookup(descriptor.category,
-                                                    shared_state_key)
+    
+    #TODO Only check the cache if this module can possibly have state
+    if user.is_authenticated():
+        instance_module = student_module_cache.lookup(descriptor.category,
+                                              descriptor.location.url())
+        
+        shared_state_key = getattr(descriptor, 'shared_state_key', None)
+        if shared_state_key is not None:
+            shared_module = student_module_cache.lookup(descriptor.category,
+                                                        shared_state_key)
+        else:
+            shared_module = None
     else:
+        instance_module = None
         shared_module = None
+        
 
     instance_state = instance_module.state if instance_module is not None else None
     shared_state = shared_module.state if shared_module is not None else None
@@ -144,9 +145,8 @@ def get_module(user, request, location, student_module_cache, position=None):
                            str(user.id) + '/' + descriptor.location.url() + '/')
 
     def _get_module(location):
-        (module, _, _, _) = get_module(user, request, location,
+        return get_module(user, request, location,
                                        student_module_cache, position)
-        return module
 
     # TODO (cpennington): When modules are shared between courses, the static
     # prefix is going to have to be specific to the module, not the directory
@@ -177,30 +177,59 @@ def get_module(user, request, location, student_module_cache, position=None):
     if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff:
         module.get_html = add_histogram(module.get_html)
 
-    # If StudentModule for this instance wasn't already in the database,
-    # and this isn't a guest user, create it.
+    return module
+
+def get_instance_module(user, module, student_module_cache):
+    """
+    Returns instance_module is a StudentModule specific to this module for this student,
+        or None if this is an anonymous user
+    """
     if user.is_authenticated():
+        instance_module = student_module_cache.lookup(module.category,
+                                              module.location.url())
+        
         if not instance_module:
             instance_module = StudentModule(
                 student=user,
-                module_type=descriptor.category,
+                module_type=module.category,
                 module_state_key=module.id,
                 state=module.get_instance_state(),
                 max_grade=module.max_score())
             instance_module.save()
-            # Add to cache. The caller and the system context have references
-            # to it, so the change persists past the return
             student_module_cache.append(instance_module)
-        if not shared_module and shared_state_key is not None:
-            shared_module = StudentModule(
-                student=user,
-                module_type=descriptor.category,
-                module_state_key=shared_state_key,
-                state=module.get_shared_state())
-            shared_module.save()
-            student_module_cache.append(shared_module)
-
-    return (module, instance_module, shared_module, descriptor.category)
+            
+        return instance_module
+    else:
+        return None
+    
+def get_shared_instance_module(user, module, student_module_cache):
+    """
+    Return shared_module is a StudentModule specific to all modules with the same
+        'shared_state_key' attribute, or None if the module does not elect to
+        share state
+    """
+    if user.is_authenticated():
+        # To get the shared_state_key, we need to descriptor
+        descriptor = modulestore().get_item(module.location)
+        
+        shared_state_key = getattr(module, 'shared_state_key', None)
+        if shared_state_key is not None:
+            shared_module = student_module_cache.lookup(module.category,
+                                                        shared_state_key)
+            if not shared_module:
+                shared_module = StudentModule(
+                    student=user,
+                    module_type=descriptor.category,
+                    module_state_key=shared_state_key,
+                    state=module.get_shared_state())
+                shared_module.save()
+                student_module_cache.append(shared_module)
+        else:
+            shared_module = None
+        
+        return shared_module
+    else:
+        return None
 
 
 # TODO: TEMPORARY BYPASS OF AUTH!
@@ -218,7 +247,9 @@ def xqueue_callback(request, userid, id, dispatch):
     user = User.objects.get(id=userid)
 
     student_module_cache = StudentModuleCache(user, modulestore().get_item(id))
-    instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache)
+    instance = get_module(request.user, request, id, student_module_cache)
+    
+    instance_module = get_instance_module(request.user, instance, student_module_cache)
 
     if instance_module is None:
         log.debug("Couldn't find module '%s' for user '%s'",
@@ -264,8 +295,11 @@ def modx_dispatch(request, dispatch=None, id=None):
     # ''' (fix emacs broken parsing)
 
     student_module_cache = StudentModuleCache(request.user, modulestore().get_item(id))
-    instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache)
-
+    instance = get_module(request.user, request, id, student_module_cache)
+    
+    instance_module = get_instance_module(request.user, instance, student_module_cache)
+    shared_module = get_shared_instance_module(request.user, instance, student_module_cache)
+    
     # Don't track state for anonymous users (who don't have student modules)
     if instance_module is not None:
         oldgrade = instance_module.grade

lms/djangoapps/courseware/views.py

@@ -93,7 +93,7 @@ def gradebook(request, course_id):
 
     for student in student_objects:
         student_module_cache = StudentModuleCache(student, course)
-        course, _, _, _ = get_module(request.user, request, course.location, student_module_cache)
+        course = get_module(request.user, request, course.location, student_module_cache)
         student_info.append({
             'username': student.username,
             'id': student.id,
@@ -122,7 +122,7 @@ def profile(request, course_id, student_id=None):
     user_info = UserProfile.objects.get(user=student)
 
     student_module_cache = StudentModuleCache(request.user, course)
-    course_module, _, _, _ = get_module(request.user, request, course.location, student_module_cache)
+    course_module = get_module(request.user, request, course.location, student_module_cache)
     
     context = {'name': user_info.name,
                'username': student.username,
@@ -213,7 +213,7 @@ def index(request, course_id, chapter=None, section=None,
         if section_descriptor is not None:
             student_module_cache = StudentModuleCache(request.user,
                                                       section_descriptor)
-            module, _, _, _ = get_module(request.user, request,
+            module = get_module(request.user, request,
                                          section_descriptor.location,
                                          student_module_cache)
             context['content'] = module.get_html()