From 897a87646c8ece39e4f36701c37392515a3e2286 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 6 Sep 2012 15:16:24 -0400 Subject: [PATCH 1/3] Make progress handle extra credit by capping at 100%, and handle malformed progress stuff by ignoring any changes --- common/lib/xmodule/xmodule/capa_module.py | 14 ++++++++++---- common/lib/xmodule/xmodule/progress.py | 8 +++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 2cf8226662..386d76696a 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -322,10 +322,16 @@ class CapaModule(XModule): before = self.get_progress() d = handlers[dispatch](get) - after = self.get_progress() - d.update({ - 'progress_changed': after != before, - 'progress_status': Progress.to_js_status_str(after), + try: + after = self.get_progress() + d.update({ + 'progress_changed': after != before, + 'progress_status': Progress.to_js_status_str(after), + }) + except ValueError: + d.update({ + 'progress_changed': False, + 'progress_status': Progress.to_js_status(before), }) return json.dumps(d, cls=ComplexEncoder) diff --git a/common/lib/xmodule/xmodule/progress.py b/common/lib/xmodule/xmodule/progress.py index 70c8ec9da1..af47f50fd7 100644 --- a/common/lib/xmodule/xmodule/progress.py +++ b/common/lib/xmodule/xmodule/progress.py @@ -39,9 +39,11 @@ class Progress(object): 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)) + if a > b: + a = b + + if b <= 0: + raise ValueError('fraction a/b = {0}/{1} must have b > 0'.format(a, b)) self._a = a self._b = b From 14eb160eea089f5a1c20194d1c47055b308bb327 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 6 Sep 2012 15:18:59 -0400 Subject: [PATCH 2/3] Just return a null progress if something goes wrong when retrieving capa_module progress (and log the error) --- common/lib/xmodule/xmodule/capa_module.py | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 386d76696a..d186bcc39c 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -202,10 +202,8 @@ class CapaModule(XModule): try: return Progress(score, total) except Exception as err: - # TODO (vshnayder): why is this still here? still needed? - if self.system.DEBUG: - return None - raise + log.exception("Got bad progress") + return None return None def get_html(self): @@ -322,16 +320,10 @@ class CapaModule(XModule): before = self.get_progress() d = handlers[dispatch](get) - try: - after = self.get_progress() - d.update({ - 'progress_changed': after != before, - 'progress_status': Progress.to_js_status_str(after), - }) - except ValueError: - d.update({ - 'progress_changed': False, - 'progress_status': Progress.to_js_status(before), + after = self.get_progress() + d.update({ + 'progress_changed': after != before, + 'progress_status': Progress.to_js_status_str(after), }) return json.dumps(d, cls=ComplexEncoder) From 5ce516a3a0cd0d9d2b14a7caec0f762252aa9b50 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 6 Sep 2012 15:23:19 -0400 Subject: [PATCH 3/3] Clamp progress to be greater than zero --- common/lib/xmodule/xmodule/progress.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/xmodule/xmodule/progress.py b/common/lib/xmodule/xmodule/progress.py index af47f50fd7..7adbb02646 100644 --- a/common/lib/xmodule/xmodule/progress.py +++ b/common/lib/xmodule/xmodule/progress.py @@ -42,6 +42,9 @@ class Progress(object): if a > b: a = b + if a < 0: + a = 0 + if b <= 0: raise ValueError('fraction a/b = {0}/{1} must have b > 0'.format(a, b))