Skip to content

E
edx-platform
Commit d0011482 by Calen Pennington
Options

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

Feature/victor/per course progress
parents 81e08bf6 d23215c6
Showing with 192 additions and 45 deletions
.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 0 → 100644
# -*- 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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment