Merge branch 'master' into cpennington/cms-editing

common/lib/xmodule/capa_module.py

@@ -11,6 +11,7 @@ from datetime import timedelta
 from lxml import etree
 
 from x_module import XModule, XModuleDescriptor
+from progress import Progress
 from capa.capa_problem import LoncapaProblem
 from capa.responsetypes import StudentInputError
 
@@ -79,24 +80,41 @@ class Module(XModule):
     def get_xml_tags(c):
         return ["problem"]
 
+
     def get_state(self):
         state = self.lcp.get_state()
         state['attempts'] = self.attempts
         return json.dumps(state)
 
+
     def get_score(self):
         return self.lcp.get_score()
 
+
     def max_score(self):
         return self.lcp.get_max_score()
 
+
+    def get_progress(self):
+        ''' For now, just return score / max_score
+        '''
+        d = self.get_score()
+        score = d['score']
+        total = d['total']
+        return Progress(score, total)
+
+
     def get_html(self):
         return self.system.render_template('problem_ajax.html', {
             'id': self.item_id,
             'ajax_url': self.ajax_url,
         })
 
+
     def get_problem_html(self, encapsulate=True):
+        '''Return html for the problem.  Adds check, reset, save buttons
+        as necessary based on the problem config and state.'''
+
         html = self.lcp.get_html()
         content = {'name': self.name,
                    'html': html,
@@ -109,7 +127,7 @@ class Module(XModule):
         reset_button = True
         save_button = True
 
-        # If we're after deadline, or user has exhuasted attempts,
+        # If we're after deadline, or user has exhausted attempts,
         # question is read-only.
         if self.closed():
             check_button = False
@@ -154,11 +172,13 @@ class Module(XModule):
                    'attempts_used': self.attempts,
                    'attempts_allowed': self.max_attempts,
                    'explain': explain,
+                   'progress': self.get_progress(),
                    }
 
         html = self.system.render_template('problem.html', context)
         if encapsulate:
-            html = '<div id="problem_{id}" class="problem" data-url="{ajax_url}">'.format(id=self.item_id, ajax_url=self.ajax_url) + html + "</div>"
+            html = '<div id="problem_{id}" class="problem" data-url="{ajax_url}">'.format(
+                id=self.item_id, ajax_url=self.ajax_url) + html + "</div>"
 
         return html
 
@@ -170,7 +190,8 @@ class Module(XModule):
 
         dom2 = etree.fromstring(xml)
 
-        self.explanation = "problems/" + only_one(dom2.xpath('/problem/@explain'), default="closed")
+        self.explanation = "problems/" + only_one(dom2.xpath('/problem/@explain'),
+                                                  default="closed")
         # TODO: Should be converted to: self.explanation=only_one(dom2.xpath('/problem/@explain'), default="closed")
         self.explain_available = only_one(dom2.xpath('/problem/@explain_available'))
 
@@ -190,19 +211,19 @@ class Module(XModule):
             self.grace_period = None
             self.close_date = self.display_due_date
 
-        self.max_attempts =only_one(dom2.xpath('/problem/@attempts'))
-        if len(self.max_attempts)>0:
-            self.max_attempts =int(self.max_attempts)
+        self.max_attempts = only_one(dom2.xpath('/problem/@attempts'))
+        if len(self.max_attempts) > 0:
+            self.max_attempts = int(self.max_attempts)
         else:
-            self.max_attempts =None
+            self.max_attempts = None
 
-        self.show_answer =only_one(dom2.xpath('/problem/@showanswer'))
+        self.show_answer = only_one(dom2.xpath('/problem/@showanswer'))
 
-        if self.show_answer =="":
-            self.show_answer ="closed"
+        if self.show_answer == "":
+            self.show_answer = "closed"
 
-        self.rerandomize =only_one(dom2.xpath('/problem/@rerandomize'))
-        if self.rerandomize =="" or self.rerandomize=="always" or self.rerandomize=="true":
+        self.rerandomize = only_one(dom2.xpath('/problem/@rerandomize'))
+        if self.rerandomize == "" or self.rerandomize=="always" or self.rerandomize=="true":
             self.rerandomize="always"
         elif self.rerandomize=="false" or self.rerandomize=="per_student":
             self.rerandomize="per_student"
@@ -253,23 +274,33 @@ class Module(XModule):
 
     def handle_ajax(self, dispatch, get):
         '''
-        This is called by courseware.module_render, to handle an AJAX call.  "get" is request.POST
+        This is called by courseware.module_render, to handle an AJAX call.
+        "get" is request.POST.
+
+        Returns a json dictionary:
+        { 'progress_changed' : True/False,
+          'progress' : 'none'/'in_progress'/'done',
+          <other request-specific values here > }
         '''
-        if dispatch=='problem_get':
-            response = self.get_problem(get)
-        elif False: #self.close_date >
-            return json.dumps({"error":"Past due date"})
-        elif dispatch=='problem_check':
-            response = self.check_problem(get)
-        elif dispatch=='problem_reset':
-            response = self.reset_problem(get)
-        elif dispatch=='problem_save':
-            response = self.save_problem(get)
-        elif dispatch=='problem_show':
-            response = self.get_answer(get)
-        else:
-            return "Error"
-        return response
+        handlers = {
+            'problem_get': self.get_problem,
+            'problem_check': self.check_problem,
+            'problem_reset': self.reset_problem,
+            'problem_save': self.save_problem,
+            'problem_show': self.get_answer,
+            }
+
+        if dispatch not in handlers:
+            return 'Error'
+
+        before = self.get_progress()
+        d = handlers[dispatch](get)
+        after = self.get_progress()
+        d.update({
+            'progress_changed' : after != before,
+            'progress' : after.ternary_str(),
+            })
+        return json.dumps(d, cls=ComplexEncoder)
 
     def closed(self):
         ''' Is the student still allowed to submit answers? '''
@@ -283,24 +314,22 @@ class Module(XModule):
 
     def answer_available(self):
         ''' Is the user allowed to see an answer?
-        TODO: simplify. 
         '''
         if self.show_answer == '':
             return False
+
         if self.show_answer == "never":
             return False
-        if self.show_answer == 'attempted' and self.attempts == 0:
-            return False
-        if self.show_answer == 'attempted' and self.attempts > 0:
-            return True
-        if self.show_answer == 'answered' and self.lcp.done:
-            return True
-        if self.show_answer == 'answered' and not self.lcp.done:
-            return False
-        if self.show_answer == 'closed' and self.closed():
-            return True
-        if self.show_answer == 'closed' and not self.closed():
-            return False
+
+        if self.show_answer == 'attempted':
+            return self.attempts > 0
+
+        if self.show_answer == 'answered':
+            return self.lcp.done
+
+        if self.show_answer == 'closed':
+            return self.closed()
+
         if self.show_answer == 'always':
             return True
         raise self.system.exception404 #TODO: Not 404
@@ -310,45 +339,65 @@ class Module(XModule):
         For the "show answer" button.
 
         TODO: show answer events should be logged here, not just in the problem.js
+
+        Returns the answers: {'answers' : answers}
         '''
         if not self.answer_available():
             raise self.system.exception404
         else:
             answers = self.lcp.get_question_answers()
-            return json.dumps(answers,
-                              cls=ComplexEncoder)
+            return {'answers' : answers}
+
 
     # Figure out if we should move these to capa_problem?
     def get_problem(self, get):
-        ''' Same as get_problem_html -- if we want to reconfirm we
-            have the right thing e.g. after several AJAX calls.'''
-        return self.get_problem_html(encapsulate=False)
+        ''' Return results of get_problem_html, as a simple dict for json-ing.
+        { 'html': <the-html> }
+
+            Used if we want to reconfirm we have the right thing e.g. after
+            several AJAX calls.
+        ''' 
+        return {'html' : self.get_problem_html(encapsulate=False)}
+
+    @staticmethod
+    def make_dict_of_responses(get):
+        '''Make dictionary of student responses (aka "answers")
+        get is POST dictionary.
+        '''
+        answers = dict()
+        for key in get:
+            # e.g. input_resistor_1 ==> resistor_1
+            _, _, name = key.partition('_')
+            answers[name] = get[key]
+
+        return answers
 
     def check_problem(self, get):
         ''' Checks whether answers to a problem are correct, and
-            returns a map of correct/incorrect answers'''
+            returns a map of correct/incorrect answers:
+
+            {'success' : bool,
+             'contents' : html}
+            '''
         event_info = dict()
         event_info['state'] = self.lcp.get_state()
         event_info['filename'] = self.filename
 
-        # make a dict of all the student responses ("answers").
-        answers=dict()
-        # input_resistor_1 ==> resistor_1
-        for key in get:
-            answers['_'.join(key.split('_')[1:])]=get[key]
+        answers = self.make_dict_of_responses(get)
 
-        event_info['answers']=answers
+        event_info['answers'] = answers
 
         # Too late. Cannot submit
         if self.closed():
-            event_info['failure']='closed'
+            event_info['failure'] = 'closed'
             self.tracker('save_problem_check_fail', event_info)
+            # TODO (vshnayder): probably not 404?
             raise self.system.exception404
 
         # Problem submitted. Student should reset before checking
         # again.
         if self.lcp.done and self.rerandomize == "always":
-            event_info['failure']='unreset'
+            event_info['failure'] = 'unreset'
             self.tracker('save_problem_check_fail', event_info)
             raise self.system.exception404
 
@@ -357,89 +406,107 @@ class Module(XModule):
             lcp_id = self.lcp.problem_id
             correct_map = self.lcp.grade_answers(answers)
         except StudentInputError as inst:
-            self.lcp = LoncapaProblem(self.filestore.open(self.filename), id=lcp_id, state=old_state, system=self.system)
+            # TODO (vshnayder): why is this line here?
+            self.lcp = LoncapaProblem(self.filestore.open(self.filename),
+                                      id=lcp_id, state=old_state, system=self.system)
             traceback.print_exc()
-            return json.dumps({'success':inst.message})
+            return {'success': inst.message}
         except:
-            self.lcp = LoncapaProblem(self.filestore.open(self.filename), id=lcp_id, state=old_state, system=self.system)
+            # TODO: why is this line here?
+            self.lcp = LoncapaProblem(self.filestore.open(self.filename),
+                                      id=lcp_id, state=old_state, system=self.system)
             traceback.print_exc()
             raise Exception,"error in capa_module"
-            return json.dumps({'success':'Unknown Error'})
+            # TODO: Dead code...  is this a bug, or just old?
+            return {'success':'Unknown Error'}
 
         self.attempts = self.attempts + 1
-        self.lcp.done=True
+        self.lcp.done = True
 
-        success = 'correct'				# success = correct if ALL questions in this problem are correct
+        # success = correct if ALL questions in this problem are correct
+        success = 'correct'
         for answer_id in correct_map:
             if not correct_map.is_correct(answer_id):
                 success = 'incorrect'
 
-        event_info['correct_map']=correct_map.get_dict()	# log this in the tracker
-        event_info['success']=success
+        event_info['correct_map'] = correct_map.get_dict()	# log this in the tracker
+        event_info['success'] = success
         self.tracker('save_problem_check', event_info)
 
         try:
             html = self.get_problem_html(encapsulate=False)	# render problem into HTML
         except Exception,err:
             log.error('failed to generate html')
-            raise Exception,err
+            raise
+
+        return {'success': success,
+                'contents': html,
+                }
 
-        return json.dumps({'success': success,
-                           'contents': html,
-                           })
 
     def save_problem(self, get):
+        '''
+        Save the passed in answers.
+        Returns a dict { 'success' : bool, ['error' : error-msg]},
+        with the error key only present if success is False.
+        '''
         event_info = dict()
         event_info['state'] = self.lcp.get_state()
         event_info['filename'] = self.filename
 
-        answers=dict()
-        for key in get:
-            answers['_'.join(key.split('_')[1:])]=get[key]
+        answers = self.make_dict_of_responses(get)
         event_info['answers'] = answers
 
         # Too late. Cannot submit
         if self.closed():
-            event_info['failure']='closed'
+            event_info['failure'] = 'closed'
             self.tracker('save_problem_fail', event_info)
-            return "Problem is closed"
+            return {'success': False,
+                    'error': "Problem is closed"}
 
         # Problem submitted. Student should reset before saving
         # again.
         if self.lcp.done and self.rerandomize == "always":
-            event_info['failure']='done'
+            event_info['failure'] = 'done'
             self.tracker('save_problem_fail', event_info)
-            return "Problem needs to be reset prior to save."
+            return {'success' : False,
+                    'error' : "Problem needs to be reset prior to save."}
 
-        self.lcp.student_answers=answers
+        self.lcp.student_answers = answers
 
+        # TODO: should this be save_problem_fail?  Looks like success to me...
         self.tracker('save_problem_fail', event_info)
-        return json.dumps({'success':True})
+        return {'success': True}
 
     def reset_problem(self, get):
         ''' Changes problem state to unfinished -- removes student answers,
-            and causes problem to rerender itself. '''
+            and causes problem to rerender itself.
+
+            Returns problem html as { 'html' : html-string }.
+        ''' 
         event_info = dict()
-        event_info['old_state']=self.lcp.get_state()
-        event_info['filename']=self.filename
+        event_info['old_state'] = self.lcp.get_state()
+        event_info['filename'] = self.filename
 
         if self.closed():
-            event_info['failure']='closed'
+            event_info['failure'] = 'closed'
             self.tracker('reset_problem_fail', event_info)
             return "Problem is closed"
 
         if not self.lcp.done:
-            event_info['failure']='not_done'
+            event_info['failure'] = 'not_done'
             self.tracker('reset_problem_fail', event_info)
             return "Refresh the page and make an attempt before resetting."
 
-        self.lcp.do_reset()		# call method in LoncapaProblem to reset itself
+        self.lcp.do_reset()
         if self.rerandomize == "always":
-            self.lcp.seed=None		# reset random number generator seed (note the self.lcp.get_state() in next line)
- 
-        self.lcp=LoncapaProblem(self.filestore.open(self.filename), self.item_id, self.lcp.get_state(), system=self.system)
+            # reset random number generator seed (note the self.lcp.get_state() in next line)
+            self.lcp.seed=None
+            
+        self.lcp = LoncapaProblem(self.filestore.open(self.filename),
+                                  self.item_id, self.lcp.get_state(), system=self.system)
 
-        event_info['new_state']=self.lcp.get_state()
+        event_info['new_state'] = self.lcp.get_state()
         self.tracker('reset_problem', event_info)
 
-        return json.dumps(self.get_problem_html(encapsulate=False))
+        return {'html' : self.get_problem_html(encapsulate=False)}

common/lib/xmodule/progress.py

@@ -0,0 +1,126 @@
+'''
+Progress class for modules.  Represents where a student is in a module.
+'''
+
+from collections import namedtuple
+import numbers
+
+class Progress(object):
+    '''Represents a progress of a/b (a out of b done)
+
+    a and b must be numeric, but not necessarily integer, with
+    0 <= a <= b and b > 0.
+
+    Progress can only represent Progress for modules where that makes sense.  Other
+    modules (e.g. html) should return None from get_progress().
+
+    TODO: add tag for module type?  Would allow for smarter merging.
+    '''
+
+    def __init__(self, a, b):
+        '''Construct a Progress object.  a and b must be numbers, and must have
+              0 <= a <= b and b > 0
+        '''
+
+        # Want to do all checking at construction time, so explicitly check types
+        if not (isinstance(a, numbers.Number) and
+                isinstance(b, numbers.Number)):
+            raise TypeError('a and b must be numbers.  Passed {0}/{1}'.format(a, b))
+        
+        if not (0 <= a <= b and b > 0):
+            raise ValueError(
+                'fraction a/b = {0}/{1} must have 0 <= a <= b and b > 0'.format(a, b))
+
+        self._a = a
+        self._b = b
+
+    def frac(self):
+        ''' Return tuple (a,b) representing progress of a/b'''
+        return (self._a, self._b)
+
+    def percent(self):
+        ''' Returns a percentage progress as a float between 0 and 100.
+
+        subclassing note: implemented in terms of frac(), assumes sanity
+        checking is done at construction time.
+        '''
+        (a, b) = self.frac()
+        return 100.0 * a / b
+
+    def started(self):
+        ''' Returns True if fractional progress is greater than 0.
+
+        subclassing note: implemented in terms of frac(), assumes sanity
+        checking is done at construction time.
+        '''
+        return self.frac()[0] > 0
+
+
+    def inprogress(self):
+        ''' Returns True if fractional progress is strictly between 0 and 1.
+
+        subclassing note: implemented in terms of frac(), assumes sanity
+        checking is done at construction time.
+        ''' 
+        (a, b) = self.frac()
+        return a > 0 and a < b
+
+    def done(self):
+        ''' Return True if this represents done.
+
+        subclassing note: implemented in terms of frac(), assumes sanity
+        checking is done at construction time.
+        '''
+        (a, b) = self.frac()
+        return a==b
+
+
+    def ternary_str(self):
+        ''' Return a string version of this progress: either
+           "none", "in_progress", or "done".
+
+        subclassing note: implemented in terms of frac()
+        ''' 
+        (a, b) = self.frac()
+        if a == 0:
+            return "none"
+        if a < b:
+            return "in_progress"
+        return "done"
+
+    def __eq__(self, other):
+        ''' Two Progress objects are equal if they have identical values.
+        Implemented in terms of frac()'''
+        if not isinstance(other, Progress):
+            return False
+        (a, b) = self.frac()
+        (a2, b2) = other.frac()
+        return a == a2 and b == b2
+
+    def __ne__(self, other):
+        ''' The opposite of equal'''
+        return not self.__eq__(other)
+        
+    
+    def __str__(self):
+        ''' Return a string representation of this string.
+
+        subclassing note: implemented in terms of frac().
+        '''
+        (a, b) = self.frac()
+        return "{0}/{1}".format(a, b)
+
+    @staticmethod
+    def add_counts(a, b):
+        '''Add two progress indicators, assuming that each represents items done:
+        (a / b) + (c / d) = (a + c) / (b + d).
+        If either is None, returns the other.
+        '''
+        if a is None:
+            return b
+        if b is None:
+            return a
+        # get numerators + denominators
+        (n, d) = a.frac()
+        (n2, d2) = b.frac()
+        return Progress(n + n2, d + d2)

common/lib/xmodule/seq_module.py

@@ -1,8 +1,12 @@
 import json
+import logging
 
 from lxml import etree
 
 from x_module import XModule, XModuleDescriptor
+from xmodule.progress import Progress
+
+log = logging.getLogger("mitx.common.lib.seq_module")
 
 # HACK: This shouldn't be hard-coded to two types
 # OBSOLETE: This obsoletes 'type'
@@ -37,6 +41,16 @@ class Module(XModule):
         self.render()
         return self.destroy_js
 
+    def get_progress(self):
+        ''' Return the total progress, adding total done and total available.
+        (assumes that each submodule uses the same "units" for progress.)
+        '''
+        # TODO: Cache progress or children array?
+        children = self.get_children()
+        progresses = [child.get_progress() for child in children]
+        progress = reduce(Progress.add_counts, progresses)
+        return progress
+
     def handle_ajax(self, dispatch, get):		# TODO: bounds checking
         ''' get = request.POST instance '''
         if dispatch=='goto_position':
@@ -53,10 +67,15 @@ class Module(XModule):
         titles = ["\n".join([i.get("name").strip() for i in e.iter() if i.get("name") is not None]) \
                        for e in self.xmltree]
 
+        children = self.get_children()
+        progresses = [child.get_progress() for child in children]
+        
         self.contents = self.rendered_children()
 
-        for contents, title in zip(self.contents, titles):
+        for contents, title, progress in zip(self.contents, titles, progresses):
             contents['title'] = title
+            contents['progress_str'] = str(progress) if progress is not None else ""
+            contents['progress_stat'] = progress.ternary_str() if progress is not None else ""
 
         for (content, element_class) in zip(self.contents, child_classes):
             new_class = 'other'
@@ -68,16 +87,17 @@ class Module(XModule):
         # Split </script> tags -- browsers handle this as end
         # of script, even if it occurs mid-string. Do this after json.dumps()ing
         # so that we can be sure of the quotations being used
-        params={'items':json.dumps(self.contents).replace('</script>', '<"+"/script>'),
-                'id':self.item_id,
+        params={'items': json.dumps(self.contents).replace('</script>', '<"+"/script>'),
+                'id': self.item_id,
                 'position': self.position,
-                'titles':titles,
-                'tag':self.xmltree.tag}
+                'titles': titles,
+                'tag': self.xmltree.tag}
 
         if self.xmltree.tag in ['sequential', 'videosequence']:
             self.content = self.system.render_template('seq_module.html', params)
         if self.xmltree.tag == 'tab':
             self.content = self.system.render_template('tab_module.html', params)
+        log.debug("rendered content: %s", content)
         self.rendered = True
 
     def __init__(self, system, xml, item_id, state=None):

common/lib/xmodule/tests.py

@@ -13,8 +13,9 @@ import numpy
 import xmodule
 import capa.calc as calc
 import capa.capa_problem as lcp
-from xmodule import graders
+from xmodule import graders, x_module
 from xmodule.graders import Score, aggregate_scores
+from xmodule.progress import Progress
 from nose.plugins.skip import SkipTest
 
 class I4xSystem(object):
@@ -26,7 +27,9 @@ class I4xSystem(object):
     def __init__(self):
         self.ajax_url = '/'
         self.track_function = lambda x: None
+        self.filestore = None
         self.render_function = lambda x: {} # Probably incorrect
+        self.module_from_xml = lambda x: None  # May need a real impl...
         self.exception404 = Exception
         self.DEBUG = True
     def __repr__(self):
@@ -488,3 +491,118 @@ class GraderTest(unittest.TestCase):
         
         #TODO: How do we test failure cases? The parser only logs an error when it can't parse something. Maybe it should throw exceptions?
 
+# --------------------------------------------------------------------------
+# Module progress tests
+        
+class ProgressTest(unittest.TestCase):
+    ''' Test that basic Progress objects work.  A Progress represents a
+    fraction between 0 and 1.
+    '''
+    not_started = Progress(0, 17)
+    part_done = Progress(2, 6)
+    half_done = Progress(3, 6)
+    also_half_done = Progress(1, 2)
+    done = Progress(7, 7)
+    
+    def test_create_object(self):
+        # These should work:
+        p = Progress(0, 2)
+        p = Progress(1, 2)
+        p = Progress(2, 2)
+
+        p = Progress(2.5, 5.0)
+        p = Progress(3.7, 12.3333)
+        
+        # These shouldn't
+        self.assertRaises(ValueError, Progress, 0, 0)
+        self.assertRaises(ValueError, Progress, 2, 0)
+        self.assertRaises(ValueError, Progress, 1, -2)
+        self.assertRaises(ValueError, Progress, 3, 2)
+        self.assertRaises(ValueError, Progress, -2, 5)
+
+        self.assertRaises(TypeError, Progress, 0, "all")
+        # check complex numbers just for the heck of it :)
+        self.assertRaises(TypeError, Progress, 2j, 3)
+
+    def test_frac(self):
+        p = Progress(1, 2)
+        (a, b) = p.frac()
+        self.assertEqual(a, 1)
+        self.assertEqual(b, 2)
+
+    def test_percent(self):
+        self.assertEqual(self.not_started.percent(), 0)
+        self.assertAlmostEqual(self.part_done.percent(), 33.33333333333333)
+        self.assertEqual(self.half_done.percent(), 50)
+        self.assertEqual(self.done.percent(), 100)
+
+        self.assertEqual(self.half_done.percent(), self.also_half_done.percent())
+
+    def test_started(self):
+        self.assertFalse(self.not_started.started())
+
+        self.assertTrue(self.part_done.started())
+        self.assertTrue(self.half_done.started())
+        self.assertTrue(self.done.started())
+
+    def test_inprogress(self):
+        # only true if working on it
+        self.assertFalse(self.done.inprogress())
+        self.assertFalse(self.not_started.inprogress())
+
+        self.assertTrue(self.part_done.inprogress())
+        self.assertTrue(self.half_done.inprogress())
+
+    def test_done(self):
+        self.assertTrue(self.done.done())
+        self.assertFalse(self.half_done.done())
+        self.assertFalse(self.not_started.done())
+        
+    def test_str(self):
+        self.assertEqual(str(self.not_started), "0/17")
+        self.assertEqual(str(self.part_done), "2/6")
+        self.assertEqual(str(self.done), "7/7")
+
+    def test_ternary_str(self):
+        self.assertEqual(self.not_started.ternary_str(), "none")
+        self.assertEqual(self.half_done.ternary_str(), "in_progress")
+        self.assertEqual(self.done.ternary_str(), "done")
+
+    def test_add(self):
+        '''Test the Progress.add_counts() method'''
+        p = Progress(0, 2)
+        p2 = Progress(1, 3)
+        p3 = Progress(2, 5)
+        pNone = None
+        add = lambda a, b: Progress.add_counts(a, b).frac()
+
+        self.assertEqual(add(p, p), (0, 4))
+        self.assertEqual(add(p, p2), (1, 5))
+        self.assertEqual(add(p2, p3), (3, 8))
+        
+        self.assertEqual(add(p2, pNone), p2.frac())
+        self.assertEqual(add(pNone, p2), p2.frac())
+
+    def test_equality(self):
+        '''Test that comparing Progress objects for equality
+        works correctly.'''
+        p = Progress(1, 2)
+        p2 = Progress(2, 4)
+        p3 = Progress(1, 2)
+        self.assertTrue(p == p3)
+        self.assertFalse(p == p2)
+
+        # Check != while we're at it
+        self.assertTrue(p != p2)
+        self.assertFalse(p != p3)
+
+
+class ModuleProgressTest(unittest.TestCase):
+    ''' Test that get_progress() does the right thing for the different modules
+    '''
+    def test_xmodule_default(self):
+        '''Make sure default get_progress exists, returns None'''
+        xm = x_module.XModule(i4xs, "<dummy />", "dummy")
+        p = xm.get_progress()
+        self.assertEqual(p, None)
+        

common/lib/xmodule/vertical_module.py

@@ -1,12 +1,14 @@
 import json
 
 from x_module import XModule, XModuleDescriptor
+from xmodule.progress import Progress
 from lxml import etree
 
 class ModuleDescriptor(XModuleDescriptor):
     pass
 
 class Module(XModule):
+    ''' Layout module for laying out submodules vertically.'''
     id_attribute = 'id'
 
     def get_state(self):
@@ -21,6 +23,13 @@ class Module(XModule):
             'items': self.contents
         })
 
+    def get_progress(self):
+        # TODO: Cache progress or children array?
+        children = self.get_children()
+        progresses = [child.get_progress() for child in children]
+        progress = reduce(Progress.add_counts, progresses)
+        return progress
+
     def __init__(self, system, xml, item_id, state=None):
         XModule.__init__(self, system, xml, item_id, state)
         xmltree=etree.fromstring(xml)

common/lib/xmodule/x_module.py

@@ -1,7 +1,9 @@
 from lxml import etree
 import pkg_resources
 import logging
+
 from keystore import Location
+from progress import Progress
 
 log = logging.getLogger('mitx.' + __name__)
 
@@ -57,6 +59,13 @@ class XModule(object):
         else: 
             raise "We should iterate through children and find a default name"
 
+    def get_children(self):
+        '''
+        Return module instances for all the children of this module.
+        '''
+        children = [self.module_from_xml(e) for e in self.__xmltree]
+        return children            
+
     def rendered_children(self):
         '''
         Render all children. 
@@ -90,6 +99,7 @@ class XModule(object):
             self.tracker = system.track_function
             self.filestore = system.filestore
             self.render_function = system.render_function
+            self.module_from_xml = system.module_from_xml
             self.DEBUG = system.DEBUG
         self.system = system
 
@@ -118,6 +128,15 @@ class XModule(object):
         '''
         return "Unimplemented"
 
+    def get_progress(self):
+        ''' Return a progress.Progress object that represents how far the student has gone
+        in this module.  Must be implemented to get correct progress tracking behavior in
+        nesting modules like sequence and vertical.
+
+        If this module has no notion of progress, return None.
+        '''
+        return None
+
     def handle_ajax(self, dispatch, get):
         ''' dispatch is last part of the URL. 
             get is a dictionary-like object '''