Merge pull request #472 from MITx/feature/victor/per-course-progress

Feature/victor/per course progress

.gitignore

@@ -26,3 +26,5 @@ Gemfile.lock
 lms/static/sass/*.css
 cms/static/sass/*.css
 lms/lib/comment_client/python
+nosetests.xml
+cover_html/

lms/djangoapps/courseware/grades.py

@@ -29,6 +29,8 @@ def grade(student, request, course, student_module_cache=None):
     output from the course grader, augmented with the final letter
     grade. The keys in the output are:
 
+    course: a CourseDescriptor
+
     - grade : A final letter grade.
     - percent : The final percent for the class (rounded up).
     - section_breakdown : A breakdown of each section that makes
@@ -42,7 +44,7 @@ def grade(student, request, course, student_module_cache=None):
     grading_context = course.grading_context
 
     if student_module_cache == None:
-        student_module_cache = StudentModuleCache(student, grading_context['all_descriptors'])
+        student_module_cache = StudentModuleCache(course.id, student, grading_context['all_descriptors'])
 
     totaled_scores = {}
     # This next complicated loop is just to collect the totaled_scores, which is
@@ -56,7 +58,8 @@ def grade(student, request, course, student_module_cache=None):
             should_grade_section = False
             # If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0%
             for moduledescriptor in section['xmoduledescriptors']:
-                if student_module_cache.lookup(moduledescriptor.category, moduledescriptor.location.url() ):
+                if student_module_cache.lookup(
+                        course.id, moduledescriptor.category, moduledescriptor.location.url()):
                     should_grade_section = True
                     break
 
@@ -64,10 +67,9 @@ def grade(student, request, course, student_module_cache=None):
                 scores = []
                 # TODO: We need the request to pass into here. If we could forgo that, our arguments
                 # would be simpler
-                course_id = CourseDescriptor.location_to_id(course.location)
                 section_module = get_module(student, request,
                                             section_descriptor.location, student_module_cache,
-                                            course_id)
+                                            course.id)
                 if section_module is None:
                     # student doesn't have access to this module, or something else
                     # went wrong.
@@ -76,7 +78,7 @@ def grade(student, request, course, student_module_cache=None):
                 # TODO: We may be able to speed this up by only getting a list of children IDs from section_module
                 # Then, we may not need to instatiate any problems if they are already in the database
                 for module in yield_module_descendents(section_module):
-                    (correct, total) = get_score(student, module, student_module_cache)
+                    (correct, total) = get_score(course.id, student, module, student_module_cache)
                     if correct is None and total is None:
                         continue
 
@@ -171,7 +173,9 @@ def progress_summary(student, course, grader, student_module_cache):
             graded = s.metadata.get('graded', False)
             scores = []
             for module in yield_module_descendents(s):
-                (correct, total) = get_score(student, module, student_module_cache)
+                # course is a module, not a descriptor...
+                course_id = course.descriptor.id
+                (correct, total) = get_score(course_id, student, module, student_module_cache)
                 if correct is None and total is None:
                     continue
 
@@ -200,7 +204,7 @@ def progress_summary(student, course, grader, student_module_cache):
     return chapters
 
 
-def get_score(user, problem, student_module_cache):
+def get_score(course_id, user, problem, student_module_cache):
     """
     Return the score for a user on a problem, as a tuple (correct, total).
 
@@ -215,10 +219,11 @@ def get_score(user, problem, student_module_cache):
     correct = 0.0
 
     # If the ID is not in the cache, add the item
-    instance_module = get_instance_module(user, problem, student_module_cache)
+    instance_module = get_instance_module(course_id, 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,
+    #                                     course_id=????,
     #                                     module_state_key=problem.id,
     #                                     student=user,
     #                                     state=None,

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

@@ -84,6 +84,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.cache_for_descriptor_descendents(
+        course_id,
             sample_user, modulestore().get_item(course_location))
         course = get_module(sample_user, None, course_location, student_module_cache)
 

lms/djangoapps/courseware/migrations/0003_done_grade_cache.py

@@ -9,6 +9,8 @@ class Migration(SchemaMigration):
 
     def forwards(self, orm):
 
+        # NOTE (vshnayder): This constraint has the wrong field order, so it doesn't actually
+        # do anything in sqlite.  Migration 0004 actually removes this index for sqlite.
         # Removing unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student']
         db.delete_unique('courseware_studentmodule', ['module_id', 'module_type', 'student_id'])
 

lms/djangoapps/courseware/migrations/0004_add_field_studentmodule_course_id.py

+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'StudentModule.course_id'
+        db.add_column('courseware_studentmodule', 'course_id',
+                      self.gf('django.db.models.fields.CharField')(default="", max_length=255, db_index=True),
+                      keep_default=False)
+
+        # Removing unique constraint on 'StudentModule', fields ['module_id', 'student']
+        db.delete_unique('courseware_studentmodule', ['module_id', 'student_id'])
+
+        # NOTE: manually remove this constaint (from 0001)--0003 tries, but fails for sqlite.
+        # Removing unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student']
+        if db.backend_name == "sqlite3":
+            db.delete_unique('courseware_studentmodule', ['student_id', 'module_id', 'module_type'])
+
+        # Adding unique constraint on 'StudentModule', fields ['course_id', 'module_state_key', 'student']
+        db.create_unique('courseware_studentmodule', ['student_id', 'module_id', 'course_id'])
+
+
+    def backwards(self, orm):
+        # Removing unique constraint on 'StudentModule', fields ['studnet_id', 'module_state_key', 'course_id']
+        db.delete_unique('courseware_studentmodule', ['student_id', 'module_id', 'course_id'])
+
+        # Deleting field 'StudentModule.course_id'
+        db.delete_column('courseware_studentmodule', 'course_id')
+
+        # Adding unique constraint on 'StudentModule', fields ['module_id', 'student']
+        db.create_unique('courseware_studentmodule', ['module_id', 'student_id'])
+
+        # Adding unique constraint on 'StudentModule', fields ['module_id', 'module_type', 'student']
+        db.create_unique('courseware_studentmodule', ['student_id', 'module_id', 'module_type'])
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'about': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'avatar_type': ('django.db.models.fields.CharField', [], {'default': "'n'", 'max_length': '1'}),
+            'bronze': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}),
+            'consecutive_days_visit_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'blank': 'True'}),
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'date_of_birth': ('django.db.models.fields.DateField', [], {'null': 'True', 'blank': 'True'}),
+            'display_tag_filter_strategy': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'email_isvalid': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'email_key': ('django.db.models.fields.CharField', [], {'max_length': '32', 'null': 'True'}),
+            'email_tag_filter_strategy': ('django.db.models.fields.SmallIntegerField', [], {'default': '1'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'gold': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}),
+            'gravatar': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'ignored_tags': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'interesting_tags': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'last_seen': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'location': ('django.db.models.fields.CharField', [], {'max_length': '100', 'blank': 'True'}),
+            'new_response_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'questions_per_page': ('django.db.models.fields.SmallIntegerField', [], {'default': '10'}),
+            'real_name': ('django.db.models.fields.CharField', [], {'max_length': '100', 'blank': 'True'}),
+            'reputation': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1'}),
+            'seen_response_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'show_country': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'silver': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}),
+            'status': ('django.db.models.fields.CharField', [], {'default': "'w'", 'max_length': '2'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}),
+            'website': ('django.db.models.fields.URLField', [], {'max_length': '200', 'blank': 'True'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'courseware.studentmodule': {
+            'Meta': {'unique_together': "(('course_id', 'student', 'module_state_key'),)", 'object_name': 'StudentModule'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'db_index': 'True', 'blank': 'True'}),
+            'done': ('django.db.models.fields.CharField', [], {'default': "'na'", 'max_length': '8', 'db_index': 'True'}),
+            'grade': ('django.db.models.fields.FloatField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'max_grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}),
+            'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}),
+            'module_state_key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_column': "'module_id'", 'db_index': 'True'}),
+            'module_type': ('django.db.models.fields.CharField', [], {'default': "'problem'", 'max_length': '32', 'db_index': 'True'}),
+            'state': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'student': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        }
+    }
+
+    complete_apps = ['courseware']

lms/djangoapps/courseware/models.py

@@ -22,6 +22,9 @@ from django.contrib.auth.models import User
 
 
 class StudentModule(models.Model):
+    """
+    Keeps student state for a particular module in a particular course.
+    """
     # For a homework problem, contains a JSON
     # object consisting of state
     MODULE_TYPES = (('problem', 'problem'),
@@ -37,9 +40,10 @@ class StudentModule(models.Model):
     # Filename for homeworks, etc.
     module_state_key = models.CharField(max_length=255, db_index=True, db_column='module_id')
     student = models.ForeignKey(User, db_index=True)
+    course_id = models.CharField(max_length=255, db_index=True)
 
     class Meta:
-        unique_together = (('student', 'module_state_key'),)
+        unique_together = (('student', 'module_state_key', 'course_id'),)
 
     ## Internal state of the object
     state = models.TextField(null=True, blank=True)
@@ -57,7 +61,8 @@ class StudentModule(models.Model):
     modified = models.DateTimeField(auto_now=True, db_index=True)
 
     def __unicode__(self):
-        return '/'.join([self.module_type, self.student.username, self.module_state_key, str(self.state)[:20]])
+        return '/'.join([self.course_id, self.module_type,
+                         self.student.username, self.module_state_key, str(self.state)[:20]])
 
 
 # TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors
@@ -67,20 +72,20 @@ class StudentModuleCache(object):
     """
     A cache of StudentModules for a specific student
     """
-    def __init__(self, user, descriptors, select_for_update=False):
+    def __init__(self, course_id, user, descriptors, select_for_update=False):
         '''
         Find any StudentModule objects that are needed by any descriptor
         in descriptors. Avoids making multiple queries to the database.
         Note: Only modules that have store_state = True or have shared
         state will have a StudentModule.
-        
+
         Arguments
         user: The user for which to fetch maching StudentModules
         descriptors: An array of XModuleDescriptors.
         select_for_update: Flag indicating whether the rows should be locked until end of transaction
         '''
         if user.is_authenticated():
-            module_ids = self._get_module_state_keys(descriptors) 
+            module_ids = self._get_module_state_keys(descriptors)
 
             # This works around a limitation in sqlite3 on the number of parameters
             # that can be put into a single query
@@ -89,78 +94,86 @@ class StudentModuleCache(object):
             for id_chunk in [module_ids[i:i + chunk_size] for i in xrange(0, len(module_ids), chunk_size)]:
                 if select_for_update:
                     self.cache.extend(StudentModule.objects.select_for_update().filter(
+                        course_id=course_id,
                         student=user,
                         module_state_key__in=id_chunk)
                     )
                 else:
                     self.cache.extend(StudentModule.objects.filter(
+                        course_id=course_id,
                         student=user,
                         module_state_key__in=id_chunk)
                     )
 
         else:
             self.cache = []
-    
-    
+
+
     @classmethod
-    def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True, select_for_update=False):
+    def cache_for_descriptor_descendents(cls, course_id, user, descriptor, depth=None,
+                                         descriptor_filter=lambda descriptor: True,
+                                         select_for_update=False):
         """
+        course_id: the course in the context of which we want StudentModules.
+        user: the django user for whom to load modules.
         descriptor: An XModuleDescriptor
         depth is the number of levels of descendent modules to load StudentModules for, in addition to
             the supplied descriptor. If depth is None, load all descendent StudentModules
-        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule 
+        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule
             should be cached
         select_for_update: Flag indicating whether the rows should be locked until end of transaction
         """
-        
+
         def get_child_descriptors(descriptor, depth, descriptor_filter):
             if descriptor_filter(descriptor):
                 descriptors = [descriptor]
             else:
                 descriptors = []
-            
+
             if depth is None or depth > 0:
                 new_depth = depth - 1 if depth is not None else depth
 
                 for child in descriptor.get_children():
                     descriptors.extend(get_child_descriptors(child, new_depth, descriptor_filter))
-                
+
             return descriptors
-        
-        
+
+
         descriptors = get_child_descriptors(descriptor, depth, descriptor_filter)
-        
-        return StudentModuleCache(user, descriptors, select_for_update)
-    
+
+        return StudentModuleCache(course_id, user, descriptors, select_for_update)
+
     def _get_module_state_keys(self, descriptors):
         '''
         Get a list of the state_keys needed for StudentModules
         required for this module descriptor
-        
-        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule 
+
+        descriptor_filter is a function that accepts a descriptor and return wether the StudentModule
             should be cached
         '''
         keys = []
         for descriptor in descriptors:
             if descriptor.stores_state:
                 keys.append(descriptor.location.url())
-            
+
             shared_state_key = getattr(descriptor, 'shared_state_key', None)
             if shared_state_key is not None:
                 keys.append(shared_state_key)
 
         return keys
 
-    def lookup(self, module_type, module_state_key):
+    def lookup(self, course_id, module_type, module_state_key):
         '''
-        Look for a student module with the given type and id in the cache.
+        Look for a student module with the given course_id, type, and id in the cache.
 
         cache -- list of student modules
 
         returns first found object, or None
         '''
         for o in self.cache:
-            if o.module_type == module_type and o.module_state_key == module_state_key:
+            if (o.course_id == course_id and
+                o.module_type == module_type and
+                o.module_state_key == module_state_key):
                 return o
         return None
 

lms/djangoapps/courseware/module_render.py

@@ -70,7 +70,8 @@ def toc_for_course(user, request, course, active_chapter, active_section, course
     None if this is not the case.
     '''
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2)
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+        course_id, user, course, depth=2)
     course = get_module(user, request, course.location, student_module_cache, course_id)
 
     chapters = list()
@@ -159,12 +160,13 @@ def get_module(user, request, location, student_module_cache, course_id, positio
     shared_module = None
     if user.is_authenticated():
         if descriptor.stores_state:
-            instance_module = student_module_cache.lookup(descriptor.category,
-                                                  descriptor.location.url())
+            instance_module = student_module_cache.lookup(
+                course_id, 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_module = student_module_cache.lookup(course_id,
+                                                        descriptor.category,
                                                         shared_state_key)
 
 
@@ -241,7 +243,7 @@ def get_module(user, request, location, student_module_cache, course_id, positio
 
     return module
 
-def get_instance_module(user, module, student_module_cache):
+def get_instance_module(course_id, 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
@@ -252,11 +254,12 @@ def get_instance_module(user, module, student_module_cache):
                           + str(module.id) + " which does not store state.")
             return None
 
-        instance_module = student_module_cache.lookup(module.category,
-                                              module.location.url())
+        instance_module = student_module_cache.lookup(
+            course_id, module.category, module.location.url())
 
         if not instance_module:
             instance_module = StudentModule(
+                course_id=course_id,
                 student=user,
                 module_type=module.category,
                 module_state_key=module.id,
@@ -285,6 +288,7 @@ def get_shared_instance_module(course_id, user, module, student_module_cache):
                                                         shared_state_key)
             if not shared_module:
                 shared_module = StudentModule(
+                    course_id=course_id,
                     student=user,
                     module_type=descriptor.category,
                     module_state_key=shared_state_key,
@@ -317,14 +321,14 @@ def xqueue_callback(request, course_id, userid, id, dispatch):
     # Retrieve target StudentModule
     user = User.objects.get(id=userid)
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(course_id,
         user, modulestore().get_instance(course_id, id), depth=0, select_for_update=True)
     instance = get_module(user, request, id, student_module_cache, course_id)
     if instance is None:
         log.debug("No module {0} for user {1}--access denied?".format(id, user))
         raise Http404
 
-    instance_module = get_instance_module(user, instance, student_module_cache)
+    instance_module = get_instance_module(course_id, user, instance, student_module_cache)
 
     if instance_module is None:
         log.debug("Couldn't find instance of module '%s' for user '%s'", id, user)
@@ -387,7 +391,7 @@ def modx_dispatch(request, dispatch, location, course_id):
                     return HttpResponse(json.dumps({'success': file_too_big_msg}))
             p[fileinput_id] = inputfiles
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(course_id,
         request.user, modulestore().get_instance(course_id, location))
 
     instance = get_module(request.user, request, location, student_module_cache, course_id)
@@ -397,7 +401,7 @@ def modx_dispatch(request, dispatch, location, course_id):
         log.debug("No module {0} for user {1}--access denied?".format(location, user))
         raise Http404
 
-    instance_module = get_instance_module(request.user, instance, student_module_cache)
+    instance_module = get_instance_module(course_id, request.user, instance, student_module_cache)
     shared_module = get_shared_instance_module(course_id, request.user, instance, student_module_cache)
 
     # Don't track state for anonymous users (who don't have student modules)

lms/djangoapps/courseware/views.py

@@ -149,8 +149,7 @@ def index(request, course_id, chapter=None, section=None,
             section_descriptor = get_section(course, chapter, section)
             if section_descriptor is not None:
                 student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
-                                                          request.user,
-                                                          section_descriptor)
+                    course_id, request.user, section_descriptor)
                 module = get_module(request.user, request,
                                     section_descriptor.location,
                                     student_module_cache, course_id)
@@ -310,7 +309,8 @@ def progress(request, course_id, student_id=None):
             raise Http404
         student = User.objects.get(id=int(student_id))
 
-    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course)
+    student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+        course_id, request.user, course)
     course_module = get_module(request.user, request, course.location,
                                student_module_cache, course_id)