diff --git a/common/lib/capa/capa_problem.py b/common/lib/capa/capa_problem.py index e70fa6cefff128e27b2bb4c98b0d902d1ec82e3f..a06ac1d7b6a9cd4a5f08357438d55c96315093b9 100644 --- a/common/lib/capa/capa_problem.py +++ b/common/lib/capa/capa_problem.py @@ -111,6 +111,7 @@ class LoncapaProblem(object): file_text = re.sub("endouttext\s*/", "/text", file_text) self.tree = etree.XML(file_text) # parse problem XML file into an element tree + self._process_includes() # handle any <include file="foo"> tags # construct script processor context (eg for customresponse problems) self.context = self._extract_context(self.tree, seed=self.seed) @@ -168,7 +169,8 @@ class LoncapaProblem(object): def get_score(self): ''' Compute score for this problem. The score is the number of points awarded. - Returns an integer, from 0 to get_max_score(). + Returns a dictionary {'score': integer, from 0 to get_max_score(), + 'total': get_max_score()}. ''' correct = 0 for key in self.correct_map: @@ -242,6 +244,36 @@ class LoncapaProblem(object): # ======= Private Methods Below ======== + def _process_includes(self): + ''' + Handle any <include file="foo"> tags by reading in the specified file and inserting it + into our XML tree. Fail gracefully if debugging. + ''' + includes = self.tree.findall('.//include') + for inc in includes: + file = inc.get('file') + if file is not None: + try: + ifp = self.system.filestore.open(file) # open using I4xSystem OSFS filestore + except Exception,err: + log.error('Error %s in problem xml include: %s' % (err,etree.tostring(inc,pretty_print=True))) + log.error('Cannot find file %s in %s' % (file,self.system.filestore)) + if not self.system.get('DEBUG'): # if debugging, don't fail - just log error + raise + else: continue + try: + incxml = etree.XML(ifp.read()) # read in and convert to XML + except Exception,err: + log.error('Error %s in problem xml include: %s' % (err,etree.tostring(inc,pretty_print=True))) + log.error('Cannot parse XML in %s' % (file)) + if not self.system.get('DEBUG'): # if debugging, don't fail - just log error + raise + else: continue + parent = inc.getparent() # insert new XML into tree in place of inlcude + parent.insert(parent.index(inc),incxml) + parent.remove(inc) + log.debug('Included %s into %s' % (file,self.fileobject)) + def _extract_context(self, tree, seed=struct.unpack('i', os.urandom(4))[0]): # private ''' Extract content of <script>...</script> from the problem.xml file, and exec it in the diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 55534f8a3e19e0df466bb4f66f639599bbe4f06c..3ace45cff41734d775ef1a2f66fcc2667b68ae05 100644 --- a/common/lib/xmodule/capa_module.py +++ b/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)} diff --git a/common/lib/xmodule/progress.py b/common/lib/xmodule/progress.py new file mode 100644 index 0000000000000000000000000000000000000000..b9e242f2b2da8c5a8e65def0b40d0fdcb57fa57f --- /dev/null +++ b/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) diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index b394227aa7d32d0efcfcf4d6177f6cb3c9876727..598fc4443e692d59bb24e79c2d2130be4ec238bc 100644 --- a/common/lib/xmodule/seq_module.py +++ b/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): diff --git a/common/lib/xmodule/tests.py b/common/lib/xmodule/tests.py index 370b3befe5d3667072bc3b1f466f10a880743926..73096ce8dafef1cab810b19b5082bcbd0134d500 100644 --- a/common/lib/xmodule/tests.py +++ b/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) + diff --git a/common/lib/xmodule/vertical_module.py b/common/lib/xmodule/vertical_module.py index 7eb6dda0b8f61ca47475c4135281d2fc0b740042..b3feec8baec4469df2f40dcd25ee5fd7464339c5 100644 --- a/common/lib/xmodule/vertical_module.py +++ b/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) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index d424de1d0e3cc766c279aebb9e1b9900fbda8318..044cfefce3dea2131fa4463ec0eb606f0ecba953 100644 --- a/common/lib/xmodule/x_module.py +++ b/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 ''' diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 354cf5991f3a4deb59bdfabfac8233871f29f192..4a11ec2d51eb24bbe1807e72ac9bde7e6086cc7e 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -174,6 +174,8 @@ def get_score(user, problem, cache, coursename=None): else: ## HACK 1: We shouldn't specifically reference capa_module ## HACK 2: Backwards-compatibility: This should be written when a grade is saved, and removed from the system + # TODO: These are no longer correct params for I4xSystem -- figure out what this code + # does, clean it up. from module_render import I4xSystem system = I4xSystem(None, None, None, coursename=coursename) total=float(xmodule.capa_module.Module(system, etree.tostring(problem), "id").max_score()) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index af5fec6b8589e27886857dd45eaf3bc4ec2dc749..9b5e7e4940cd5092062e0a13da17dbe920d4e0dc 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -34,7 +34,8 @@ class I4xSystem(object): and user, or other environment-specific info. ''' def __init__(self, ajax_url, track_function, render_function, - render_template, request=None, filestore=None): + module_from_xml, render_template, request=None, + filestore=None): ''' Create a closure around the system environment. @@ -43,6 +44,8 @@ class I4xSystem(object): or otherwise tracking the event. TODO: Not used, and has inconsistent args in different files. Update or remove. + module_from_xml - function that takes (module_xml) and returns a corresponding + module instance object. render_function - function that takes (module_xml) and renders it, returning a dictionary with a context for rendering the module to html. Dictionary will contain keys 'content' @@ -62,6 +65,7 @@ class I4xSystem(object): if settings.DEBUG: log.info("[courseware.module_render.I4xSystem] filestore path = %s", filestore) + self.module_from_xml = module_from_xml self.render_function = render_function self.render_template = render_template self.exception404 = Http404 @@ -127,6 +131,18 @@ def grade_histogram(module_id): return [] return grades + +def make_module_from_xml_fn(user, request, student_module_cache, position): + '''Create the make_from_xml() function''' + def module_from_xml(xml): + '''Modules need a way to convert xml to instance objects. + Pass the rest of the context through.''' + (instance, sm, module_type) = get_module( + user, request, xml, student_module_cache, position) + return instance + return module_from_xml + + def get_module(user, request, module_xml, student_module_cache, position=None): ''' Get an instance of the xmodule class corresponding to module_xml, setting the state based on an existing StudentModule, or creating one if none @@ -165,6 +181,9 @@ def get_module(user, request, module_xml, student_module_cache, position=None): # Setup system context for module instance ajax_url = settings.MITX_ROOT_URL + '/modx/' + module_type + '/' + module_id + '/' + module_from_xml = make_module_from_xml_fn( + user, request, student_module_cache, position) + system = I4xSystem(track_function = make_track_function(request), render_function = lambda xml: render_x_module( user, request, xml, student_module_cache, position), @@ -172,6 +191,7 @@ def get_module(user, request, module_xml, student_module_cache, position=None): ajax_url = ajax_url, request = request, filestore = OSFS(data_root), + module_from_xml = module_from_xml, ) # pass position specified in URL to module through I4xSystem system.set('position', position) @@ -295,9 +315,17 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): response = HttpResponse(json.dumps({'success': error_msg})) return response + # TODO: This doesn't have a cache of child student modules. Just + # passing the current one. If ajax calls end up needing children, + # this won't work (but fixing it may cause performance issues...) + # Figure out :) + module_from_xml = make_module_from_xml_fn( + request.user, request, [s], None) + # Create the module system = I4xSystem(track_function = make_track_function(request), - render_function = None, + render_function = None, + module_from_xml = module_from_xml, render_template = render_to_string, ajax_url = ajax_url, request = request, @@ -316,7 +344,11 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): return response # Let the module handle the AJAX - ajax_return = instance.handle_ajax(dispatch, request.POST) + try: + ajax_return = instance.handle_ajax(dispatch, request.POST) + except: + log.exception("error processing ajax call") + raise # Save the state back to the database s.state = instance.get_state() diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index aa3d1b778113be083f046de2a36cfb2be44d82fc..5cbbe18d7d1bccea53ea4bfbab35aaf08fc2f73d 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -82,7 +82,11 @@ def profile(request, student_id=None): def render_accordion(request, course, chapter, section): ''' Draws navigation bar. Takes current position in accordion as - parameter. Returns (initialization_javascript, content)''' + parameter. + + If chapter and section are '' or None, renders a default accordion. + + Returns (initialization_javascript, content)''' if not course: course = "6.002 Spring 2012" @@ -221,10 +225,8 @@ def index(request, course=None, chapter=None, section=None, ''' Fixes URLs -- we convert spaces to _ in URLs to prevent funny encoding characters and keep the URLs readable. This undoes that transformation. - - TODO: Properly replace underscores. (Q: what is properly?) ''' - return s.replace('_', ' ') + return s.replace('_', ' ') if s is not None else None def get_submodule_ids(module_xml): ''' diff --git a/lms/static/coffee/files.json b/lms/static/coffee/files.json index 1e5e010d738ce4a149090eb919b53026207554af..28b843021f742a02a3cd146bc0dddb679a59fc51 100644 --- a/lms/static/coffee/files.json +++ b/lms/static/coffee/files.json @@ -1,7 +1,7 @@ { "js_files": [ - "/static/js/jquery-1.6.2.min.js", - "/static/js/jquery-ui-1.8.16.custom.min.js", + "/static/js/jquery.min.js", + "/static/js/jquery-ui.min.js", "/static/js/jquery.leanModal.js", "/static/js/flot/jquery.flot.js" ], diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index aad41f23d4a3e4c50be33f1d57e61f8ff773f1dd..1b254d5d3f79c63d2c748387cb35ae4734d2234c 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -17,12 +17,20 @@ class @Problem @$('section.action input.save').click @save @$('input.math').keyup(@refreshMath).each(@refreshMath) + update_progress: (response) => + if response.progress_changed + @element.attr progress: response.progress + @element.trigger('progressChanged') + render: (content) -> if content @element.html(content) @bind() else - @element.load @content_url, @bind + $.postWithPrefix "/modx/problem/#{@id}/problem_get", '', (response) => + @element.html(response.html) + @bind() + check: => Logger.log 'problem_check', @answers @@ -30,19 +38,22 @@ class @Problem switch response.success when 'incorrect', 'correct' @render(response.contents) + @update_progress response else alert(response.success) reset: => Logger.log 'problem_reset', @answers - $.postWithPrefix "/modx/problem/#{@id}/problem_reset", id: @id, (content) => - @render(content) + $.postWithPrefix "/modx/problem/#{@id}/problem_reset", id: @id, (response) => + @render(response.html) + @update_progress response show: => if !@element.hasClass 'showed' Logger.log 'problem_show', problem: @id $.postWithPrefix "/modx/problem/#{@id}/problem_show", (response) => - $.each response, (key, value) => + answers = response.answers + $.each answers, (key, value) => if $.isArray(value) for choice in value @$("label[for='input_#{key}_#{choice}']").attr correct_answer: 'true' @@ -51,6 +62,7 @@ class @Problem MathJax.Hub.Queue ["Typeset", MathJax.Hub] @$('.show').val 'Hide Answer' @element.addClass 'showed' + @update_progress response else @$('[id^=answer_], [id^=solution_]').text '' @$('[correct_answer]').attr correct_answer: null @@ -62,6 +74,7 @@ class @Problem $.postWithPrefix "/modx/problem/#{@id}/problem_save", @answers, (response) => if response.success alert 'Saved' + @update_progress response refreshMath: (event, element) => element = event.target unless element diff --git a/lms/static/coffee/src/modules/sequence.coffee b/lms/static/coffee/src/modules/sequence.coffee index 463bf419fcce6645715e5e9de0aadcbb0533424b..72a1c82ab6ccd6d0b4ed9f51236138bb94e3bfe6 100644 --- a/lms/static/coffee/src/modules/sequence.coffee +++ b/lms/static/coffee/src/modules/sequence.coffee @@ -2,6 +2,7 @@ class @Sequence constructor: (@id, @elements, @tag, position) -> @element = $("#sequence_#{@id}") @buildNavigation() + @initProgress() @bind() @render position @@ -11,11 +12,52 @@ class @Sequence bind: -> @$('#sequence-list a').click @goto + initProgress: -> + @progressTable = {} # "#problem_#{id}" -> progress + + + hookUpProgressEvent: -> + $('.problems-wrapper').bind 'progressChanged', @updateProgress + + mergeProgress: (p1, p2) -> + 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" + if w1 or w2 + return "in_progress" + + return "none" + + updateProgress: => + new_progress = "none" + _this = this + $('.problems-wrapper').each (index) -> + progress = $(this).attr 'progress' + new_progress = _this.mergeProgress progress, new_progress + + @progressTable[@position] = new_progress + @setProgress(new_progress, @link_for(@position)) + + setProgress: (progress, element) -> + element.removeClass('progress-none') + .removeClass('progress-some') + .removeClass('progress-done') + switch progress + when 'none' then element.addClass('progress-none') + when 'in_progress' then element.addClass('progress-some') + when 'done' then element.addClass('progress-done') + buildNavigation: -> $.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 list_item = $('<li>').append(link.append(title)) + @setProgress item.progress_stat, link + @$('#sequence-list').append list_item toggleArrows: => @@ -36,13 +78,14 @@ class @Sequence if @position != undefined @mark_visited @position $.postWithPrefix "/modx/#{@tag}/#{@id}/goto_position", position: new_position - + @mark_active new_position @$('#seq_content').html @elements[new_position - 1].content MathJax.Hub.Queue(["Typeset", MathJax.Hub]) @position = new_position @toggleArrows() + @hookUpProgressEvent() @element.trigger 'contentChanged' goto: (event) => @@ -67,7 +110,17 @@ class @Sequence @$("#sequence-list a[data-element=#{position}]") mark_visited: (position) -> - @link_for(position).attr class: "seq_#{@elements[position - 1].type}_visited" + # Don't overwrite class attribute to avoid changing Progress class + type = @elements[position - 1].type + element = @link_for(position) + element.removeClass("seq_#{type}_inactive") + .removeClass("seq_#{type}_active") + .addClass("seq_#{type}_visited") mark_active: (position) -> - @link_for(position).attr class: "seq_#{@elements[position - 1].type}_active" + # Don't overwrite class attribute to avoid changing Progress class + type = @elements[position - 1].type + element = @link_for(position) + element.removeClass("seq_#{type}_inactive") + .removeClass("seq_#{type}_visited") + .addClass("seq_#{type}_active") diff --git a/lms/templates/dogfood.html b/lms/templates/dogfood.html index 301f319cf0a0be48a7cb45c43a60817b0a320f1d..9a3c08b5288d6021ee1d810b3e0686065346b350 100644 --- a/lms/templates/dogfood.html +++ b/lms/templates/dogfood.html @@ -19,8 +19,8 @@ ## <link rel="stylesheet" href="/static/sass/application.css" type="text/css" media="all" / > % endif - <script type="text/javascript" src="${static.url('js/jquery-1.6.2.min.js')}"></script> - <script type="text/javascript" src="${static.url('js/jquery-ui-1.8.16.custom.min.js')}"></script> + <script type="text/javascript" src="${static.url('js/jquery.min.js')}"></script> + <script type="text/javascript" src="${static.url('js/jquery-ui.min.js')}"></script> <script type="text/javascript" src="${static.url('js/swfobject/swfobject.js')}"></script> % if settings.MITX_FEATURES['USE_DJANGO_PIPELINE']: diff --git a/lms/templates/marketing.html b/lms/templates/marketing.html index dada0fc8c12ebe412ca72bc0c74135c52f3c916d..6d34a47839caa5cc22f2fd4c12e3d5a257d87213 100644 --- a/lms/templates/marketing.html +++ b/lms/templates/marketing.html @@ -15,8 +15,8 @@ <%static:css group='marketing-ie'/> <![endif]--> - <script type="text/javascript" src="${static.url('js/jquery-1.6.2.min.js')}"></script> - <script type="text/javascript" src="${static.url('js/jquery-ui-1.8.16.custom.min.js')}"></script> + <script type="text/javascript" src="${static.url('js/jquery.min.js')}"></script> + <script type="text/javascript" src="${static.url('js/jquery-ui.min.js')}"></script> <script type="text/javascript" src="${static.url('js/jquery.leanModal.min.js')}"></script> <!--script type="text/javascript" src="${static.url('js/swfobject/swfobject.js')}"></script--> diff --git a/lms/templates/quickedit.html b/lms/templates/quickedit.html index ff44108a4f6b122cf2b411031265710b3c2ecf27..3fee3864aad24782b95e14fb054a00b1dfc12e52 100644 --- a/lms/templates/quickedit.html +++ b/lms/templates/quickedit.html @@ -19,8 +19,8 @@ ## <link rel="stylesheet" href="/static/sass/application.css" type="text/css" media="all" / > % endif - <script type="text/javascript" src="${static.url('js/jquery-1.6.2.min.js')}"></script> - <script type="text/javascript" src="${static.url('js/jquery-ui-1.8.16.custom.min.js')}"></script> + <script type="text/javascript" src="${static.url('js/jquery.min.js')}"></script> + <script type="text/javascript" src="${static.url('js/jquery-ui.min.js')}"></script> <script type="text/javascript" src="${static.url('js/swfobject/swfobject.js')}"></script> % if settings.MITX_FEATURES['USE_DJANGO_PIPELINE']: