From 8f17e6ae9ed76fa75b3caf867b65ccb632cb6870 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 15 Feb 2013 01:30:14 -0500 Subject: [PATCH 01/39] First pass at implementing problem history. --- .../0006_create_student_module_history.py | 109 ++++++++++++++++++ .../0007_allow_null_version_in_history.py | 100 ++++++++++++++++ lms/djangoapps/courseware/models.py | 33 +++++- lms/djangoapps/courseware/views.py | 49 +++++++- lms/envs/common.py | 4 + lms/static/sass/shared/_modal.scss | 2 + .../courseware/submission_history.html | 13 +++ lms/templates/courseware/xqa_interface.html | 21 ++++ lms/templates/staff_problem_info.html | 25 +++- lms/urls.py | 9 +- 10 files changed, 360 insertions(+), 5 deletions(-) create mode 100644 lms/djangoapps/courseware/migrations/0006_create_student_module_history.py create mode 100644 lms/djangoapps/courseware/migrations/0007_allow_null_version_in_history.py create mode 100644 lms/templates/courseware/submission_history.html diff --git a/lms/djangoapps/courseware/migrations/0006_create_student_module_history.py b/lms/djangoapps/courseware/migrations/0006_create_student_module_history.py new file mode 100644 index 0000000000..8bf40cfb20 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0006_create_student_module_history.py @@ -0,0 +1,109 @@ +# -*- 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 model 'StudentModuleHistory' + db.create_table('courseware_studentmodulehistory', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('student_module', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['courseware.StudentModule'])), + ('version', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('created', self.gf('django.db.models.fields.DateTimeField')(db_index=True)), + ('state', self.gf('django.db.models.fields.TextField')(null=True, blank=True)), + ('grade', self.gf('django.db.models.fields.FloatField')(null=True, blank=True)), + ('max_grade', self.gf('django.db.models.fields.FloatField')(null=True, blank=True)), + )) + db.send_create_signal('courseware', ['StudentModuleHistory']) + + + def backwards(self, orm): + # Deleting model 'StudentModuleHistory' + db.delete_table('courseware_studentmodulehistory') + + + 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'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': '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'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + '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'}) + }, + '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.offlinecomputedgrade': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'OfflineComputedGrade'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'gradeset': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'courseware.offlinecomputedgradelog': { + 'Meta': {'ordering': "['-created']", 'object_name': 'OfflineComputedGradeLog'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'nstudents': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'seconds': ('django.db.models.fields.IntegerField', [], {'default': '0'}) + }, + 'courseware.studentmodule': { + 'Meta': {'unique_together': "(('student', 'module_state_key', 'course_id'),)", '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']"}) + }, + 'courseware.studentmodulehistory': { + 'Meta': {'object_name': 'StudentModuleHistory'}, + 'created': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}), + 'grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'max_grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'state': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'student_module': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['courseware.StudentModule']"}), + 'version': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}) + } + } + + complete_apps = ['courseware'] \ No newline at end of file diff --git a/lms/djangoapps/courseware/migrations/0007_allow_null_version_in_history.py b/lms/djangoapps/courseware/migrations/0007_allow_null_version_in_history.py new file mode 100644 index 0000000000..f6204294c4 --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0007_allow_null_version_in_history.py @@ -0,0 +1,100 @@ +# -*- 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): + + # Changing field 'StudentModuleHistory.version' + db.alter_column('courseware_studentmodulehistory', 'version', self.gf('django.db.models.fields.CharField')(max_length=255, null=True)) + + def backwards(self, orm): + + # User chose to not deal with backwards NULL issues for 'StudentModuleHistory.version' + raise RuntimeError("Cannot reverse this migration. 'StudentModuleHistory.version' and its values cannot be restored.") + + 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'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': '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'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + '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'}) + }, + '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.offlinecomputedgrade': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'OfflineComputedGrade'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'gradeset': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'courseware.offlinecomputedgradelog': { + 'Meta': {'ordering': "['-created']", 'object_name': 'OfflineComputedGradeLog'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'nstudents': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'seconds': ('django.db.models.fields.IntegerField', [], {'default': '0'}) + }, + 'courseware.studentmodule': { + 'Meta': {'unique_together': "(('student', 'module_state_key', 'course_id'),)", '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']"}) + }, + 'courseware.studentmodulehistory': { + 'Meta': {'object_name': 'StudentModuleHistory'}, + 'created': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}), + 'grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'max_grade': ('django.db.models.fields.FloatField', [], {'null': 'True', 'blank': 'True'}), + 'state': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'student_module': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['courseware.StudentModule']"}), + 'version': ('django.db.models.fields.CharField', [], {'default': 'None', 'max_length': '255', 'null': 'True', 'db_index': 'True'}) + } + } + + complete_apps = ['courseware'] \ No newline at end of file diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index ac9bde77cd..58ede38d58 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -12,8 +12,10 @@ file and check it in at the same time as your model changes. To do that, ASSUMPTIONS: modules have unique IDs, even across different module_types """ -from django.db import models from django.contrib.auth.models import User +from django.db import models +from django.db.models.signals import post_save +from django.dispatch import receiver class StudentModule(models.Model): """ @@ -60,6 +62,35 @@ class StudentModule(models.Model): self.student.username, self.module_state_key, str(self.state)[:20]]) +class StudentModuleHistory(models.Model): + """Keeps a complete history of state changes for a given XModule for a given + Student. Right now, we restrict this to problems so that the table doesn't + explode in size.""" + + class Meta: + get_latest_by = "created" + + student_module = models.ForeignKey(StudentModule, db_index=True) + version = models.CharField(max_length=255, null=True, blank=True, db_index=True) + + # This should be populated from the modified field in StudentModule + created = models.DateTimeField(db_index=True) + state = models.TextField(null=True, blank=True) + grade = models.FloatField(null=True, blank=True) + max_grade = models.FloatField(null=True, blank=True) + + @receiver(post_save, sender=StudentModule) + def save_history(sender, instance, **kwargs): + if instance.module_type == 'problem': + history_entry = StudentModuleHistory(student_module=instance, + version=None, + created=instance.modified, + state=instance.state, + grade=instance.grade, + max_grade=instance.max_grade) + history_entry.save() + + # TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index fb351e1c01..b6fb31fc25 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -5,10 +5,11 @@ from functools import partial from django.conf import settings from django.core.context_processors import csrf +from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required -from django.http import Http404, HttpResponseRedirect +from django.http import Http404, HttpResponse, HttpResponseRedirect from django.shortcuts import redirect from mitxmako.shortcuts import render_to_response, render_to_string #from django.views.decorators.csrf import ensure_csrf_cookie @@ -20,7 +21,7 @@ from courseware.access import has_access from courseware.courses import (get_courses, get_course_with_access, get_courses_by_university, sort_by_announcement) import courseware.tabs as tabs -from courseware.models import StudentModule, StudentModuleCache +from courseware.models import StudentModule, StudentModuleCache, StudentModuleHistory from module_render import toc_for_course, get_module, get_instance_module, get_module_for_descriptor from django_comment_client.utils import get_discussion_title @@ -606,3 +607,47 @@ def progress(request, course_id, student_id=None): context.update() return render_to_response('courseware/progress.html', context) + + +@login_required +def submission_history(request, course_id, student_username, location): + """Render an HTML fragment (meant for inclusion elsewhere) that renders a + history of all state changes made by this user for this problem location. + Right now this only works for problems because that's all + StudentModuleHistory records. + """ + # Make sure our has_access check uses the course_id, eh? or is ourself + course = get_course_with_access(request.user, course_id, 'load') + staff_access = has_access(request.user, course, 'staff') + + if (student_username != request.user.username) and (not staff_access): + raise PermissionDenied + + try: + student = User.objects.get(username=student_username) + student_module = StudentModule.objects.get(course_id=course_id, + module_state_key=location, + student_id=student.id) + except User.DoesNotExist: + return HttpResponse("User {0} does not exist.".format(student_username)) + except StudentModule.DoesNotExist: + return HttpResponse("{0} has never accessed problem {1}" + .format(student_username, location)) + + history_entries = StudentModuleHistory.objects \ + .filter(student_module=student_module).order_by('-created') + + # If no history records exist, let's force a save to get history started. + if not history_entries: + student_module.save() + history_entries = StudentModuleHistory.objects \ + .filter(student_module=student_module).order_by('-created') + + context = { + 'history_entries': history_entries, + 'username': student.username, + 'location': location, + 'course_id': course_id + } + + return render_to_response('courseware/submission_history.html', context) diff --git a/lms/envs/common.py b/lms/envs/common.py index eb8c9989f0..371b7d9dcd 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -84,6 +84,10 @@ MITX_FEATURES = { # Flip to True when the YouTube iframe API breaks (again) 'USE_YOUTUBE_OBJECT_API': False, + + # Give a UI to show a student's submission history in a problem by the + # Staff Debug tool. + 'ENABLE_STUDENT_HISTORY_VIEW': True } # Used for A/B testing diff --git a/lms/static/sass/shared/_modal.scss b/lms/static/sass/shared/_modal.scss index 1685487b89..bfa803fee2 100644 --- a/lms/static/sass/shared/_modal.scss +++ b/lms/static/sass/shared/_modal.scss @@ -57,6 +57,8 @@ border: 1px solid rgba(0, 0, 0, 0.9); @include box-shadow(inset 0 1px 0 0 rgba(255, 255, 255, 0.7)); overflow: hidden; + padding-left: 10px; + padding-right: 10px; padding-bottom: 30px; position: relative; z-index: 2; diff --git a/lms/templates/courseware/submission_history.html b/lms/templates/courseware/submission_history.html new file mode 100644 index 0000000000..cd14e9b2ab --- /dev/null +++ b/lms/templates/courseware/submission_history.html @@ -0,0 +1,13 @@ +<% import json %> +

${username} > ${course_id} > ${location}

+ +% for i, entry in enumerate(history_entries): +
+
+#${len(history_entries) - i}: ${entry.created} EST
+Score: ${entry.grade} / ${entry.max_grade} +
+${json.dumps(json.loads(entry.state), indent=2, sort_keys=True) | h}
+
+
+% endfor diff --git a/lms/templates/courseware/xqa_interface.html b/lms/templates/courseware/xqa_interface.html index c314cc7fb0..89ec77b31f 100644 --- a/lms/templates/courseware/xqa_interface.html +++ b/lms/templates/courseware/xqa_interface.html @@ -5,6 +5,27 @@ function setup_debug(element_id, edit_link, staff_context){ $('#' + element_id + '_trig').leanModal(); $('#' + element_id + '_xqa_log').leanModal(); $('#' + element_id + '_xqa_form').submit(function () {sendlog(element_id, edit_link, staff_context);}); + + $("#" + element_id + "_history_trig").leanModal(); + + $('#' + element_id + '_history_form').submit( + function () { + var username = $("#" + element_id + "_history_student_username").val(); + var location = $("#" + element_id + "_history_location").val(); + + // This is a ridiculous way to get the course_id, but I'm not sure + // how to do it sensibly from within the staff debug code. + // staff_problem_info.html is rendered through a wrapper to get_html + // that's injected by the code that adds the histogram -- it's all + // kinda bizarre, and it remains awkward to simply ask "what course + // is this problem being shown in the context of." + var path_parts = window.location.pathname.split('/'); + var course_id = path_parts[2] + "/" + path_parts[3] + "/" + path_parts[4]; + $("#" + element_id + "_history_text").load('/courses/' + course_id + + "/submission_history/" + username + "/" + location); + return false; + } + ); } function sendlog(element_id, edit_link, staff_context){ diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index 9324445dd1..f427709bce 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -1,3 +1,4 @@ +## The JS for this is defined in xqa_interface.html ${module_content} %if location.category in ['problem','video','html']: % if edit_link: @@ -13,6 +14,10 @@ ${module_content} % endif
Staff Debug Info
+% if settings.MITX_FEATURES.get('ENABLE_STUDENT_HISTORY_VIEW'): +
Submission history
+% endif + -
+ + +
+ + """) + + # Create the problem + problem = LoncapaProblem(xml_str, '1', system=test_system) + + # Render the HTML + rendered_html = etree.XML(problem.get_html()) + + # Expect that the script element has been removed from the rendered HTML + script_element = rendered_html.find('script') + self.assertEqual(None, script_element) + + def test_render_response_xml(self): + # Generate some XML for a string response + kwargs = {'question_text': "Test question", + 'explanation_text': "Test explanation", + 'answer': 'Test answer', + 'hints': [('test prompt', 'test_hint', 'test hint text')]} + xml_str = StringResponseXMLFactory().build_xml(**kwargs) + + # Mock out the template renderer + test_system.render_template = mock.Mock() + test_system.render_template.return_value = "
Input Template Render
" + + # Create the problem and render the HTML + problem = LoncapaProblem(xml_str, '1', system=test_system) + rendered_html = etree.XML(problem.get_html()) + + # Expect problem has been turned into a
+ self.assertEqual(rendered_html.tag, "div") + + # Expect question text is in a

child + question_element = rendered_html.find("p") + self.assertEqual(question_element.text, "Test question") + + # Expect that the response has been turned into a + response_element = rendered_html.find("span") + self.assertEqual(response_element.tag, "span") + + # Expect that the response + # that contains a

for the textline + textline_element = response_element.find("div") + self.assertEqual(textline_element.text, 'Input Template Render') + + # Expect a child
for the solution + # with the rendered template + solution_element = rendered_html.find("div") + self.assertEqual(solution_element.text, 'Input Template Render') + + # Expect that the template renderer was called with the correct + # arguments, once for the textline input and once for + # the solution + expected_textline_context = {'status': 'unsubmitted', + 'value': '', + 'preprocessor': None, + 'msg': '', + 'inline': False, + 'hidden': False, + 'do_math': False, + 'id': '1_2_1', + 'size': None} + + expected_solution_context = {'id': '1_solution_1'} + + expected_calls = [mock.call('textline.html', expected_textline_context), + mock.call('solutionspan.html', expected_solution_context)] + + self.assertEqual(test_system.render_template.call_args_list, + expected_calls) + + + def test_render_response_with_overall_msg(self): + # CustomResponse script that sets an overall_message + script=textwrap.dedent(""" + def check_func(*args): + return {'overall_message': 'Test message', + 'input_list': [ {'ok': True, 'msg': '' } ] } + """) + + # Generate some XML for a CustomResponse + kwargs = {'script':script, 'cfn': 'check_func'} + xml_str = CustomResponseXMLFactory().build_xml(**kwargs) + + # Create the problem and render the html + problem = LoncapaProblem(xml_str, '1', system=test_system) + + # Grade the problem + correctmap = problem.grade_answers({'1_2_1': 'test'}) + + # Render the html + rendered_html = etree.XML(problem.get_html()) + + + # Expect that there is a
within the response
+ # with css class response_message + msg_div_element = rendered_html.find(".//div[@class='response_message']") + self.assertEqual(msg_div_element.tag, "div") + self.assertEqual(msg_div_element.get('class'), "response_message") + + + def test_substitute_python_vars(self): + # Generate some XML with Python variables defined in a script + # and used later as attributes + xml_str = textwrap.dedent(""" + + + + + """) + + # Create the problem and render the HTML + problem = LoncapaProblem(xml_str, '1', system=test_system) + rendered_html = etree.XML(problem.get_html()) + + # Expect that the variable $test has been replaced with its value + span_element = rendered_html.find('span') + self.assertEqual(span_element.get('attr'), "TEST") + + def _create_test_file(self, path, content_str): + test_fp = test_system.filestore.open(path, "w") + test_fp.write(content_str) + test_fp.close() + + self.addCleanup(lambda: os.remove(test_fp.name)) From 952716af13fa3365bf1952c929354787cc315f7d Mon Sep 17 00:00:00 2001 From: Will Daly Date: Fri, 1 Mar 2013 09:02:04 -0500 Subject: [PATCH 16/39] Added clean-up code for message HTML. If overall message is a parseable XHTML tree, it is inserted as a tree rather than text. --- common/lib/capa/capa/responsetypes.py | 47 ++++++++++++------- .../lib/capa/capa/tests/test_html_render.py | 7 ++- .../lib/capa/capa/tests/test_responsetypes.py | 4 +- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 897f922e93..698ec41a0a 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -201,7 +201,17 @@ class LoncapaResponse(object): if response_msg: response_msg_div = etree.SubElement(tree, 'div') response_msg_div.set("class", "response_message") - response_msg_div.text = response_msg + + # If the response message can be represented as an XHTML tree, + # create the tree and append it to the message
+ try: + response_tree = etree.XML(response_msg) + response_msg_div.append(response_tree) + + # Otherwise, assume that the message is text (not XHTML) + # and insert it as the text of the message
+ except: + response_msg_div.text = response_msg return tree @@ -1069,20 +1079,7 @@ def sympy_check2(): if 'ok' in ret: correct = ['correct'] * len(idset) if ret['ok'] else ['incorrect'] * len(idset) msg = ret.get('msg', None) - - if 1: - # try to clean up message html - msg = '' + msg + '' - msg = msg.replace('<', '<') - #msg = msg.replace('<','<') - msg = etree.tostring(fromstring_bs(msg, convertEntities=None), - pretty_print=True) - #msg = etree.tostring(fromstring_bs(msg),pretty_print=True) - msg = msg.replace(' ', '') - #msg = re.sub('(.*)','\\1',msg,flags=re.M|re.DOTALL) # python 2.7 - msg = re.sub('(?ms)(.*)', '\\1', msg) - - messages[0] = msg + messages[0] = self.clean_message_html(msg) # Another kind of dictionary the check function can return has @@ -1101,7 +1098,8 @@ def sympy_check2(): messages = [] for input_dict in input_list: correct.append('correct' if input_dict['ok'] else 'incorrect') - messages.append(input_dict['msg'] if 'msg' in input_dict else None) + msg = self.clean_message_html(input_dict['msg']) if 'msg' in input_dict else None + messages.append(msg) # Otherwise, we do not recognize the dictionary # Raise an exception @@ -1117,13 +1115,30 @@ def sympy_check2(): # build map giving "correct"ness of the answer(s) correct_map = CorrectMap() + + overall_message = self.clean_message_html(overall_message) correct_map.set_overall_message(overall_message) + for k in range(len(idset)): npoints = self.maxpoints[idset[k]] if correct[k] == 'correct' else 0 correct_map.set(idset[k], correct[k], msg=messages[k], npoints=npoints) return correct_map + def clean_message_html(self, msg): + # try to clean up message html + msg = '' + msg + '' + msg = msg.replace('<', '<') + #msg = msg.replace('<','<') + msg = etree.tostring(fromstring_bs(msg, convertEntities=None), + pretty_print=True) + #msg = etree.tostring(fromstring_bs(msg),pretty_print=True) + msg = msg.replace(' ', '') + #msg = re.sub('(.*)','\\1',msg,flags=re.M|re.DOTALL) # python 2.7 + msg = re.sub('(?ms)(.*)', '\\1', msg) + + return msg.strip() + def get_answers(self): ''' Give correct answer expected for this response. diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index aa5312aa14..257e63b611 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -135,7 +135,7 @@ class CapaHtmlRenderTest(unittest.TestCase): # CustomResponse script that sets an overall_message script=textwrap.dedent(""" def check_func(*args): - return {'overall_message': 'Test message', + return {'overall_message': '

Test message

', 'input_list': [ {'ok': True, 'msg': '' } ] } """) @@ -159,6 +159,11 @@ class CapaHtmlRenderTest(unittest.TestCase): self.assertEqual(msg_div_element.tag, "div") self.assertEqual(msg_div_element.get('class'), "response_message") + # Expect that the
contains our message (as part of the XML tree) + msg_p_element = msg_div_element.find('p') + self.assertEqual(msg_p_element.tag, "p") + self.assertEqual(msg_p_element.text, "Test message") + def test_substitute_python_vars(self): # Generate some XML with Python variables defined in a script diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 451e6ed14b..538ee6fe50 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -712,7 +712,7 @@ class CustomResponseTest(ResponseTest): msg = correct_map.get_msg('1_2_1') self.assertEqual(correctness, 'correct') - self.assertEqual(msg, "Message text\n") + self.assertEqual(msg, "Message text") # Incorrect answer input_dict = {'1_2_1': '0'} @@ -722,7 +722,7 @@ class CustomResponseTest(ResponseTest): msg = correct_map.get_msg('1_2_1') self.assertEqual(correctness, 'incorrect') - self.assertEqual(msg, "Message text\n") + self.assertEqual(msg, "Message text") def test_function_code_multiple_input_no_msg(self): From 9ac8ef542f4ced63fb7b8cf1769d0f6f8036c486 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Fri, 1 Mar 2013 09:35:37 -0500 Subject: [PATCH 17/39] Overall message HTMl now correctly renders when multiple HTML tags are passed as the message (even when there's no root tag) --- common/lib/capa/capa/responsetypes.py | 37 ++++++++++++------- .../lib/capa/capa/tests/test_html_render.py | 12 ++++-- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 698ec41a0a..6b57895013 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -199,19 +199,7 @@ class LoncapaResponse(object): # Add a
for the message at the end of the response if response_msg: - response_msg_div = etree.SubElement(tree, 'div') - response_msg_div.set("class", "response_message") - - # If the response message can be represented as an XHTML tree, - # create the tree and append it to the message
- try: - response_tree = etree.XML(response_msg) - response_msg_div.append(response_tree) - - # Otherwise, assume that the message is text (not XHTML) - # and insert it as the text of the message
- except: - response_msg_div.text = response_msg + tree.append(self._render_response_msg_html(response_msg)) return tree @@ -337,6 +325,29 @@ class LoncapaResponse(object): def __unicode__(self): return u'LoncapaProblem Response %s' % self.xml.tag + def _render_response_msg_html(self, response_msg): + """ Render a
for a message that applies to the entire response. + + *response_msg* is a string, which may contain XHTML markup + + Returns an etree element representing the response message
""" + # First try wrapping the text in a
and parsing + # it as an XHTML tree + try: + response_msg_div = etree.XML('
%s
' % str(response_msg)) + + # If we can't do that, create the
and set the message + # as the text of the
+ except: + response_msg_div = etree.Element('div') + response_msg_div.text = str(response_msg) + + + # Set the css class of the message
+ response_msg_div.set("class", "response_message") + + return response_msg_div + #----------------------------------------------------------------------------- diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index 257e63b611..64f031ea59 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -135,7 +135,8 @@ class CapaHtmlRenderTest(unittest.TestCase): # CustomResponse script that sets an overall_message script=textwrap.dedent(""" def check_func(*args): - return {'overall_message': '

Test message

', + msg = '

Test message 1

Test message 2

' + return {'overall_message': msg, 'input_list': [ {'ok': True, 'msg': '' } ] } """) @@ -160,9 +161,12 @@ class CapaHtmlRenderTest(unittest.TestCase): self.assertEqual(msg_div_element.get('class'), "response_message") # Expect that the
contains our message (as part of the XML tree) - msg_p_element = msg_div_element.find('p') - self.assertEqual(msg_p_element.tag, "p") - self.assertEqual(msg_p_element.text, "Test message") + msg_p_elements = msg_div_element.findall('p') + self.assertEqual(msg_p_elements[0].tag, "p") + self.assertEqual(msg_p_elements[0].text, "Test message 1") + + self.assertEqual(msg_p_elements[1].tag, "p") + self.assertEqual(msg_p_elements[1].text, "Test message 2") def test_substitute_python_vars(self): From c3da73ed1eea6be8afb24d5505190b2d4fa65ce9 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Fri, 1 Mar 2013 10:57:57 -0500 Subject: [PATCH 18/39] Changed title of custom response doc. Added custom response doc to index --- .../course_data_formats/custom_response.rst | 142 ++++++++++++++++++ doc/public/index.rst | 1 + 2 files changed, 143 insertions(+) create mode 100644 doc/public/course_data_formats/custom_response.rst diff --git a/doc/public/course_data_formats/custom_response.rst b/doc/public/course_data_formats/custom_response.rst new file mode 100644 index 0000000000..1ca8214915 --- /dev/null +++ b/doc/public/course_data_formats/custom_response.rst @@ -0,0 +1,142 @@ +#################################### +CustomResponse XML and Python Script +#################################### + +This document explains how to write a CustomResponse problem. CustomResponse +problems execute Python script to check student answers and provide hints. + +There are two general ways to create a CustomResponse problem: + + +***************** +Answer tag format +***************** +One format puts the Python code in an ```` tag: + +.. code-block:: xml + + +

What is the sum of 2 and 3?

+ + + + + + + # Python script goes here + + + + +The Python script interacts with these variables in the global context: + * ``answers``: An ordered list of answers the student provided. + For example, if the student answered ``6``, then ``answers[0]`` would + equal ``6``. + * ``expect``: The value of the ``expect`` attribute of ```` + (if provided). + * ``correct``: An ordered list of strings indicating whether the + student answered the question correctly. Valid values are + ``"correct"``, ``"incorrect"``, and ``"unknown"``. You can set these + values in the script. + * ``messages``: An ordered list of message strings that will be displayed + beneath each input. You can use this to provide hints to users. + For example ``messages[0] = "The capital of California is Sacramento"`` + would display that message beneath the first input of the response. + * ``overall_message``: A string that will be displayed beneath the + entire problem. You can use this to provide a hint that applies + to the entire problem rather than a particular input. + +Example of a checking script: + +.. code-block:: python + + if answers[0] == expect: + correct[0] = 'correct' + overall_message = 'Good job!' + else: + correct[0] = 'incorrect' + messages[0] = 'This answer is incorrect' + overall_message = 'Please try again' + +**Important**: Python is picky about indentation. Within the ```` tag, +you must begin your script with no indentation. + +***************** +Script tag format +***************** +The other way to create a CustomResponse is to put a "checking function" +in a `` + + + +**Important**: Python is picky about indentation. Within the `` From 88a30cb7330a079238b00ea68b6ae619dd3e20da Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 4 Mar 2013 13:21:19 -0500 Subject: [PATCH 33/39] Fixes in response to Victor's comments. --- lms/djangoapps/courseware/models.py | 4 +++- lms/djangoapps/courseware/views.py | 3 ++- lms/templates/courseware/submission_history.html | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 58ede38d58..3ad850f066 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -67,6 +67,8 @@ class StudentModuleHistory(models.Model): Student. Right now, we restrict this to problems so that the table doesn't explode in size.""" + HISTORY_SAVING_TYPES = {'problem'} + class Meta: get_latest_by = "created" @@ -81,7 +83,7 @@ class StudentModuleHistory(models.Model): @receiver(post_save, sender=StudentModule) def save_history(sender, instance, **kwargs): - if instance.module_type == 'problem': + if instance.module_type in StudentModuleHistory.HISTORY_SAVING_TYPES: history_entry = StudentModuleHistory(student_module=instance, version=None, created=instance.modified, diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b6fb31fc25..d2da893776 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -616,10 +616,11 @@ def submission_history(request, course_id, student_username, location): Right now this only works for problems because that's all StudentModuleHistory records. """ - # Make sure our has_access check uses the course_id, eh? or is ourself course = get_course_with_access(request.user, course_id, 'load') staff_access = has_access(request.user, course, 'staff') + # Permission Denied if they don't have staff access and are trying to see + # somebody else's submission history. if (student_username != request.user.username) and (not staff_access): raise PermissionDenied diff --git a/lms/templates/courseware/submission_history.html b/lms/templates/courseware/submission_history.html index cd14e9b2ab..3d78cbd4f0 100644 --- a/lms/templates/courseware/submission_history.html +++ b/lms/templates/courseware/submission_history.html @@ -4,7 +4,7 @@ % for i, entry in enumerate(history_entries):
-#${len(history_entries) - i}: ${entry.created} EST
+#${len(history_entries) - i}: ${entry.created} UTC
Score: ${entry.grade} / ${entry.max_grade} 
 ${json.dumps(json.loads(entry.state), indent=2, sort_keys=True) | h}

From a81e9a673c45d58088387f932d929d20c710c444 Mon Sep 17 00:00:00 2001
From: Chris Dodge 
Date: Mon, 4 Mar 2013 14:59:45 -0500
Subject: [PATCH 34/39] additional courseware view optimizations. Do a 'depth'
 fetch on the selected section so that it does a more efficient set of queries
 to the database. Also, in the CachingDescriptorSystem, if we have a 'cache
 miss', when we do the actual fetch (which creates a new 'system'), keep that
 fetched data around in our own collection, in case it is queried again

---
 common/lib/xmodule/xmodule/modulestore/mongo.py | 5 ++++-
 lms/djangoapps/courseware/views.py              | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index 8068129559..b46c29b2bc 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -64,7 +64,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         location = Location(location)
         json_data = self.module_data.get(location)
         if json_data is None:
-            return self.modulestore.get_item(location)
+            module = self.modulestore.get_item(location)
+            if module is not None:
+                self.module_data.update(module.system.module_data)
+            return module
         else:
             # load the module and apply the inherited metadata
             try:
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 8b48572818..a9e8298db7 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -306,6 +306,10 @@ def index(request, course_id, chapter=None, section=None,
                 # Specifically asked-for section doesn't exist
                 raise Http404
 
+            # cdodge: this looks silly, but let's refetch the section_descriptor with depth=None
+            # which will prefetch the children more efficiently than doing a recursive load
+            section_descriptor = modulestore().get_instance(course.id, section_descriptor.location, depth=None)
+
             # Load all descendants of the section, because we're going to display its
             # html, which in general will need all of its children
             section_module_cache = StudentModuleCache.cache_for_descriptor_descendents(

From 01a1bf6b6f3a3adb07a249e767c3d8a6b56d03d0 Mon Sep 17 00:00:00 2001
From: David Ormsbee 
Date: Mon, 4 Mar 2013 15:19:24 -0500
Subject: [PATCH 35/39] It's not UTC time, it's whatever the local settings are
 set to (New York in prod)

---
 lms/templates/courseware/submission_history.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lms/templates/courseware/submission_history.html b/lms/templates/courseware/submission_history.html
index 3d78cbd4f0..683c61c5a0 100644
--- a/lms/templates/courseware/submission_history.html
+++ b/lms/templates/courseware/submission_history.html
@@ -4,7 +4,7 @@
 % for i, entry in enumerate(history_entries):
 

 

-#${len(history_entries) - i}: ${entry.created} UTC

+#${len(history_entries) - i}: ${entry.created} (${TIME_ZONE} time)

 Score: ${entry.grade} / ${entry.max_grade}
 
 ${json.dumps(json.loads(entry.state), indent=2, sort_keys=True) | h}

From 020e1e94fbf7b6977b8d34c872f9a09a635910ae Mon Sep 17 00:00:00 2001
From: Don Mitchell 
Date: Mon, 4 Mar 2013 15:38:47 -0500
Subject: [PATCH 36/39] Move side effecting of definition for grader to the
 course_module and don't make caller responsible.

---
 cms/djangoapps/models/settings/course_grading.py |  2 --
 common/lib/xmodule/xmodule/course_module.py      | 13 ++++---------
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py
index f1f68b5f5b..3d0b8f78af 100644
--- a/cms/djangoapps/models/settings/course_grading.py
+++ b/cms/djangoapps/models/settings/course_grading.py
@@ -118,8 +118,6 @@ class CourseGradingModel(object):
             descriptor.raw_grader[index] = grader
         else:
             descriptor.raw_grader.append(grader)
-        # make definition notice the update
-        descriptor.raw_grader = descriptor.raw_grader
 
         get_modulestore(course_location).update_item(course_location, descriptor.definition['data'])
 
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 2ed780fcae..86ae673ae8 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -127,6 +127,7 @@ class CourseDescriptor(SequenceDescriptor):
         # NOTE (THK): This is a last-minute addition for Fall 2012 launch to dynamically
         #   disable the syllabus content for courses that do not provide a syllabus
         self.syllabus_present = self.system.resources_fs.exists(path('syllabus'))
+        self._grading_policy = {}
         self.set_grading_policy(self.definition['data'].get('grading_policy', None))
 
         self.test_center_exams = []
@@ -196,11 +197,9 @@ class CourseDescriptor(SequenceDescriptor):
         grading_policy.update(course_policy)
 
         # Here is where we should parse any configurations, so that we can fail early
-        grading_policy['RAW_GRADER'] = grading_policy['GRADER']  # used for cms access
-        grading_policy['GRADER'] = grader_from_conf(grading_policy['GRADER'])
-        self._grading_policy = grading_policy
-
-
+        # Use setters so that side effecting to .definitions works
+        self.raw_grader = grading_policy['GRADER']  # used for cms access
+        self.grade_cutoffs = grading_policy['GRADE_CUTOFFS']
 
     @classmethod
     def read_grading_policy(cls, paths, system):
@@ -317,10 +316,6 @@ class CourseDescriptor(SequenceDescriptor):
         if isinstance(value, time.struct_time):
             self.metadata['enrollment_end'] = stringify_time(value)
 
-    @property
-    def grader(self):
-        return self._grading_policy['GRADER']
-
     @property
     def raw_grader(self):
         return self._grading_policy['RAW_GRADER']

From 0c4a52567e036fd27d1142ae29fc98ab96d5e1a0 Mon Sep 17 00:00:00 2001
From: Don Mitchell 
Date: Mon, 4 Mar 2013 16:13:29 -0500
Subject: [PATCH 37/39] Provide convenience converter of the grading policy but
 don't save it on the object.

---
 common/lib/xmodule/xmodule/course_module.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 86ae673ae8..72196f92a2 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -316,6 +316,10 @@ class CourseDescriptor(SequenceDescriptor):
         if isinstance(value, time.struct_time):
             self.metadata['enrollment_end'] = stringify_time(value)
 
+    @property
+    def grader(self):
+        return grader_from_conf(self.raw_grader)
+
     @property
     def raw_grader(self):
         return self._grading_policy['RAW_GRADER']

From 94db91fceff1edcd65b39f1f9bbfb1cc1bd25982 Mon Sep 17 00:00:00 2001
From: John Hess 
Date: Mon, 4 Mar 2013 18:19:24 -0500
Subject: [PATCH 38/39] Cast cursor responses as lists.  MySQL returns them as
 tuples.

Removed print statemetns
Moved import call to top of file
---
 lms/djangoapps/dashboard/views.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lms/djangoapps/dashboard/views.py b/lms/djangoapps/dashboard/views.py
index e74d462432..266e769db5 100644
--- a/lms/djangoapps/dashboard/views.py
+++ b/lms/djangoapps/dashboard/views.py
@@ -3,6 +3,7 @@ import json
 from datetime import datetime
 from django.http import Http404
 from mitxmako.shortcuts import render_to_response
+from django.db import connection
 
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
 from django.contrib.auth.models import User
@@ -12,16 +13,18 @@ def dictfetchall(cursor):
     '''Returns a list of all rows from a cursor as a column: result dict.
     Borrowed from Django documentation'''
     desc = cursor.description
-    table=[]
+    table = []
     table.append([col[0] for col in desc])
-    table = table + cursor.fetchall()
-    print "Table: " + str(table)
+    
+    # ensure response from db is a list, not a tuple (which is returned
+    # by MySQL backed django instances)
+    rows_from_cursor=cursor.fetchall()
+    table = table + [list(row) for row in rows_from_cursor]
     return table
 
 def SQL_query_to_list(cursor, query_string):
     cursor.execute(query_string)
     raw_result=dictfetchall(cursor)
-    print raw_result
     return raw_result
 
 def dashboard(request):
@@ -50,7 +53,6 @@ def dashboard(request):
     results["scalars"]["Total Enrollments Across All Courses"]=CourseEnrollment.objects.count()
 
     # establish a direct connection to the database (for executing raw SQL)
-    from django.db import connection
     cursor = connection.cursor()
 
     # define the queries that will generate our user-facing tables

From 6ce3493f00a5c7395923273c50d0094cc2732d4a Mon Sep 17 00:00:00 2001
From: Chris Dodge 
Date: Mon, 4 Mar 2013 19:59:18 -0500
Subject: [PATCH 39/39] add comment

---
 common/lib/xmodule/xmodule/modulestore/mongo.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index b46c29b2bc..e2a4524188 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -66,6 +66,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         if json_data is None:
             module = self.modulestore.get_item(location)
             if module is not None:
+                # update our own cache after going to the DB to get cache miss
                 self.module_data.update(module.system.module_data)
             return module
         else: