Address review comments

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -186,7 +186,7 @@ class CombinedOpenEndedFields(object):
     old_task_states = List(
         help=("A list of lists of state dictionaries for student states that are saved."
                "This field is only populated if the instructor changes tasks after"
-               "the module is created and students have attempted it (ie changes a self assessed problem to "
+               "the module is created and students have attempted it (for example changes a self assessed problem to "
                "self and peer assessed."),
         scope = Scope.user_state
     )

common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py

@@ -173,13 +173,14 @@ class CombinedOpenEndedV1Module():
         If that is the case, moved it to old_task_states and delete task_states.
         """
 
-        #If we are on a task that is greater than the number of available tasks, it is an invalid state
-        #If the current task number is greater than the number of tasks we have in the xml definition, our state is invalid.
+        # If we are on a task that is greater than the number of available tasks,
+        # it is an invalid state. If the current task number is greater than the number of tasks
+        # we have in the definition, our state is invalid.
         if self.current_task_number > len(self.task_states) or self.current_task_number > len(self.task_xml):
-            self.current_task_number = min([len(self.task_states), len(self.task_xml)]) - 1
+            self.current_task_number = max(min(len(self.task_states), len(self.task_xml)) - 1, 0)
         #If the length of the task xml is less than the length of the task states, state is invalid
         if len(self.task_xml) < len(self.task_states):
-            self.current_task_number = 0
+            self.current_task_number = len(self.task_xml) - 1
             self.task_states = self.task_states[:len(self.task_xml)]
         #Loop through each task state and make sure it matches the xml definition
         for (i, t) in enumerate(self.task_states):
@@ -221,8 +222,14 @@ class CombinedOpenEndedV1Module():
                 break
 
     def reset_task_state(self, message=""):
-        info_message = "Combined open ended user state for user {0} in location {1} was invalid.  Reset it. {2}".format(self.system.anonymous_student_id, self.location.url(), message)
+        """
+        Resets the task states.  Moves current task state to an old_state variable, and then makes the task number 0.
+        :param message: A message to put in the log.
+        :return: None
+        """
+        info_message = "Combined open ended user state for user {0} in location {1} was invalid.  It has been reset, and you now have a new attempt. {2}".format(self.system.anonymous_student_id, self.location.url(), message)
         self.current_task_number = 0
+        self.student_attempts = 0
         self.old_task_states.append(self.task_states)
         self.task_states = []
         log.info(info_message)

common/lib/xmodule/xmodule/tests/test_combined_open_ended.py

@@ -572,9 +572,9 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
         self.assertEqual(score_dict['score'], 15.0)
         self.assertEqual(score_dict['total'], 15.0)
 
-    def ai_state(self, task_state, task_number, task_xml):
+    def generate_oe_module(self, task_state, task_number, task_xml):
         """
-        See if state is properly reset or left unchanged
+        Return a combined open ended module with the specified parameters
         """
         definition = {'prompt': etree.XML(self.prompt), 'rubric': etree.XML(self.rubric),
                       'task_xml': task_xml}
@@ -595,17 +595,23 @@ class CombinedOpenEndedModuleTest(unittest.TestCase):
         """
         See if state is properly reset
         """
-        combinedoe = self.ai_state(task_state, task_number, [self.task_xml2])
+        combinedoe = self.generate_oe_module(task_state, task_number, [self.task_xml2])
         html = combinedoe.get_html()
         self.assertIsInstance(html, basestring)
 
+        score = combinedoe.get_score()
+        if combinedoe.is_scored:
+            self.assertEqual(score['score'], 0)
+        else:
+            self.assertEqual(score['score'], None)
+
     def ai_state_success(self, task_state, task_number=None, iscore=2, tasks=None):
         """
         See if state stays the same
         """
         if tasks is None:
             tasks = [self.task_xml1, self.task_xml2]
-        combinedoe = self.ai_state(task_state, task_number, tasks)
+        combinedoe = self.generate_oe_module(task_state, task_number, tasks)
         html = combinedoe.get_html()
         self.assertIsInstance(html, basestring)
         score = combinedoe.get_score()