From 09d3ca0ab6b9ed68a8a3e4f914a50903f6bd8b07 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Tue, 2 Jul 2013 09:40:47 -0400 Subject: [PATCH 1/6] Fixed bug when user votes w/o permission - now displays a friendly error message instead of failing. Fixed bug when hinter module displays a hint, then is asked to display nothing. (Used to not update in this case.) --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 2 +- .../js/src/crowdsource_hinter/display.coffee | 2 ++ common/templates/hinter_display.html | 19 ++++++++++++------- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index f84b366d2c..5b9c0a1899 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -227,7 +227,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): Returns key 'hint_and_votes', a list of (hint_text, #votes) pairs. """ if self.user_voted: - return json.dumps({'contents': 'Sorry, but you have already voted!'}) + return {} ans_no = int(get['answer']) hint_no = str(get['hint']) answer = self.previous_answers[ans_no][0] diff --git a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee index f8bc6037db..72522f5b03 100644 --- a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -72,3 +72,5 @@ class @Hinter JavascriptLoader.executeModuleScripts @el, () => @bind() @$('#previous-answer-0').css('display', 'inline') + else + @el.hide() 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 + From b68c11785467eeff57f7846cacaa2e50bd32a51b Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Wed, 3 Jul 2013 10:17:10 -0400 Subject: [PATCH 2/6] Fixed bug: When student submits empty string, hinter returns a 500 error. Added tests to catch said bug. Fixed some pylint violations. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 6 +- .../xmodule/tests/test_crowdsource_hinter.py | 124 ++++++++++-------- 2 files changed, 73 insertions(+), 57 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 5b9c0a1899..a776e2da60 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -140,7 +140,11 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): - 'rand_hint_1' and 'rand_hint_2' are two random hints to the answer in `get`. - 'answer' is the parsed answer that was submitted. """ - answer = self.capa_answer_to_str(get) + try: + answer = self.capa_answer_to_str(get) + except ValueError: + # Sometimes, we get an answer that's just not parsable. Do nothing. + 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.) 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... From bc1dea3de1262e11aca2996223d862afab5c7ca1 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Wed, 3 Jul 2013 10:17:10 -0400 Subject: [PATCH 3/6] Fixed bug: When student submits empty string, hinter returns a 500 error. Added tests to catch said bug. Fixed some pylint violations. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 6 +- .../xmodule/tests/test_crowdsource_hinter.py | 124 ++++++++++-------- 2 files changed, 73 insertions(+), 57 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 5b9c0a1899..a776e2da60 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -140,7 +140,11 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): - 'rand_hint_1' and 'rand_hint_2' are two random hints to the answer in `get`. - 'answer' is the parsed answer that was submitted. """ - answer = self.capa_answer_to_str(get) + try: + answer = self.capa_answer_to_str(get) + except ValueError: + # Sometimes, we get an answer that's just not parsable. Do nothing. + 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.) 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... From dd8f1f5385a8ed8855ab4d235d714e2ccf7e09ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= Date: Mon, 8 Jul 2013 09:22:35 -0400 Subject: [PATCH 4/6] Rename get to data in crowdsourced hinter module --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index a776e2da60..17e6e633ab 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,22 +128,22 @@ 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. """ try: - answer = self.capa_answer_to_str(get) + answer = self.capa_answer_to_str(data) except ValueError: - # Sometimes, we get an answer that's just not parsable. Do nothing. + # Sometimes, we data an answer that's just not parsable. Do nothing. return # Look for a hint to give. # Make a local copy of self.hints - this means we only need to do one json unpacking. @@ -180,12 +180,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. @@ -220,20 +220,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 @@ -253,19 +253,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!'} From 3418feba521b59db16957bf6f1129c89feb6f5e2 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Tue, 9 Jul 2013 13:26:02 -0400 Subject: [PATCH 5/6] Added error logging for unparsable answers. --- common/lib/xmodule/xmodule/crowdsource_hinter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index a776e2da60..6b5545aab1 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -144,6 +144,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): answer = self.capa_answer_to_str(get) except ValueError: # Sometimes, we get an answer that's just not parsable. Do nothing. + log.exception('Answer not parsable: ' + str(get)) return # Look for a hint to give. # Make a local copy of self.hints - this means we only need to do one json unpacking. From 6c55726134b2531e10e73f320d17a80234c1b308 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Tue, 9 Jul 2013 14:34:21 -0400 Subject: [PATCH 6/6] Fixed naming error in crowdsource_hinter. (Introduced in git merge.) --- common/lib/xmodule/xmodule/crowdsource_hinter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 5a1b25f8e5..7c7f94a26b 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -141,10 +141,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): - 'answer' is the parsed answer that was submitted. """ try: - answer = self.capa_answer_to_str(get) + 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(get)) + 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.