Fixed numerous 500 errors that result from mal-formatted post requests.

Fixed tests to work with symbolic answer changes.

common/lib/xmodule/xmodule/crowdsource_hinter.py

@@ -173,6 +173,9 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
 
         if out is None:
             out = {'op': 'empty'}
+        elif 'error' in out:
+            # Error in processing.
+            out.update({'op': 'error'})
         else:
             out.update({'op': dispatch})
         return json.dumps({'contents': self.system.render_template('hinter_display.html', out)})
@@ -288,14 +291,23 @@ 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 {'error': 'Sorry, but you have already voted!'}
         ans = data['answer']
         signature = self.answer_signature(ans)
+        if signature is None:
+            # Uh oh.  Invalid answer.
+            log.exception('Failure in hinter tally_vote: Unable to parse answer: ' + ans)
+            return {'error': 'Failure in voting!'}
         hint_pk = str(data['hint'])
         pk_list = json.loads(data['pk_list'])
         # We use temp_dict because we need to do a direct write for the database to update.
         temp_dict = self.hints
-        temp_dict[signature][hint_pk][1] += 1
+        try:
+            temp_dict[signature][hint_pk][1] += 1
+        except KeyError:
+            log.exception('Failure in hinter tally_vote: User voted for non-existant hint: Answer=' +
+                          ans + ' pk=' + hint_pk)
+            return {'error': 'Failure in voting!'}
         self.hints = temp_dict
         # Don't let the user vote again!
         self.user_voted = True
@@ -303,7 +315,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
         # Return a list of how many votes each hint got.
         hint_and_votes = []
         for vote_pk in pk_list:
-            hint_and_votes.append(temp_dict[signature][str(vote_pk)])
+            try:
+                hint_and_votes.append(temp_dict[signature][str(vote_pk)])
+            except KeyError:
+                log.exception('In hinter tally_vote: pk_list contains non-existant pk: ' + str(vote_pk))
 
         hint_and_votes.sort(key=lambda pair: pair[1], reverse=True)
         # Reset self.previous_answers.
@@ -324,6 +339,9 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
         hint = escape(data['hint'])
         answer = data['answer']
         signature = self.answer_signature(answer)
+        if signature is None:
+            log.exception('Failure in hinter submit_hint: Unable to parse answer: ' + answer)
+            return {'error': 'Could not submit answer'}
         # Only allow a student to vote or submit a hint once.
         if self.user_voted:
             return {'message': 'Sorry, but you have already voted!'}

common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py

@@ -2,13 +2,15 @@
 Tests the crowdsourced hinter xmodule.
 """
 
-from mock import Mock
+from mock import Mock, MagicMock
 import unittest
 import copy
 
 from xmodule.crowdsource_hinter import CrowdsourceHinterModule
 from xmodule.vertical_module import VerticalModule, VerticalDescriptor
 
+from capa.responsetypes import StudentInputError
+
 from . import get_test_system
 
 import json
@@ -94,12 +96,54 @@ class CHModuleFactory(object):
         if moderate is not None:
             model_data['moderate'] = moderate
 
-        descriptor = Mock(weight="1")
+        descriptor = Mock(weight='1')
+        # Make the descriptor have a capa problem child.
+        capa_descriptor = MagicMock()
+        capa_descriptor.name = 'capa'
+        descriptor.get_children = lambda: [capa_descriptor]
+
+        # Make a fake capa module.
+        capa_module = MagicMock()
+        capa_module.responders = {'responder0': MagicMock()}
+        capa_module.displayable_items = lambda: [capa_module]
+
         system = get_test_system()
+        # Make the system have a marginally-functional get_module
+
+        def fake_get_module(descriptor):
+            """
+            A fake module-maker.
+            """
+            if descriptor.name == 'capa':
+                return capa_module
+        system.get_module = fake_get_module
+
         module = CrowdsourceHinterModule(system, descriptor, model_data)
 
         return module
 
+    @staticmethod
+    def setup_formula_response(module):
+        """
+        Adds additional mock methods to the module, so that we can test
+        formula responses.
+        """
+        # Do a bunch of monkey patching, to mock the lon-capa problem.
+        responder = MagicMock()
+        responder.randomize_variables = MagicMock(return_value=4)
+
+        def fake_hash_answers(answer, test_values):
+            """ A fake answer hasher """
+            if test_values == 4 and answer == 'x*y^2':
+                return 'good'
+            raise StudentInputError
+
+        responder.hash_answers = fake_hash_answers
+        lcp = MagicMock()
+        lcp.responders = {'responder0': responder}
+        module.get_display_items()[0].lcp = lcp
+        return module
+
 
 class VerticalWithModulesFactory(object):
     """
@@ -226,6 +270,44 @@ class CrowdsourceHinterTest(unittest.TestCase):
         self.assertTrue('Test numerical problem.' in out_html)
         self.assertTrue('Another test numerical problem.' in out_html)
 
+    def test_numerical_answer_to_str(self):
+        """
+        Tests the get request to string converter for numerical responses.
+        """
+        mock_module = CHModuleFactory.create()
+        get = {'response1': '4'}
+        parsed = mock_module.numerical_answer_to_str(get)
+        self.assertTrue(parsed == '4.0')
+
+    def test_formula_answer_to_str(self):
+        """
+        Tests the get request to string converter for formula responses.
+        """
+        mock_module = CHModuleFactory.create()
+        get = {'response1': 'x*y^2'}
+        parsed = mock_module.formula_answer_to_str(get)
+        self.assertTrue(parsed == 'x*y^2')
+
+    def test_formula_answer_signature(self):
+        """
+        Tests the answer signature generator for formula responses.
+        """
+        mock_module = CHModuleFactory.create()
+        mock_module = CHModuleFactory.setup_formula_response(mock_module)
+        answer = 'x*y^2'
+        out = mock_module.formula_answer_signature(answer)
+        self.assertTrue(out == 'good')
+
+    def test_formula_answer_signature_failure(self):
+        """
+        Makes sure that bad answer strings return None as a signature.
+        """
+        mock_module = CHModuleFactory.create()
+        mock_module = CHModuleFactory.setup_formula_response(mock_module)
+        answer = 'fish'
+        out = mock_module.formula_answer_signature(answer)
+        self.assertTrue(out is None)
+
     def test_gethint_0hint(self):
         """
         Someone asks for a hint, when there's no hint to give.
@@ -343,6 +425,44 @@ class CrowdsourceHinterTest(unittest.TestCase):
         self.assertTrue(['Best hint', 40] in dict_out['hint_and_votes'])
         self.assertTrue(['Another hint', 31] in dict_out['hint_and_votes'])
 
+    def test_vote_unparsable(self):
+        """
+        A user somehow votes for an unparsable answer.
+        Should return a friendly error.
+        (This is an unusual exception path - I don't know how it occurs,
+        except if you manually make a post request.  But, it seems to happen
+        occasionally.)
+        """
+        mock_module = CHModuleFactory.create()
+        # None means that the answer couldn't be parsed.
+        mock_module.answer_signature = lambda text: None
+        json_in = {'answer': 'fish', 'hint': 3, 'pk_list': '[]'}
+        dict_out = mock_module.tally_vote(json_in)
+        print dict_out
+        self.assertTrue(dict_out == {'error': 'Failure in voting!'})
+
+    def test_vote_nohint(self):
+        """
+        A user somehow votes for a hint that doesn't exist.
+        Should return a friendly error.
+        """
+        mock_module = CHModuleFactory.create()
+        json_in = {'answer': '24.0', 'hint': '25', 'pk_list': '[]'}
+        dict_out = mock_module.tally_vote(json_in)
+        self.assertTrue(dict_out == {'error': 'Failure in voting!'})
+
+    def test_vote_badpklist(self):
+        """
+        Some of the pk's specified in pk_list are invalid.
+        Should just skip those.
+        """
+        mock_module = CHModuleFactory.create()
+        json_in = {'answer': '24.0', 'hint': '0', 'pk_list': json.dumps([0, 12])}
+        hint_and_votes = mock_module.tally_vote(json_in)['hint_and_votes']
+        self.assertTrue(['Best hint', 41] in hint_and_votes)
+        self.assertTrue(len(hint_and_votes) == 1)
+
+
     def test_submithint_nopermission(self):
         """
         A user tries to submit a hint, but he has already voted.
@@ -399,6 +519,17 @@ class CrowdsourceHinterTest(unittest.TestCase):
         mock_module.submit_hint(json_in)
         self.assertTrue(mock_module.hints['29.0']['0'][0] == u'<script> alert("Trololo"); </script>')
 
+    def test_submithint_unparsable(self):
+        mock_module = CHModuleFactory.create()
+        mock_module.answer_signature = lambda text: None
+        json_in = {'answer': 'fish', 'hint': 'A hint'}
+        dict_out = mock_module.submit_hint(json_in)
+        print dict_out
+        print mock_module.hints
+        self.assertTrue('error' in dict_out)
+        self.assertTrue(None not in mock_module.hints)
+        self.assertTrue('fish' not in mock_module.hints)
+
     def test_template_gethint(self):
         """
         Test the templates for get_hint.

common/templates/hinter_display.html

@@ -137,6 +137,10 @@ What would you say to help someone who got this wrong answer?
     ${simple_message()}
 % endif
 
+% if op == "error":
+    ${error}
+% endif
+
 % if op == "vote":
     ${show_votes()}
 % endif