Teach OEE to consider old task_states when trying to recover from an xml mismatch

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

 import json
 import logging
+import traceback
 from lxml import etree
 from xmodule.timeinfo import TimeInfo
 from xmodule.capa_module import ComplexEncoder
@@ -172,27 +173,18 @@ class CombinedOpenEndedV1Module():
         self.fix_invalid_state()
         self.setup_next_task()
 
-    def fix_invalid_state(self):
-        """
-        Sometimes a teacher will change the xml definition of a problem in Studio.
-        This means that the state passed to the module is invalid.
-        If that is the case, moved it to old_task_states and delete task_states.
+    def validate_task_states(self, tasks_xml, task_states):
         """
+        Check whether the provided task_states are valid for the supplied task_xml.
 
-        # 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 = 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 = len(self.task_xml) - 1
-            self.task_states = self.task_states[:len(self.task_xml)]
+        Returns a list of messages indicating what is invalid about the state.
+        If the list is empty, then the state is valid
+        """
+        msgs = []
         #Loop through each task state and make sure it matches the xml definition
-        for (i, t) in enumerate(self.task_states):
-            tag_name = self.get_tag_name(self.task_xml[i])
+        for task_xml, task_state in zip(tasks_xml, task_states):
+            tag_name = self.get_tag_name(task_xml)
             children = self.child_modules()
-            task_xml = self.task_xml[i]
             task_descriptor = children['descriptors'][tag_name](self.system)
             task_parsed_xml = task_descriptor.definition_from_xml(etree.fromstring(task_xml), self.system)
             try:
@@ -202,30 +194,156 @@ class CombinedOpenEndedV1Module():
                     task_parsed_xml,
                     task_descriptor,
                     self.static_data,
-                    instance_state=t,
+                    instance_state=task_state,
                 )
                 #Loop through each attempt of the task and see if it is valid.
-                for att in task.child_history:
-                    if "post_assessment" not in att:
+                for attempt in task.child_history:
+                    if "post_assessment" not in attempt:
                         continue
-                    pa = att['post_assessment']
+                    post_assessment = attempt['post_assessment']
                     try:
-                        pa = json.loads(pa)
+                        post_assessment = json.loads(post_assessment)
                     except ValueError:
                         #This is okay, the value may or may not be json encoded.
                         pass
-                    if tag_name == "openended" and isinstance(pa, list):
-                        self.reset_task_state("Type is open ended and post assessment is a list.")
+                    if tag_name == "openended" and isinstance(post_assessment, list):
+                        msgs.append("Type is open ended and post assessment is a list.")
                         break
-                    elif tag_name == "selfassessment" and not isinstance(pa, list):
-                        self.reset_task_state("Type is self assessment and post assessment is not a list.")
+                    elif tag_name == "selfassessment" and not isinstance(post_assessment, list):
+                        msgs.append("Type is self assessment and post assessment is not a list.")
                         break
                 #See if we can properly render the task.  Will go into the exception clause below if not.
                 task.get_html(self.system)
-            except Exception as err:
+            except Exception:
                 #If one task doesn't match, the state is invalid.
-                self.reset_task_state("Could not parse task. {0}".format(err))
+                msgs.append("Could not parse task with xml {xml!r} and states {state!r}: {err}".format(
+                    xml=task_xml,
+                    state=task_state,
+                    err=traceback.format_exc()
+                ))
                 break
+        return msgs
+
+    def is_initial_child_state(self, task_child):
+        """
+        Returns true if this is a child task in an initial configuration
+        """
+        task_child = json.loads(task_child)
+        return (
+            task_child['child_state'] == self.INITIAL and
+            task_child['child_history'] == []
+        )
+
+    def is_reset_task_states(self, task_state):
+        """
+        Returns True if this task_state is from something that was just reset
+        """
+        return all(self.is_initial_child_state(child) for child in task_state)
+
+
+    def states_sort_key(self, idx_task_states):
+        """
+        Return a key for sorting a list of indexed task_states, by how far the student got
+        through the tasks, what their highest score was, and then the index of the submission.
+        """
+        idx, task_states = idx_task_states
+
+        state_values = {
+            self.INITIAL: 0,
+            self.ASSESSING: 1,
+            self.INTERMEDIATE_DONE: 2,
+            self.DONE: 3
+        }
+
+        if not task_states:
+            return (0, 0, state_values[self.INITITIAL], idx)
+
+        final_child_state = json.loads(task_states[-1])
+        best_score = max(attempt.get('score', 0) for attempt in final_child_state.get('child_history', []))
+        return (
+            len(task_states),
+            best_score,
+            state_values[final_child_state.get('child_state', self.INITIAL)],
+            idx
+        )
+
+    def fix_invalid_state(self):
+        """
+        Sometimes a teacher will change the xml definition of a problem in Studio.
+        This means that the state passed to the module is invalid.
+        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 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 = 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 = len(self.task_xml) - 1
+            self.task_states = self.task_states[:len(self.task_xml)]
+
+        if not self.old_task_states and not self.task_states:
+            # No validation needed when a student first looks at the problem
+            return
+
+        # Pick out of self.task_states and self.old_task_states the state that is
+        # a) valid for the current task definition
+        # b) not the result of a reset due to not having a valid task state
+        # c) has the highest total score
+        # d) is the most recent (if the other two conditions are met)
+
+        valid_states = [
+            task_states
+            for task_states
+            in self.old_task_states + [self.task_states]
+            if (
+                len(self.validate_task_states(self.task_xml, task_states)) == 0 and
+                not self.is_reset_task_states(task_states)
+            )
+        ]
+
+        # If there are no valid states, don't try and use an old state
+        if len(valid_states) == 0:
+            # If this isn't an initial task state, then reset to an initial state
+            if not self.is_reset_task_states(self.task_states):
+                self.reset_task_state('\n'.join(self.validate_task_states(self.task_xml, self.task_states)))
+
+            return
+
+        sorted_states = sorted(enumerate(valid_states), key=self.states_sort_key, reverse=True)
+        idx, best_task_states = sorted_states[0]
+
+        if best_task_states == self.task_states:
+            return
+
+        log.warning(
+            "Updating current task state for %s to %r for student with anonymous id %r",
+            self.system.location,
+            best_task_states,
+            self.system.anonymous_student_id
+        )
+
+        self.old_task_states.remove(best_task_states)
+        self.old_task_states.append(self.task_states)
+        self.task_states = best_task_states
+
+        # The state is ASSESSING unless all of the children are done, or all
+        # of the children haven't been started yet
+        children = [json.loads(child) for child in best_task_states]
+        if all(child['child_state'] == self.DONE for child in children):
+            self.state = self.DONE
+        elif all(child['child_state'] == self.INITIAL for child in children):
+            self.state = self.INITIAL
+        else:
+            self.state = self.ASSESSING
+
+        # The current task number is the index of the last completed child + 1,
+        # limited by the number of tasks
+        last_completed_child = next((i for i, child in reversed(list(enumerate(children))) if child['child_state'] == self.DONE), 0)
+        self.current_task_number = min(last_completed_child + 1, len(best_task_states) - 1)
+
 
     def reset_task_state(self, message=""):
         """