diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 5b9c0a1899..7c7f94a26b 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -107,18 +107,18 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): """ return str(float(answer.values()[0])) - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """ This is the landing method for AJAX calls. """ if dispatch == 'get_hint': - out = self.get_hint(get) + out = self.get_hint(data) elif dispatch == 'get_feedback': - out = self.get_feedback(get) + out = self.get_feedback(data) elif dispatch == 'vote': - out = self.tally_vote(get) + out = self.tally_vote(data) elif dispatch == 'submit_hint': - out = self.submit_hint(get) + out = self.submit_hint(data) else: return json.dumps({'contents': 'Error - invalid operation.'}) @@ -128,19 +128,24 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): out.update({'op': dispatch}) return json.dumps({'contents': self.system.render_template('hinter_display.html', out)}) - def get_hint(self, get): + def get_hint(self, data): """ - The student got the incorrect answer found in get. Give him a hint. + The student got the incorrect answer found in data. Give him a hint. Called by hinter javascript after a problem is graded as incorrect. Args: - `get` -- must be interpretable by capa_answer_to_str. + `data` -- must be interpretable by capa_answer_to_str. Output keys: - 'best_hint' is the hint text with the most votes. - - 'rand_hint_1' and 'rand_hint_2' are two random hints to the answer in `get`. + - 'rand_hint_1' and 'rand_hint_2' are two random hints to the answer in `data`. - 'answer' is the parsed answer that was submitted. """ - answer = self.capa_answer_to_str(get) + try: + answer = self.capa_answer_to_str(data) + except ValueError: + # Sometimes, we get an answer that's just not parsable. Do nothing. + log.exception('Answer not parsable: ' + str(data)) + return # Look for a hint to give. # Make a local copy of self.hints - this means we only need to do one json unpacking. # (This is because xblocks storage makes the following command a deep copy.) @@ -176,12 +181,12 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): 'rand_hint_2': rand_hint_2, 'answer': answer} - def get_feedback(self, get): + def get_feedback(self, data): """ The student got it correct. Ask him to vote on hints, or submit a hint. Args: - `get` -- not actually used. (It is assumed that the answer is correct.) + `data` -- not actually used. (It is assumed that the answer is correct.) Output keys: - 'index_to_hints' maps previous answer indices to hints that the user saw earlier. - 'index_to_answer' maps previous answer indices to the actual answer submitted. @@ -216,20 +221,20 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): return {'index_to_hints': index_to_hints, 'index_to_answer': index_to_answer} - def tally_vote(self, get): + def tally_vote(self, data): """ Tally a user's vote on his favorite hint. Args: - `get` -- expected to have the following keys: + `data` -- expected to have the following keys: 'answer': ans_no (index in previous_answers) 'hint': hint_pk Returns key 'hint_and_votes', a list of (hint_text, #votes) pairs. """ if self.user_voted: return {} - ans_no = int(get['answer']) - hint_no = str(get['hint']) + ans_no = int(data['answer']) + hint_no = str(data['hint']) answer = self.previous_answers[ans_no][0] # We use temp_dict because we need to do a direct write for the database to update. temp_dict = self.hints @@ -249,19 +254,19 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): self.previous_answers = [] return {'hint_and_votes': hint_and_votes} - def submit_hint(self, get): + def submit_hint(self, data): """ Take a hint submission and add it to the database. Args: - `get` -- expected to have the following keys: + `data` -- expected to have the following keys: 'answer': answer index in previous_answers 'hint': text of the new hint that the user is adding Returns a thank-you message. """ # Do html escaping. Perhaps in the future do profanity filtering, etc. as well. - hint = escape(get['hint']) - answer = self.previous_answers[int(get['answer'])][0] + hint = escape(data['hint']) + answer = self.previous_answers[int(data['answer'])][0] # Only allow a student to vote or submit a hint once. if self.user_voted: return {'message': 'Sorry, but you have already voted!'} diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index f57e28ef46..17ae33e560 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -185,15 +185,15 @@ class CrowdsourceHinterTest(unittest.TestCase): A simple test of get_html - make sure it returns the html of the inner problem. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() def fake_get_display_items(): """ A mock of get_display_items """ return [FakeChild()] - m.get_display_items = fake_get_display_items - out_html = m.get_html() + mock_module.get_display_items = fake_get_display_items + out_html = mock_module.get_html() self.assertTrue('This is supposed to be test html.' in out_html) self.assertTrue('this/is/a/fake/ajax/url' in out_html) @@ -202,15 +202,15 @@ class CrowdsourceHinterTest(unittest.TestCase): get_html, except the module has no child :( Should return a polite error message. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() def fake_get_display_items(): """ Returns no children. """ return [] - m.get_display_items = fake_get_display_items - out_html = m.get_html() + mock_module.get_display_items = fake_get_display_items + out_html = mock_module.get_html() self.assertTrue('Error in loading crowdsourced hinter' in out_html) @unittest.skip("Needs to be finished.") @@ -220,8 +220,8 @@ class CrowdsourceHinterTest(unittest.TestCase): is called. NOT WORKING RIGHT NOW """ - m = VerticalWithModulesFactory.create() - out_html = m.get_html() + mock_module = VerticalWithModulesFactory.create() + out_html = mock_module.get_html() print out_html self.assertTrue('Test numerical problem.' in out_html) self.assertTrue('Another test numerical problem.' in out_html) @@ -232,20 +232,32 @@ class CrowdsourceHinterTest(unittest.TestCase): - Output should be blank. - New entry should be added to previous_answers """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() json_in = {'problem_name': '26.0'} - out = m.get_hint(json_in) + out = mock_module.get_hint(json_in) self.assertTrue(out is None) - self.assertTrue(['26.0', [None, None, None]] in m.previous_answers) + self.assertTrue(['26.0', [None, None, None]] in mock_module.previous_answers) + + def test_gethint_unparsable(self): + """ + Someone submits a hint that cannot be parsed into a float. + - The answer should not be added to previous_answers. + """ + mock_module = CHModuleFactory.create() + old_answers = copy.deepcopy(mock_module.previous_answers) + json_in = {'problem_name': 'fish'} + out = mock_module.get_hint(json_in) + self.assertTrue(out is None) + self.assertTrue(mock_module.previous_answers == old_answers) def test_gethint_1hint(self): """ Someone asks for a hint, with exactly one hint in the database. Output should contain that hint. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() json_in = {'problem_name': '25.0'} - out = m.get_hint(json_in) + out = mock_module.get_hint(json_in) self.assertTrue(out['best_hint'] == 'Really popular hint') def test_gethint_manyhints(self): @@ -256,9 +268,9 @@ class CrowdsourceHinterTest(unittest.TestCase): Currently, the best hint could be returned twice - need to fix this in implementation. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() json_in = {'problem_name': '24.0'} - out = m.get_hint(json_in) + out = mock_module.get_hint(json_in) self.assertTrue(out['best_hint'] == 'Best hint') self.assertTrue('rand_hint_1' in out) self.assertTrue('rand_hint_2' in out) @@ -268,9 +280,9 @@ class CrowdsourceHinterTest(unittest.TestCase): Someone has gotten the problem correct on the first try. Output should be empty. """ - m = CHModuleFactory.create(previous_answers=[]) + mock_module = CHModuleFactory.create(previous_answers=[]) json_in = {'problem_name': '42.5'} - out = m.get_feedback(json_in) + out = mock_module.get_feedback(json_in) self.assertTrue(out is None) def test_getfeedback_1wronganswer_nohints(self): @@ -279,9 +291,9 @@ class CrowdsourceHinterTest(unittest.TestCase): answer. However, we don't actually have hints for this problem. There should be a dialog to submit a new hint. """ - m = CHModuleFactory.create(previous_answers=[['26.0', [None, None, None]]]) + mock_module = CHModuleFactory.create(previous_answers=[['26.0', [None, None, None]]]) json_in = {'problem_name': '42.5'} - out = m.get_feedback(json_in) + out = mock_module.get_feedback(json_in) print out['index_to_answer'] self.assertTrue(out['index_to_hints'][0] == []) self.assertTrue(out['index_to_answer'][0] == '26.0') @@ -292,9 +304,9 @@ class CrowdsourceHinterTest(unittest.TestCase): a voting dialog, with the correct choices, plus a hint submission dialog. """ - m = CHModuleFactory.create(previous_answers=[['24.0', [0, 3, None]]]) + mock_module = CHModuleFactory.create(previous_answers=[['24.0', [0, 3, None]]]) json_in = {'problem_name': '42.5'} - out = m.get_feedback(json_in) + out = mock_module.get_feedback(json_in) print out['index_to_hints'] self.assertTrue(len(out['index_to_hints'][0]) == 2) @@ -303,10 +315,10 @@ class CrowdsourceHinterTest(unittest.TestCase): Someone gets a problem correct, but one of the hints that he saw earlier (pk=100) has been deleted. Should just skip that hint. """ - m = CHModuleFactory.create( + mock_module = CHModuleFactory.create( previous_answers=[['24.0', [0, 100, None]]]) json_in = {'problem_name': '42.5'} - out = m.get_feedback(json_in) + out = mock_module.get_feedback(json_in) self.assertTrue(len(out['index_to_hints'][0]) == 1) def test_vote_nopermission(self): @@ -314,23 +326,23 @@ class CrowdsourceHinterTest(unittest.TestCase): A user tries to vote for a hint, but he has already voted! Should not change any vote tallies. """ - m = CHModuleFactory.create(user_voted=True) + mock_module = CHModuleFactory.create(user_voted=True) json_in = {'answer': 0, 'hint': 1} - old_hints = copy.deepcopy(m.hints) - m.tally_vote(json_in) - self.assertTrue(m.hints == old_hints) + old_hints = copy.deepcopy(mock_module.hints) + mock_module.tally_vote(json_in) + self.assertTrue(mock_module.hints == old_hints) def test_vote_withpermission(self): """ A user votes for a hint. Also tests vote result rendering. """ - m = CHModuleFactory.create( + mock_module = CHModuleFactory.create( previous_answers=[['24.0', [0, 3, None]]]) json_in = {'answer': 0, 'hint': 3} - dict_out = m.tally_vote(json_in) - self.assertTrue(m.hints['24.0']['0'][1] == 40) - self.assertTrue(m.hints['24.0']['3'][1] == 31) + dict_out = mock_module.tally_vote(json_in) + self.assertTrue(mock_module.hints['24.0']['0'][1] == 40) + self.assertTrue(mock_module.hints['24.0']['3'][1] == 31) self.assertTrue(['Best hint', 40] in dict_out['hint_and_votes']) self.assertTrue(['Another hint', 31] in dict_out['hint_and_votes']) @@ -338,34 +350,34 @@ class CrowdsourceHinterTest(unittest.TestCase): """ A user tries to submit a hint, but he has already voted. """ - m = CHModuleFactory.create(user_voted=True) + mock_module = CHModuleFactory.create(user_voted=True) json_in = {'answer': 1, 'hint': 'This is a new hint.'} - print m.user_voted - m.submit_hint(json_in) - print m.hints - self.assertTrue('29.0' not in m.hints) + print mock_module.user_voted + mock_module.submit_hint(json_in) + print mock_module.hints + self.assertTrue('29.0' not in mock_module.hints) def test_submithint_withpermission_new(self): """ A user submits a hint to an answer for which no hints exist yet. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() json_in = {'answer': 1, 'hint': 'This is a new hint.'} - m.submit_hint(json_in) - self.assertTrue('29.0' in m.hints) + mock_module.submit_hint(json_in) + self.assertTrue('29.0' in mock_module.hints) def test_submithint_withpermission_existing(self): """ A user submits a hint to an answer that has other hints already. """ - m = CHModuleFactory.create(previous_answers=[['25.0', [1, None, None]]]) + mock_module = CHModuleFactory.create(previous_answers=[['25.0', [1, None, None]]]) json_in = {'answer': 0, 'hint': 'This is a new hint.'} - m.submit_hint(json_in) + mock_module.submit_hint(json_in) # Make a hint request. json_in = {'problem name': '25.0'} - out = m.get_hint(json_in) + out = mock_module.get_hint(json_in) self.assertTrue((out['best_hint'] == 'This is a new hint.') or (out['rand_hint_1'] == 'This is a new hint.')) @@ -375,27 +387,27 @@ class CrowdsourceHinterTest(unittest.TestCase): show up in the mod_queue, not the public-facing hints dict. """ - m = CHModuleFactory.create(moderate='True') + mock_module = CHModuleFactory.create(moderate='True') json_in = {'answer': 1, 'hint': 'This is a new hint.'} - m.submit_hint(json_in) - self.assertTrue('29.0' not in m.hints) - self.assertTrue('29.0' in m.mod_queue) + mock_module.submit_hint(json_in) + self.assertTrue('29.0' not in mock_module.hints) + self.assertTrue('29.0' in mock_module.mod_queue) def test_submithint_escape(self): """ Make sure that hints are being html-escaped. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() json_in = {'answer': 1, 'hint': ''} - m.submit_hint(json_in) - print m.hints - self.assertTrue(m.hints['29.0'][0][0] == u'<script> alert("Trololo"); </script>') + mock_module.submit_hint(json_in) + print mock_module.hints + self.assertTrue(mock_module.hints['29.0'][0][0] == u'<script> alert("Trololo"); </script>') def test_template_gethint(self): """ Test the templates for get_hint. """ - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() def fake_get_hint(get): """ @@ -407,9 +419,9 @@ class CrowdsourceHinterTest(unittest.TestCase): 'rand_hint_2': 'Another random hint', 'answer': '42.5'} - m.get_hint = fake_get_hint + mock_module.get_hint = fake_get_hint json_in = {'problem_name': '42.5'} - out = json.loads(m.handle_ajax('get_hint', json_in))['contents'] + out = json.loads(mock_module.handle_ajax('get_hint', json_in))['contents'] self.assertTrue('This is the best hint.' in out) self.assertTrue('A random hint' in out) self.assertTrue('Another random hint' in out) @@ -420,7 +432,7 @@ class CrowdsourceHinterTest(unittest.TestCase): NOT FINISHED from lxml import etree - m = CHModuleFactory.create() + mock_module = CHModuleFactory.create() def fake_get_feedback(get): index_to_answer = {'0': '42.0', '1': '9000.01'} @@ -429,9 +441,9 @@ class CrowdsourceHinterTest(unittest.TestCase): '1': [('A hint for 9000.01', 32)]} return {'index_to_hints': index_to_hints, 'index_to_answer': index_to_answer} - m.get_feedback = fake_get_feedback + mock_module.get_feedback = fake_get_feedback json_in = {'problem_name': '42.5'} - out = json.loads(m.handle_ajax('get_feedback', json_in))['contents'] + out = json.loads(mock_module.handle_ajax('get_feedback', json_in))['contents'] html_tree = etree.XML(out) # To be continued... diff --git a/common/templates/hinter_display.html b/common/templates/hinter_display.html index bc49bf18bd..6f5d6f37fb 100644 --- a/common/templates/hinter_display.html +++ b/common/templates/hinter_display.html @@ -95,13 +95,17 @@ What would you say to help someone who got this wrong answer? <%def name="show_votes()"> - Thank you for voting! -
- % for hint, votes in hint_and_votes: - ${votes} votes. - ${hint} -
- % endfor + % if hint_and_votes is UNDEFINED: + Sorry, but you've already voted! + % else: + Thank you for voting! +
+ % for hint, votes in hint_and_votes: + ${votes} votes. + ${hint} +
+ % endfor + % endif <%def name="simple_message()"> @@ -123,3 +127,4 @@ What would you say to help someone who got this wrong answer? % if op == "vote": ${show_votes()} % endif +