From e5a791e031b74db53c68fadbb8baea0cbb0fcd5f Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 20 Jun 2012 15:53:59 -0400 Subject: [PATCH 1/3] Add scaffolding comments for progress tracking in video modules. --- common/lib/xmodule/video_module.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/video_module.py b/common/lib/xmodule/video_module.py index d7c7f80291..f3d615fd3d 100644 --- a/common/lib/xmodule/video_module.py +++ b/common/lib/xmodule/video_module.py @@ -4,6 +4,7 @@ import logging from lxml import etree from x_module import XModule, XModuleDescriptor +from progress import Progress log = logging.getLogger("mitx.courseware.modules") @@ -15,17 +16,32 @@ class Module(XModule): video_time = 0 def handle_ajax(self, dispatch, get): + ''' + Handle ajax calls to this video. + TODO (vshnayder): This is not being called right now, so the position + is not being saved. + ''' log.debug(u"GET {0}".format(get)) log.debug(u"DISPATCH {0}".format(dispatch)) if dispatch == 'goto_position': self.position = int(float(get['position'])) - log.debug(u"NEW POSITION {0}".format(self.position)) + log.info(u"NEW POSITION {0}".format(self.position)) return json.dumps({'success':True}) raise Http404() + def get_progress(self): + ''' TODO (vshnayder): Get and save duration of youtube video, then return + fraction watched. + (Be careful to notice when video link changes and update) + + For now, we have no way of knowing if the video has even been watched, so + just return None. + ''' + return None + def get_state(self): log.debug(u"STATE POSITION {0}".format(self.position)) - return json.dumps({ 'position':self.position }) + return json.dumps({ 'position': self.position }) @classmethod def get_xml_tags(c): @@ -41,15 +57,16 @@ class Module(XModule): 'id': self.item_id, 'position': self.position, 'name': self.name, - 'annotations': self.annotations + 'annotations': self.annotations, }) def __init__(self, system, xml, item_id, state=None): XModule.__init__(self, system, xml, item_id, state) - xmltree=etree.fromstring(xml) + xmltree = etree.fromstring(xml) self.youtube = xmltree.get('youtube') self.name = xmltree.get('name') self.position = 0 + if state is not None: state = json.loads(state) if 'position' in state: From 85bee9b42b815cd1d71ed07fd2db03c988c07349 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 20 Jun 2012 15:55:21 -0400 Subject: [PATCH 2/3] Initial UI for sequence progress. * change bottom border of links: red for not started yellow for in_progress green for done * This should probably be designed at some point. * Obvious problems: the yellow is not very visible, and lots of people are red/green color-blind. --- lms/static/sass/courseware/_sequence-nav.scss | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lms/static/sass/courseware/_sequence-nav.scss b/lms/static/sass/courseware/_sequence-nav.scss index 9a2aee57af..4472724e6d 100644 --- a/lms/static/sass/courseware/_sequence-nav.scss +++ b/lms/static/sass/courseware/_sequence-nav.scss @@ -66,6 +66,26 @@ nav.sequence-nav { @include transition(all, .4s, $ease-in-out-quad); width: 100%; + &.progress { + border-bottom-style: solid; + border-bottom-width: 4px; + } + + &.progress-none { + @extend .progress; + border-bottom-color: red; + } + + &.progress-some { + @extend .progress; + border-bottom-color: yellow; + } + + &.progress-done { + @extend .progress; + border-bottom-color: green; + } + //video &.seq_video_inactive { @extend .inactive; From 5e7535fbfbc30d91e6379164fbb9b1c72f503de9 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 20 Jun 2012 16:01:56 -0400 Subject: [PATCH 3/3] 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