Merge pull request #650 from MITx/feature/bridger/course_grading

Feature/bridger/course grading

common/lib/xmodule/xmodule/abtest_module.py

@@ -47,11 +47,14 @@ class ABTestModule(XModule):
 
     def get_shared_state(self):
         return json.dumps({'group': self.group})
-
+    
+    def get_children_locations(self):
+        return self.definition['data']['group_content'][self.group]
+        
     def displayable_items(self):
-        child_locations = self.definition['data']['group_content'][self.group]
-        children = [self.system.get_module(loc) for loc in child_locations]
-        return [c for c in children if c is not None]
+        # Most modules return "self" as the displayable_item. We never display ourself
+        # (which is why we don't implement get_html). We only display our children.
+        return self.get_children()
 
 
 # TODO (cpennington): Use Groups should be a first class object, rather than being
@@ -158,3 +161,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
                 group_elem.append(etree.fromstring(child.export_to_xml(resource_fs)))
 
         return xml_object
+    
+    
+    def has_dynamic_children(self):
+        return True

common/lib/xmodule/xmodule/capa_module.py

@@ -125,12 +125,6 @@ class CapaModule(XModule):
 
         self.name = only_one(dom2.xpath('/problem/@name'))
 
-        weight_string = only_one(dom2.xpath('/problem/@weight'))
-        if weight_string:
-            self.weight = float(weight_string)
-        else:
-            self.weight = None
-
         if self.rerandomize == 'never':
             self.seed = 1
         elif self.rerandomize == "per_student" and hasattr(self.system, 'id'):
@@ -279,7 +273,7 @@ class CapaModule(XModule):
 
         content = {'name': self.display_name,
                    'html': html,
-                   'weight': self.weight,
+                   'weight': self.descriptor.weight,
                    }
 
         # We using strings as truthy values, because the terminology of the
@@ -659,3 +653,12 @@ class CapaDescriptor(RawDescriptor):
             'problems/' + path[8:],
             path[8:],
         ]
+        
+    def __init__(self, *args, **kwargs):
+        super(CapaDescriptor, self).__init__(*args, **kwargs)
+        
+        weight_string = self.metadata.get('weight', None)
+        if weight_string:
+            self.weight = float(weight_string)
+        else:
+            self.weight = None

common/lib/xmodule/xmodule/course_module.py

@@ -189,6 +189,7 @@ class CourseDescriptor(SequenceDescriptor):
             for s in c.get_children():
                 if s.metadata.get('graded', False):
                     xmoduledescriptors = list(yield_descriptor_descendents(s))
+                    xmoduledescriptors.append(s)
 
                     # The xmoduledescriptors included here are only the ones that have scores.
                     section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : filter(lambda child: child.has_score, xmoduledescriptors) }

common/lib/xmodule/xmodule/x_module.py

@@ -219,13 +219,28 @@ class XModule(HTMLSnippet):
         Return module instances for all the children of this module.
         '''
         if self._loaded_children is None:
-            child_locations = self.definition.get('children', [])
+            child_locations = self.get_children_locations()
             children = [self.system.get_module(loc) for loc in child_locations]
             # get_module returns None if the current user doesn't have access
             # to the location.
             self._loaded_children = [c for c in children if c is not None]
 
         return self._loaded_children
+    
+    def get_children_locations(self):
+        '''
+        Returns the locations of each of child modules.
+        
+        Overriding this changes the behavior of get_children and
+        anything that uses get_children, such as get_display_items.
+        
+        This method will not instantiate the modules of the children
+        unless absolutely necessary, so it is cheaper to call than get_children
+        
+        These children will be the same children returned by the
+        descriptor unless descriptor.has_dynamic_children() is true.
+        '''
+        return self.definition.get('children', [])
 
     def get_display_items(self):
         '''
@@ -489,6 +504,18 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
             self,
             metadata=self.metadata
         )
+    
+    
+    def has_dynamic_children(self):
+        """
+        Returns True if this descriptor has dynamic children for a given
+        student when the module is created.
+        
+        Returns False if the children of this descriptor are the same
+        children that the module will return for any student. 
+        """
+        return False
+        
 
     # ================================= JSON PARSING ===========================
     @staticmethod

common/test/data/graded/README.md

+This is a very very simple course, useful for initial debugging of processing code.

common/test/data/graded/course.xml

+roots/2012_Fall.xml
\ No newline at end of file

common/test/data/graded/course/2012_Fall.xml

+<course>
+  <chapter url_name="GradedChapter">
+    
+    <vertical url_name="Homework1">
+      <problem url_name="H1P1">
+        <optionresponse>
+          <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+          <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+        </optionresponse>
+      </problem>
+      
+      <abtest experiment="HiddenProblem">
+        <group name="SeeProblem" portion="1">
+          
+          <problem url_name="H1P2">
+            <optionresponse>
+              <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+              <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+            </optionresponse>
+          </problem>
+          
+        </group>
+        
+        <group name="HiddenProblem" portion="0">
+          
+          <problem url_name="H1P3">
+            <optionresponse>
+              <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+              <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+            </optionresponse>
+          </problem>
+          
+        </group>
+
+      </abtest>
+    </vertical>
+    
+    
+    <videosequence url_name="Homework2">
+      <vertical url_name="Homework2Inner">
+        
+        <problem url_name="H2P1">
+          <optionresponse>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+          </optionresponse>
+        </problem>
+
+        <problem url_name="H2P2">
+          <optionresponse>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+          </optionresponse>
+        </problem>
+
+      </vertical>
+    </videosequence>
+    
+    <videosequence url_name="Homework3">
+      <vertical url_name="Homework3Inner">
+        
+        <problem url_name="H3P1">
+          <optionresponse>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+          </optionresponse>
+        </problem>
+
+        <problem url_name="H3P2">
+          <optionresponse>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+            <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+          </optionresponse>
+        </problem>
+        
+      </vertical>
+    </videosequence>
+    
+    <problem url_name="FinalQuestion">
+      <optionresponse>
+        <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+        <optioninput options="('Correct', 'Incorrect')" correct="Correct"></optioninput>
+      </optionresponse>
+    </problem>
+    
+  </chapter>
+</course>

common/test/data/graded/grading_policy.json

+{
+  "GRADER" : [
+    {
+      "type" : "Homework",
+      "min_count" : 3,
+      "drop_count" : 1,
+      "short_label" : "HW",
+      "weight" : 0.5
+    },
+    {
+      "type" : "Final",
+      "name" : "Final Question",
+      "short_label" : "Final",
+      "weight" : 0.5
+    }
+  ],
+  "GRADE_CUTOFFS" : {
+    "A" : 0.8,
+    "B" : 0.7,
+    "C" : 0.6
+  }
+}

common/test/data/graded/policies/2012_Fall.json

+{
+    "course/2012_Fall": {
+        "graceperiod": "2 days 5 hours 59 minutes 59 seconds",
+        "start": "2010-07-17T12:00",
+        "display_name": "Graded Course",
+        "graded": "true"
+    },
+	
+	
+    "vertical/Homework1": {
+        "display_name": "Homework 1", 
+        "graded": true, 
+        "format": "Homework"
+    },
+	
+	
+    "videosequence/Homework2": {
+        "display_name": "Homework 2", 
+        "graded": true, 
+        "format": "Homework"
+    },
+    
+    "problem/H2P1": {
+        "weight": 4
+    },
+    
+    "videosequence/Homework3": {
+        "display_name": "Homework 3", 
+        "graded": true, 
+        "format": "Homework"
+    },
+
+	
+    "vertical/Homework1": {
+        "display_name": "Homework 1", 
+        "graded": true, 
+        "format": "Homework"
+    },
+	
+    "problem/FinalQuestion": {
+        "display_name": "Final Question", 
+        "graded": true, 
+        "format": "Final"
+    },
+	
+    "chapter/Overview": {
+        "display_name": "Overview"
+    }
+
+}

common/test/data/graded/roots/2012_Fall.xml

+<course org="edX" course="graded" url_name="2012_Fall"/>
\ No newline at end of file

lms/djangoapps/courseware/tests/tests.py

@@ -10,8 +10,9 @@ from pprint import pprint
 from urlparse import urlsplit, urlunsplit
 
 from django.contrib.auth.models import User, Group
+from django.core.handlers.wsgi import WSGIRequest
 from django.test import TestCase
-from django.test.client import Client
+from django.test.client import Client, RequestFactory
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from mock import patch, Mock
@@ -20,7 +21,9 @@ from override_settings import override_settings
 import xmodule.modulestore.django
 
 # Need access to internal func to put users in the right group
+from courseware import grades
 from courseware.access import _course_staff_group_name
+from courseware.models import StudentModuleCache
 
 from student.models import Registration
 from xmodule.modulestore.django import modulestore
@@ -640,3 +643,133 @@ class RealCoursesLoadTestCase(PageLoader):
 
 
     # ========= TODO: check ajax interaction here too?
+
+
+@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+class TestCourseGrader(PageLoader):
+    """Check that a course gets graded properly"""
+
+    # NOTE: setUpClass() runs before override_settings takes effect, so
+    # can't do imports there without manually hacking settings.
+
+    def setUp(self):
+        xmodule.modulestore.django._MODULESTORES = {}
+        courses = modulestore().get_courses()
+
+        def find_course(course_id):
+            """Assumes the course is present"""
+            return [c for c in courses if c.id==course_id][0]
+
+        self.graded_course = find_course("edX/graded/2012_Fall")
+        
+        # create a test student
+        self.student = 'view@test.com'
+        self.password = 'foo'
+        self.create_account('u1', self.student, self.password)
+        self.activate_user(self.student)
+        self.enroll(self.graded_course)
+        
+        self.student_user = user(self.student)
+        
+        self.factory = RequestFactory()
+    
+    def check_grade_percent(self, percent):
+        
+        student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
+            self.graded_course.id, self.student_user, self.graded_course)
+        
+        fake_request = self.factory.get(reverse('progress',
+                                       kwargs={'course_id': self.graded_course.id}))
+        
+        grade_summary = grades.grade(self.student_user, fake_request, 
+                                self.graded_course, student_module_cache)
+        self.assertEqual(grade_summary['percent'], percent)
+    
+    def submit_question_answer(self, problem_url_name, responses):
+        """
+        The field names of a problem are hard to determine. This method only works
+        for the problems used in the edX/graded course, which has fields named in the
+        following form:
+        input_i4x-edX-graded-problem-H1P3_2_1
+        input_i4x-edX-graded-problem-H1P3_2_2
+        """
+        
+        
+        problem_location = "i4x://edX/graded/problem/{0}".format(problem_url_name)
+        
+        modx_url = reverse('modx_dispatch', 
+                            kwargs={
+                                'course_id' : self.graded_course.id,
+                                'location' : problem_location,
+                                'dispatch' : 'problem_check', }
+                          )
+        
+        resp = self.client.post(modx_url, {
+            'input_i4x-edX-graded-problem-{0}_2_1'.format(problem_url_name): responses[0],
+            'input_i4x-edX-graded-problem-{0}_2_2'.format(problem_url_name): responses[1],
+            })
+        print "modx_url" , modx_url, "responses" , responses
+        print "resp" , resp
+        
+        return resp
+    
+    def reset_question_answer(self, problem_url_name):
+        problem_location = "i4x://edX/graded/problem/{0}".format(problem_url_name)
+        
+        modx_url = reverse('modx_dispatch', 
+                            kwargs={
+                                'course_id' : self.graded_course.id,
+                                'location' : problem_location,
+                                'dispatch' : 'problem_reset', }
+                          )
+        
+        resp = self.client.post(modx_url)
+        return resp
+        
+     
+    def test_get_graded(self):
+        #### Check that the grader shows we have 0% in the course
+        self.check_grade_percent(0)
+        
+        
+        #### Submit the answers to a few problems as ajax calls
+        
+        # Only get half of the first problem correct
+        self.submit_question_answer('H1P1', ['Correct', 'Incorrect'])
+        self.check_grade_percent(0.06)
+        
+        # Get both parts of the first problem correct
+        self.reset_question_answer('H1P1')
+        self.submit_question_answer('H1P1', ['Correct', 'Correct'])
+        self.check_grade_percent(0.13)
+        
+        # This problem is shown in an ABTest
+        self.submit_question_answer('H1P2', ['Correct', 'Correct'])
+        self.check_grade_percent(0.25)
+        
+        # This problem is hidden in an ABTest. Getting it correct doesn't change total grade
+        self.submit_question_answer('H1P3', ['Correct', 'Correct'])
+        self.check_grade_percent(0.25)
+        
+        
+        # On the second homework, we only answer half of the questions.
+        # Then it will be dropped when homework three becomes the higher percent
+        # This problem is also weighted to be 4 points (instead of default of 2)
+        # If the problem was unweighted the percent would have been 0.38 so we 
+        # know it works.
+        self.submit_question_answer('H2P1', ['Correct', 'Correct'])
+        self.check_grade_percent(0.42)
+        
+        
+        # Third homework
+        self.submit_question_answer('H3P1', ['Correct', 'Correct'])
+        self.check_grade_percent(0.42) # Score didn't change
+        
+        self.submit_question_answer('H3P2', ['Correct', 'Correct'])
+        self.check_grade_percent(0.5) # Now homework2 dropped. Score changes
+        
+        
+        # Now we answer the final question (worth half of the grade)
+        self.submit_question_answer('FinalQuestion', ['Correct', 'Correct'])
+        self.check_grade_percent(1.0) # Hooray! We got 100%
+

lms/djangoapps/courseware/views.py

@@ -23,7 +23,7 @@ from courseware import grades
 from courseware.access import has_access
 from courseware.courses import (get_course_with_access, get_courses_by_university)
 import courseware.tabs as tabs
-from models import StudentModuleCache
+from courseware.models import StudentModuleCache
 from module_render import toc_for_course, get_module, get_instance_module
 from student.models import UserProfile
 
@@ -484,16 +484,14 @@ def progress(request, course_id, student_id=None):
 
     student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
         course_id, student, course)
-    course_module = get_module(student, request, course.location,
-                               student_module_cache, course_id)
 
-    # The course_module should be accessible, but check anyway just in case something went wrong:
-    if course_module is None:
-        raise Http404("Course does not exist")
-
-    courseware_summary = grades.progress_summary(student, course_module,
-                                                 course.grader, student_module_cache)
+    courseware_summary = grades.progress_summary(student, request, course,
+                                                 student_module_cache)
     grade_summary = grades.grade(student, request, course, student_module_cache)
+    
+    if courseware_summary is None:
+        #This means the student didn't have access to the course (which the instructor requested)
+        raise Http404
 
     context = {'course': course,
                'courseware_summary': courseware_summary,

lms/djangoapps/instructor/views.py

@@ -295,7 +295,7 @@ def gradebook(request, course_id):
     """
     course = get_course_with_access(request.user, course_id, 'staff')
 
-    enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).order_by('username')
+    enrolled_students = User.objects.filter(courseenrollment__course_id=course_id).order_by('username').select_related("profile")
 
     # TODO (vshnayder): implement pagination.
     enrolled_students = enrolled_students[:1000]   # HACK!

lms/templates/courseware/progress.html

@@ -50,7 +50,11 @@ ${progress_graph.body(grade_summary, course.grade_cutoffs, "grade-detail-graph")
               %>
 
               <h3><a href="${reverse('courseware_section', kwargs=dict(course_id=course.id, chapter=chapter['url_name'], section=section['url_name']))}">
-                ${ section['display_name'] }</a><span> ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}</span></h3>
+                ${ section['display_name'] }</a>
+                %if total > 0 or earned > 0:
+                  <span> ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}</span>
+                %endif
+              </h3>
               <p>
                 ${section['format']}