Merge pull request #1623 from MITx/fix/will/checkbox_bug

Fixed checkbox bug in capa_module

common/lib/xmodule/xmodule/capa_module.py

@@ -582,7 +582,7 @@ class CapaModule(XModule):
     @staticmethod
     def make_dict_of_responses(get):
         '''Make dictionary of student responses (aka "answers")
-        get is POST dictionary.
+        get is POST dictionary (Djano QueryDict).
 
         The *get* dict has keys of the form 'x_y', which are mapped
         to key 'y' in the returned dict.  For example,
@@ -606,6 +606,7 @@ class CapaModule(XModule):
             to 'input_1' in the returned dict)
         '''
         answers = dict()
+        
         for key in get:
             # e.g. input_resistor_1 ==> resistor_1
             _, _, name = key.partition('_')
@@ -613,7 +614,7 @@ class CapaModule(XModule):
             # If key has no underscores, then partition
             # will return (key, '', '')
             # We detect this and raise an error
-            if name is '':
+            if not name:
                 raise ValueError("%s must contain at least one underscore" % str(key))
 
             else:
@@ -625,10 +626,7 @@ class CapaModule(XModule):
                 name = name[:-2] if is_list_key else name
 
                 if is_list_key:
-                    if type(get[key]) is list:
-                        val = get[key]
-                    else:
-                        val = [get[key]]
+                    val = get.getlist(key)
                 else:
                     val = get[key]
 

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -11,6 +11,8 @@ from xmodule.capa_module import CapaModule
 from xmodule.modulestore import Location
 from lxml import etree
 
+from django.http import QueryDict
+
 from . import test_system
 
 
@@ -326,14 +328,18 @@ class CapaModuleTest(unittest.TestCase):
 
     def test_parse_get_params(self):
 
+        # We have to set up Django settings in order to use QueryDict
+        from django.conf import settings
+        settings.configure()
+
         # Valid GET param dict
-        valid_get_dict = {'input_1': 'test',
-                        'input_1_2': 'test',
-                        'input_1_2_3': 'test',
-                        'input_[]_3': 'test',
-                        'input_4': None,
-                        'input_5': [],
-                        'input_6': 5}
+        valid_get_dict = self._querydict_from_dict({'input_1': 'test',
+                                                    'input_1_2': 'test',
+                                                    'input_1_2_3': 'test',
+                                                    'input_[]_3': 'test',
+                                                    'input_4': None,
+                                                    'input_5': [],
+                                                    'input_6': 5})
 
         result = CapaModule.make_dict_of_responses(valid_get_dict)
 
@@ -347,20 +353,19 @@ class CapaModuleTest(unittest.TestCase):
 
 
         # Valid GET param dict with list keys
-        valid_get_dict = {'input_2[]': ['test1', 'test2']}
+        valid_get_dict = self._querydict_from_dict({'input_2[]': ['test1', 'test2']})
         result = CapaModule.make_dict_of_responses(valid_get_dict)
         self.assertTrue('2' in result)
-        self.assertEqual(valid_get_dict['input_2[]'], result['2'])
+        self.assertEqual(['test1','test2'], result['2'])
 
         # If we use [] at the end of a key name, we should always
         # get a list, even if there's just one value
-        valid_get_dict = {'input_1[]': 'test'}
+        valid_get_dict = self._querydict_from_dict({'input_1[]': 'test'})
         result = CapaModule.make_dict_of_responses(valid_get_dict)
         self.assertEqual(result['1'], ['test'])
 
-
         # If we have no underscores in the name, then the key is invalid
-        invalid_get_dict = {'input': 'test'}
+        invalid_get_dict = self._querydict_from_dict({'input': 'test'})
         with self.assertRaises(ValueError):
             result = CapaModule.make_dict_of_responses(invalid_get_dict)
 
@@ -368,11 +373,32 @@ class CapaModuleTest(unittest.TestCase):
         # Two equivalent names (one list, one non-list)
         # One of the values would overwrite the other, so detect this
         # and raise an exception
-        invalid_get_dict = {'input_1[]': 'test 1',
-                            'input_1': 'test 2' }
+        invalid_get_dict = self._querydict_from_dict({'input_1[]': 'test 1',
+                                                    'input_1': 'test 2' })
         with self.assertRaises(ValueError):
             result = CapaModule.make_dict_of_responses(invalid_get_dict)
 
+    def _querydict_from_dict(self, param_dict):
+        """ Create a Django QueryDict from a Python dictionary """
+
+        # QueryDict objects are immutable by default, so we make
+        # a copy that we can update.
+        querydict = QueryDict('')
+        copyDict = querydict.copy()
+
+        for (key, val) in param_dict.items():
+
+            # QueryDicts handle lists differently from ordinary values,
+            # so we have to specifically tell the QueryDict that
+            # this is a list
+            if type(val) is list:
+                copyDict.setlist(key, val)
+            else:
+                copyDict[key] = val
+
+        return copyDict
+
+
     def test_check_problem_correct(self):
 
         module = CapaFactory.create(attempts=1)