Revert "Fix bug with multiple responses per problem"

This reverts commit f1151eeb.

common/lib/capa/capa/responsetypes.py

@@ -826,8 +826,8 @@ class MultipleChoiceResponse(LoncapaResponse):
         using the regular (non-masked) names.
         Fails loudly if called on a response that is not masking.
         """
-        choices = self.xml.xpath('choicegroup/choice')
-        return [self.unmask_name(choice.get("name")) for choice in choices]
+        choicegroups = self.xml.xpath('//choicegroup')
+        return [self.unmask_name(choice.get("name")) for choice in choicegroups[0].getchildren()]
 
     def do_shuffle(self, tree):
         """
@@ -835,12 +835,10 @@ class MultipleChoiceResponse(LoncapaResponse):
         based on the seed. Otherwise does nothing.
         Does nothing if the tree has already been processed.
         """
-        # The tree is already pared down to this <multichoiceresponse> so this query just
-        # gets the child choicegroup (i.e. no leading //)
-        choicegroups = tree.xpath('choicegroup[@shuffle="true"]')
+        choicegroups = tree.xpath('//choicegroup[@shuffle="true"]')
         if choicegroups:
             if len(choicegroups) > 1:
-                raise LoncapaProblemError('We support one shuffled choicegroup per response')
+                raise LoncapaProblemError('We support at most one shuffled choicegroup')
             choicegroup = choicegroups[0]
             # Note that this tree has been processed.
             if choicegroup.get('shuffle-done') is not None:
@@ -893,16 +891,11 @@ class MultipleChoiceResponse(LoncapaResponse):
         pool size. If that attribute is zero or not present, no operation is performed.
         Calling this a second time does nothing.
         """
-        choicegroups = tree.xpath("choicegroup[@answer-pool]")
+        choicegroups = tree.xpath("//choicegroup[@answer-pool]")
 
         # Uses self.seed -- but want to randomize every time reaches this problem,
         # so problem's "randomization" should be set to "always"
         rnd = random.Random(self.context['seed'])
-        
-        # TODO: currently, each choicegroup uses the seed from the problem.
-        # This has the unfortunate effect of making choicegroups look similar
-        # when we have many in one problem. Could perturb the rnd a little
-        # per choicegroup so they look different.
 
         for choicegroup in choicegroups:
             num_str = choicegroup.get('answer-pool')

common/lib/capa/capa/tests/test_answer_pool.py

@@ -73,7 +73,7 @@ class CapaAnswerPoolTest(unittest.TestCase):
 
             <p>What is the correct answer?</p>
             <multiplechoiceresponse>
-              <choicegroup type="MultipleChoice" answer-pool="4 ">
+              <choicegroup type="MultipleChoice" answer-pool="4">
                 <choice correct="false">wrong-1</choice>
                 <choice correct="false">wrong-2</choice>
                 <choice correct="true" explanation-id="solution1">correct-1</choice>
@@ -361,11 +361,9 @@ class CapaAnswerPoolTest(unittest.TestCase):
 
         problem = new_loncapa_problem(xml_str, seed=723)
         the_html = problem.get_html()
-        print the_html
 
         str1 = r"<div>.*\[.*'wrong-3'.*'correct-2'.*'wrong-2'.*'wrong-4'.*\].*</div>"
-        str2 = r"<div>.*\[.*'correct-2'.*'wrong-2'.*'wrong-3'.*\].*</div>"
-
+        str2 = r"<div>.*\[.*'wrong-2'.*'wrong-1'.*'correct-2'.*\].*</div>"
         str3 = r"<div>\{.*'1_solution_2'.*\}</div>"
         str4 = r"<div>\{.*'1_solution_4'.*\}</div>"
 
@@ -449,9 +447,9 @@ class CapaAnswerPoolTest(unittest.TestCase):
         the_html = problem.get_html()
 
         str1 = r"<div>.*\[.*'wrong-4'.*'wrong-3'.*'correct-1'.*\].*</div>"
-        str2 = r"<div>.*\[.*'wrong-1'.*'wrong-4'.*'wrong-3'.*'correct-1'.*\].*</div>"
+        str2 = r"<div>.*\[.*'wrong-2'.*'wrong-3'.*'wrong-4'.*'correct-2'.*\].*</div>"
         str3 = r"<div>\{.*'1_solution_1'.*\}</div>"
-        str4 = r"<div>\{.*'1_solution_3'.*\}</div>"
+        str4 = r"<div>\{.*'1_solution_4'.*\}</div>"
 
         self.assertRegexpMatches(the_html, str1)
         self.assertRegexpMatches(the_html, str2)

common/lib/capa/capa/tests/test_shuffle.py

@@ -245,37 +245,3 @@ class CapaShuffleTest(unittest.TestCase):
         problem = new_loncapa_problem(xml_str, seed=0)
         the_html = problem.get_html()
         self.assertRegexpMatches(the_html, r"<div>.*\[.*'A'.*'Mid'.*'Mid'.*'C'.*'D'.*\].*</div>")
-
-    def test_multiple_shuffle_responses(self):
-        xml_str = textwrap.dedent("""
-            <problem>
-            <multiplechoiceresponse>
-              <choicegroup type="MultipleChoice" shuffle="true">
-                <choice correct="false">Apple</choice>
-                <choice correct="false">Banana</choice>
-                <choice correct="false">Chocolate</choice>
-                <choice correct ="true">Donut</choice>
-              </choicegroup>
-            </multiplechoiceresponse>
-            <p>Here is some text</p>
-            <multiplechoiceresponse>
-              <choicegroup type="MultipleChoice" shuffle="true">
-                <choice correct="false">A</choice>
-                <choice correct="false">B</choice>
-                <choice correct="false">C</choice>
-                <choice correct ="true">D</choice>
-              </choicegroup>
-            </multiplechoiceresponse>
-            </problem>
-        """)
-        problem = new_loncapa_problem(xml_str, seed=0)
-        orig_html = problem.get_html()
-        self.assertEqual(orig_html, problem.get_html(), 'should be able to call get_html() twice')
-        html = orig_html.replace('\n', ' ')  # avoid headaches with .* matching
-        self.assertRegexpMatches(html, r"<div>.*\[.*'Banana'.*'Apple'.*'Chocolate'.*'Donut'.*\].*</div>.*" +
-                                       r"<div>.*\[.*'B'.*'A'.*'C'.*'D'.*\].*</div>")
-        responses = problem.responders.values()
-        self.assertTrue(hasattr(responses[0], 'is_masked'))
-        self.assertTrue(hasattr(responses[1], 'is_masked'))
-        self.assertEqual(responses[0].unmask_order(), ['choice_1', 'choice_0', 'choice_2', 'choice_3'])
-        self.assertEqual(responses[1].unmask_order(), ['choice_1', 'choice_0', 'choice_2', 'choice_3'])

common/lib/xmodule/xmodule/capa_base.py

@@ -1055,9 +1055,7 @@ class CapaMixin(CapaFields):
                     event_info['state']['student_answers'][response.answer_id] = response.unmask_name(answer)
 
                 # 3. Record the shuffled ordering
-                if not 'display_order' in event_info:
-                    event_info['display_order'] = {}
-                event_info['display_order'][response.answer_id] = response.unmask_order()
+                event_info['display_order'] = {response.answer_id: response.unmask_order()}
 
     def pretty_print_seconds(self, num_seconds):
         """