From 5e7535fbfbc30d91e6379164fbb9b1c72f503de9 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 20 Jun 2012 16:01:56 -0400 Subject: [PATCH] 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 | 2 +- common/lib/xmodule/progress.py | 36 ++++++++++++++++++- common/lib/xmodule/seq_module.py | 4 +-- common/lib/xmodule/tests.py | 16 +++++++++ lms/djangoapps/courseware/module_render.py | 2 +- lms/static/coffee/src/modules/problem.coffee | 2 +- lms/static/coffee/src/modules/sequence.coffee | 19 +++++++--- 7 files changed, 71 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 3ace45cff4..2d21f14d1e 100644 --- a/common/lib/xmodule/capa_module.py +++ b/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) diff --git a/common/lib/xmodule/progress.py b/common/lib/xmodule/progress.py index b9e242f2b2..fe4793cca4 100644 --- a/common/lib/xmodule/progress.py +++ b/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) diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index 598fc4443e..8c3a148a51 100644 --- a/common/lib/xmodule/seq_module.py +++ b/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' diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index 73096ce8da..90187abc2a 100644 --- a/common/lib/xmodule/tests.py +++ b/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) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 9b5e7e4940..3a6fcbfb45 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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(): diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index 1b254d5d3f..a1759b28af 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/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) -> diff --git a/lms/static/coffee/src/modules/sequence.coffee b/lms/static/coffee/src/modules/sequence.coffee index 72a1c82ab6..32a90f51a5 100644 --- a/lms/static/coffee/src/modules/sequence.coffee +++ b/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 = $('').attr class: "seq_#{item.type}_inactive", 'data-element': index + 1 title = $('

').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 = $('

  • ').append(link.append(title)) - @setProgress item.progress_stat, link + @setProgress item.progress_status, link @$('#sequence-list').append list_item