Progress tracking cleanup.

* use clearer names for the two status strings passed to js
* add functions to do canonical conversion of progress to js string
* fix updating bug in sequence.coffee
* add some todo comments and other docs to make future expansion easier

common/lib/xmodule/capa_module.py

@@ -298,7 +298,7 @@ class Module(XModule):
         after = self.get_progress()
         d.update({
             'progress_changed' : after != before,
-            'progress' : after.ternary_str(),
+            'progress_status' : Progress.to_js_status_str(after),
             })
         return json.dumps(d, cls=ComplexEncoder)
 

common/lib/xmodule/progress.py

@@ -1,6 +1,17 @@
 '''
 Progress class for modules.  Represents where a student is in a module.
-'''
+
+Useful things to know:
+ - Use Progress.to_js_status_str() to convert a progress into a simple
+   status string to pass to js.  
+ - Use Progress.to_js_detail_str() to convert a progress into a more detailed
+   string to pass to js.
+
+In particular, these functions have a canonical handing of None.
+
+For most subclassing needs, you should only need to reimplement
+frac() and __str__().
+''' 
 
 from collections import namedtuple
 import numbers
@@ -124,3 +135,26 @@ class Progress(object):
         (n, d) = a.frac()
         (n2, d2) = b.frac()
         return Progress(n + n2, d + d2)
+
+    @staticmethod
+    def to_js_status_str(progress):
+        '''
+        Return the "status string" version of the passed Progress
+        object that should be passed to js.  Use this function when
+        sending Progress objects to js to limit dependencies.
+        '''
+        if progress is None:
+            return "NA"
+        return progress.ternary_str()
+
+
+    @staticmethod
+    def to_js_detail_str(progress):
+        '''
+        Return the "detail string" version of the passed Progress
+        object that should be passed to js.  Use this function when
+        passing Progress objects to js to limit dependencies.
+        '''
+        if progress is None:
+            return "NA"
+        return str(progress)

common/lib/xmodule/seq_module.py

@@ -74,8 +74,8 @@ class Module(XModule):
 
         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 ""
+            contents['progress_status'] = Progress.to_js_status_str(progress)
+            contents['progress_detail'] = Progress.to_js_detail_str(progress)
 
         for (content, element_class) in zip(self.contents, child_classes):
             new_class = 'other'

common/lib/xmodule/tests.py

@@ -568,6 +568,22 @@ class ProgressTest(unittest.TestCase):
         self.assertEqual(self.half_done.ternary_str(), "in_progress")
         self.assertEqual(self.done.ternary_str(), "done")
 
+    def test_to_js_status(self):
+        '''Test the Progress.to_js_status_str() method'''
+        
+        self.assertEqual(Progress.to_js_status_str(self.not_started), "none")
+        self.assertEqual(Progress.to_js_status_str(self.half_done), "in_progress")
+        self.assertEqual(Progress.to_js_status_str(self.done), "done")
+        self.assertEqual(Progress.to_js_status_str(None), "NA")
+
+    def test_to_js_detail_str(self):
+        '''Test the Progress.to_js_detail_str() method'''
+        f = Progress.to_js_detail_str
+        for p in (self.not_started, self.half_done, self.done):
+            self.assertEqual(f(p), str(p))
+        # But None should be encoded as NA
+        self.assertEqual(f(None), "NA")
+
     def test_add(self):
         '''Test the Progress.add_counts() method'''
         p = Progress(0, 2)

lms/djangoapps/courseware/module_render.py

@@ -199,7 +199,7 @@ def get_module(user, request, module_xml, student_module_cache, position=None):
                             etree.tostring(module_xml), 
                             module_id, 
                             state=state)
-    
+
     # If StudentModule for this instance wasn't already in the database,
     # and this isn't a guest user, create it.
     if not smod and user.is_authenticated():

lms/static/coffee/src/modules/problem.coffee

@@ -19,7 +19,7 @@ class @Problem
 
   update_progress: (response) =>
     if response.progress_changed
-        @element.attr progress: response.progress
+        @element.attr progress: response.progress_status
         @element.trigger('progressChanged')
 
   render: (content) ->

lms/static/coffee/src/modules/sequence.coffee

@@ -20,8 +20,16 @@ class @Sequence
     $('.problems-wrapper').bind 'progressChanged', @updateProgress
 
   mergeProgress: (p1, p2) ->
+    # if either is "NA", return the other one
+    if p1 == "NA"
+      return p2
+    if p2 == "NA"
+      return p1
+
+    # Both real progresses
     if p1 == "done" and p2 == "done"
       return "done"
+
     # not done, so if any progress on either, in_progress
     w1 = p1 == "done" or p1 == "in_progress"
     w2 = p2 == "done" or p2 == "in_progress"
@@ -31,7 +39,7 @@ class @Sequence
     return "none"
 
   updateProgress: =>
-    new_progress = "none"
+    new_progress = "NA"
     _this = this
     $('.problems-wrapper').each (index) ->
       progress = $(this).attr 'progress'
@@ -41,6 +49,7 @@ class @Sequence
     @setProgress(new_progress, @link_for(@position))
 
   setProgress: (progress, element) ->
+      # If progress is "NA", don't add any css class
       element.removeClass('progress-none')
              .removeClass('progress-some')
              .removeClass('progress-done')
@@ -53,10 +62,12 @@ class @Sequence
     $.each @elements, (index, item) =>
       link = $('<a>').attr class: "seq_#{item.type}_inactive", 'data-element': index + 1
       title = $('<p>').html(item.title)
-      # TODO: add item.progress_str either to the title or somewhere else.
-      # Make sure it gets updated after ajax calls
+      # TODO (vshnayder): add item.progress_detail either to the title or somewhere else.
+      # Make sure it gets updated after ajax calls.
+      # implementation note: will need to figure out how to handle combining detail
+      # statuses of multiple modules in js. 
       list_item = $('<li>').append(link.append(title))
-      @setProgress item.progress_stat, link
+      @setProgress item.progress_status, link
         
       @$('#sequence-list').append list_item