[SOL-1980] Review notes

drag_and_drop_v2/drag_and_drop_v2.py

@@ -343,7 +343,9 @@ def do_attempt(self, data, suffix=''):
         """
         Checks submitted solution and returns feedback.
 
-        Raises 400 error in standard mode.
+        Raises:
+             * JsonHandlerError with 400 error code in standard mode.
+             * JsonHandlerError with 409 error code if no more attempts left
         """
         self._validate_do_attempt()
 
@@ -524,7 +526,7 @@ def _drop_item_standard(self, item_attempt):
 
         return {
             'correct': is_correct,
-            'finished': self._is_finished(),
+            'finished': self._is_answer_correct(),
             'overall_feedback': self._present_overall_feedback(overall_feedback),
             'feedback': item_feedback
         }
@@ -568,14 +570,15 @@ def _make_state_from_attempt(attempt, correct):
 
     def _mark_complete_and_publish_grade(self):
         """
-        Helper method to update `self.comnpleted` and submit grade event if appropriate conditions met.
+        Helper method to update `self.completed` and submit grade event if appropriate conditions met.
         """
-        if not self.completed or (self.mode == self.ASSESSMENT_MODE and not self.attempts_remain):
-            self.completed = self._is_finished() or not self.attempts_remain
-            grade = self._get_grade()
-            if grade > self.grade:
-                self.grade = grade
-                self._publish_grade()
+        # There's no going back from "completed" status to "incomplete"
+        self.completed = self.completed or self._is_answer_correct() or not self.attempts_remain
+        grade = self._get_grade()
+        # ... and from higher grade to lower
+        if grade > self.grade:
+            self.grade = grade
+            self._publish_grade()
 
     def _publish_grade(self):
         """
@@ -658,7 +661,7 @@ def _get_user_state(self):
 
         overall_feedback_msgs, __ = self._get_feedback()
         if self.mode == self.STANDARD_MODE:
-            is_finished = self._is_finished()
+            is_finished = self._is_answer_correct()
         else:
             is_finished = not self.attempts_remain
 
@@ -788,9 +791,12 @@ def _answer_correctness(self):
         else:
             return self.SOLUTION_PARTIAL
 
-    def _is_finished(self):
+    def _is_answer_correct(self):
         """
         Helper - checks if answer is correct
+
+        Returns:
+            bool: True if current answer is correct
         """
         return self._answer_correctness() == self.SOLUTION_CORRECT
 

tests/integration/test_interaction.py

@@ -608,10 +608,12 @@ def test_max_attempts_reached_submit_and_reset_disabled(self):
         """
         self.place_item(0, TOP_ZONE_ID)
 
+        submit_button, reset_button = self._get_submit_button(), self._get_reset_button()
+
         for _ in xrange(self.MAX_ATTEMPTS):
+            self.assertEqual(submit_button.get_attribute('disabled'), None)
             self.click_submit()
 
-        submit_button, reset_button = self._get_submit_button(), self._get_reset_button()
         self.assertEqual(submit_button.get_attribute('disabled'), 'true')
         self.assertEqual(reset_button.get_attribute('disabled'), 'true')
 

tests/unit/test_advanced.py

@@ -180,8 +180,6 @@ class AssessmentModeFixture(BaseDragAndDropAjaxFixture):
     """
     Common tests for drag and drop in assessment mode
     """
-    CORRECT_SOLUTION = {}
-
     @staticmethod
     def _make_submission(item_id, zone_id):
         x_percent, y_percent = str(random.randint(0, 100)) + '%', str(random.randint(0, 100)) + '%'
@@ -193,8 +191,8 @@ def _submit_solution(self, solution):
             data = self._make_submission(item_id, zone_id)
             self.call_handler(self.DROP_ITEM_HANDLER, data)
 
-    def _submit_complete_solution(self):
-        self._submit_solution(self.CORRECT_SOLUTION)
+    def _submit_complete_solution(self):  # pylint: disable=no-self-use
+        raise NotImplementedError()
 
     def _submit_partial_solution(self):  # pylint: disable=no-self-use
         raise NotImplementedError()
@@ -285,9 +283,7 @@ def test_do_attempt_incorrect_publish_grade(self):
             })
 
     def test_do_attempt_post_correct_no_publish_grade(self):
-        for item_id, zone_id in self.CORRECT_SOLUTION.iteritems():
-            data = self._make_submission(item_id, zone_id)
-            self.call_handler(self.DROP_ITEM_HANDLER, data)
+        self._submit_complete_solution()
 
         self.call_handler(self.DO_ATTEMPT_HANDLER, data={})  # sets self.complete
 
@@ -364,6 +360,14 @@ def test_do_attempt_misplaced_ids(self):
             res[self.OVERALL_FEEDBACK_KEY]
         )
 
+    def test_do_attempt_shows_final_feedback_at_last_attempt(self):
+        self._set_final_attempt()
+
+        self._submit_partial_solution()
+        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+        expected_message = self._make_feedback_message(self.FINAL_FEEDBACK)
+        self.assertIn(expected_message, res[self.OVERALL_FEEDBACK_KEY])
+
     def test_get_user_state_does_not_include_correctness(self):
         self._submit_complete_solution()
         original_item_state = self.block.item_state
@@ -428,12 +432,6 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
     ZONE_1 = "zone-1"
     ZONE_2 = "zone-2"
 
-    CORRECT_SOLUTION = {
-        0: ZONE_1,
-        1: ZONE_2,
-        2: ZONE_2
-    }
-
     FEEDBACK = {
         0: {"correct": "Yes 1", "incorrect": "No 1"},
         1: {"correct": "Yes 2", "incorrect": "No 2"},
@@ -443,6 +441,9 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
     INITIAL_FEEDBACK = "This is the initial feedback."
     FINAL_FEEDBACK = "This is the final feedback."
 
+    def _submit_complete_solution(self):
+        self._submit_solution({0: self.ZONE_1, 1: self.ZONE_2, 2: self.ZONE_2})
+
     def _submit_partial_solution(self):
         self._submit_solution({0: self.ZONE_1})
         return 1.0 / 3.0
@@ -536,11 +537,3 @@ def test_do_attempt_keeps_highest_score(self):
             FeedbackMessages.MessageClasses.PARTIAL_SOLUTION
         )
         self.assertIn(expected_feedback, res[self.OVERALL_FEEDBACK_KEY])
-
-    def test_do_attempt_shows_final_feedback_at_last_attempt(self):
-        self._set_final_attempt()
-
-        self._submit_partial_solution()
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
-        expected_message = self._make_feedback_message(self.FINAL_FEEDBACK)
-        self.assertIn(expected_message, res[self.OVERALL_FEEDBACK_KEY])