Assessment mode: showing per-item feedback

* "Negative" item feedback is displayed for misplaced items when an attempt is submitted.
* Avoid showing item-related general feedback (correctly placed-misplaced-not placed) after problem reset.
* Disabled submit button while drop_item is in progress.
* Added notes and todos about implicit invocation order some methods expect.
* Item feedback popup only close when close button is hit or another item is dragged
* Translation fixes

README.md

@@ -138,8 +138,8 @@ image. You can define custom success and error feedback for each item. In
 standard mode, the feedback text is displayed in a popup after the learner drops
 the item on a zone - the success feedback is shown if the item is dropped on a
 correct zone, while the error feedback is shown when dropping the item on an
-incorrect drop zone.  In assessment mode, the success and error feedback texts
-are not used.
+incorrect drop zone.  In assessment mode, the success feedback texts
+are not used, while error feedback texts are shown when learner submits a solution.
 
 You can select any number of zones for an item to belong to using
 the checkboxes; all zones defined in the previous step are available.

drag_and_drop_v2/drag_and_drop_v2.py

@@ -246,7 +246,6 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
             "target_img_description": self.target_img_description,
             "item_background_color": self.item_background_color or None,
             "item_text_color": self.item_text_color or None,
-            "initial_feedback": self.data['feedback']['start'],
             # final feedback (data.feedback.finish) is not included - it may give away answers.
         }
 
@@ -392,17 +391,29 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         self._validate_do_attempt()
 
         self.attempts += 1
-        self._mark_complete_and_publish_grade()  # must happen before _get_feedback
+        # pylint: disable=fixme
+        # TODO: Refactor this method to "freeze" item_state and pass it to methods that need access to it.
+        # These implicit dependencies between methods exist because most of them use `item_state` or other
+        # fields, either as an "input" (i.e. read value) or as output (i.e. set value) or both. As a result,
+        # incorrect order of invocation causes issues:
+        self._mark_complete_and_publish_grade()  # must happen before _get_feedback - sets grade
+        correct = self._is_answer_correct()  # must happen before manipulating item_state - reads item_state
 
-        overall_feedback_msgs, misplaced_ids = self._get_feedback()
+        overall_feedback_msgs, misplaced_ids = self._get_feedback(include_item_feedback=True)
 
+        misplaced_items = []
         for item_id in misplaced_ids:
             del self.item_state[item_id]
+            misplaced_items.append(self._get_item_definition(int(item_id)))
+
+        feedback_msgs = [FeedbackMessage(item['feedback']['incorrect'], None) for item in misplaced_items]
 
         return {
+            'correct': correct,
             'attempts': self.attempts,
             'misplaced_items': list(misplaced_ids),
-            'overall_feedback': self._present_overall_feedback(overall_feedback_msgs)
+            'feedback': self._present_feedback(feedback_msgs),
+            'overall_feedback': self._present_feedback(overall_feedback_msgs)
         }
 
     @XBlock.json_handler
@@ -487,7 +498,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
                 self.i18n_service.gettext("Max number of attempts reached")
             )
 
-    def _get_feedback(self):
+    def _get_feedback(self, include_item_feedback=False):
         """
         Builds overall feedback for both standard and assessment modes
         """
@@ -510,17 +521,15 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
                 message = message_template(len(ids_list), self.i18n_service.ngettext)
                 feedback_msgs.append(FeedbackMessage(message, message_class))
 
+        if self.item_state or include_item_feedback:
             _add_msg_if_exists(
-            items.correctly_placed, FeedbackMessages.correctly_placed, FeedbackMessages.MessageClasses.CORRECTLY_PLACED
+                items.correctly_placed,
+                FeedbackMessages.correctly_placed,
+                FeedbackMessages.MessageClasses.CORRECTLY_PLACED
             )
             _add_msg_if_exists(misplaced_ids, FeedbackMessages.misplaced, FeedbackMessages.MessageClasses.MISPLACED)
             _add_msg_if_exists(missing_ids, FeedbackMessages.not_placed, FeedbackMessages.MessageClasses.NOT_PLACED)
 
-        if misplaced_ids and self.attempts_remain:
-            feedback_msgs.append(
-                FeedbackMessage(FeedbackMessages.MISPLACED_ITEMS_RETURNED, None)
-            )
-
         if self.attempts_remain and (misplaced_ids or missing_ids):
             problem_feedback_message = self.data['feedback']['start']
         else:
@@ -539,7 +548,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         return feedback_msgs, misplaced_ids
 
     @staticmethod
-    def _present_overall_feedback(feedback_messages):
+    def _present_feedback(feedback_messages):
         """
         Transforms feedback messages into format expected by frontend code
         """
@@ -563,14 +572,14 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         self._publish_item_dropped_event(item_attempt, is_correct)
 
         item_feedback_key = 'correct' if is_correct else 'incorrect'
-        item_feedback = item['feedback'][item_feedback_key]
+        item_feedback = FeedbackMessage(item['feedback'][item_feedback_key], None)
         overall_feedback, __ = self._get_feedback()
 
         return {
             'correct': is_correct,
             'finished': self._is_answer_correct(),
-            'overall_feedback': self._present_overall_feedback(overall_feedback),
-            'feedback': item_feedback
+            'overall_feedback': self._present_feedback(overall_feedback),
+            'feedback': self._present_feedback([item_feedback])
         }
 
     def _drop_item_assessment(self, item_attempt):
@@ -612,6 +621,18 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         """
         Helper method to update `self.completed` and submit grade event if appropriate conditions met.
         """
+        # pylint: disable=fixme
+        # TODO: (arguable) split this method into "clean" functions (with no side effects and implicit state)
+        # This method implicitly depends on self.item_state (via _is_answer_correct and _get_grade)
+        # and also updates self.grade if some conditions are met. As a result this method implies some order of
+        # invocation:
+        # * it should be called after learner-caused updates to self.item_state is applied
+        # * it should be called before self.item_state cleanup is applied (i.e. returning misplaced items to item bank)
+        # * it should be called before any method that depends on self.grade (i.e. self._get_feedback)
+
+        # Splitting it into a "clean" functions will allow to capture this implicit invocation order in caller method
+        # and help avoid bugs caused by invocation order violation in future.
+
         # 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()
@@ -694,7 +715,7 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
             'items': item_state,
             'finished': is_finished,
             'attempts': self.attempts,
-            'overall_feedback': self._present_overall_feedback(overall_feedback_msgs)
+            'overall_feedback': self._present_feedback(overall_feedback_msgs)
         }
 
     def _get_item_state(self):
@@ -794,8 +815,8 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
         """
         Returns the student's grade for this block.
         """
-        correct_count, required_count = self._get_item_stats()
-        return correct_count / float(required_count) * self.weight
+        correct_count, total_count = self._get_item_stats()
+        return correct_count / float(total_count) * self.weight
 
     def _answer_correctness(self):
         """
@@ -807,8 +828,8 @@ class DragAndDropBlock(XBlock, XBlockWithSettingsMixin, ThemableXBlockMixin):
                 * Partial: Some items are at their correct place.
                 * Incorrect: None items are at their correct place.
         """
-        correct_count, required_count = self._get_item_stats()
-        if correct_count == required_count:
+        correct_count, total_count = self._get_item_stats()
+        if correct_count == total_count:
             return self.SOLUTION_CORRECT
         elif correct_count == 0:
             return self.SOLUTION_INCORRECT

drag_and_drop_v2/public/css/drag_and_drop.css

@@ -356,13 +356,11 @@
 
 .xblock--drag-and-drop .popup .popup-content {
     color: #ffffff;
-    margin-left: 15px;
-    margin-top: 35px;
-    margin-bottom: 15px;
+    margin: 35px 15px 15px 15px;
     font-size: 14px;
 }
 
-.xblock--drag-and-drop .popup .close {
+.xblock--drag-and-drop .popup .close-feedback-popup-button {
     cursor: pointer;
     float: right;
     margin-right: 8px;
@@ -373,7 +371,7 @@
     font-size: 18pt;
 }
 
-.xblock--drag-and-drop .popup .close:focus {
+.xblock--drag-and-drop .popup .close-feedback-popup-button:focus {
     outline: 2px solid white;
 }
 

drag_and_drop_v2/public/js/drag_and_drop.js

@@ -367,9 +367,73 @@ function DragAndDropTemplates(configuration) {
         )
     };
 
+    var itemFeedbackPopupTemplate = function(ctx) {
+        var popupSelector = 'div.popup.item-feedback-popup.popup-wrapper';
+        var msgs = ctx.feedback_messages || [];
+        var have_messages = msgs.length > 0;
+        var popup_content;
+
+        var close_button_describedby_id = "close-popup-"+configuration.url_name;
+
+        if (msgs.length > 0 && !ctx.last_action_correct) {
+            popupSelector += '.popup-incorrect';
+        }
+
+        if (ctx.mode == DragAndDropBlock.ASSESSMENT_MODE) {
+            var content_items = [
+                (!ctx.last_action_correct) ? h("p", {}, gettext("Some of your answers were not correct.")) : null,
+                h("p", {}, gettext("Hints:")),
+                h("ul", {}, msgs.map(function(message) {
+                    return h("li", {innerHTML: message.message});
+                }))
+            ];
+            popup_content = h("div.popup-content", {}, have_messages ? content_items : []);
+        } else {
+            popup_content = h("div.popup-content", {}, msgs.map(function(message) {
+                return h("p", {innerHTML: message.message});
+            }))
+        }
+
+        return h(
+            popupSelector,
+            {
+                style: {display: have_messages ? 'block' : 'none'},
+                attributes: {
+                    "tabindex": "-1",
+                    'aria-live': 'polite',
+                    'aria-atomic': 'true',
+                    'aria-relevant': 'additions',
+                }
+            },
+            [
+                h(
+                    'button.unbutton.close-feedback-popup-button',
+                    {},
+                    [
+                        h(
+                            'span.sr',
+                            {
+                                innerHTML: gettext("Close item feedback popup")
+                            }
+                        ),
+                        h(
+                            'span.icon.fa.fa-times-circle',
+                            {
+                                attributes: {
+                                    'aria-hidden': true
+                                }
+                            }
+                        )
+                    ]
+                ),
+                popup_content
+            ]
+        )
+    };
+
     var mainTemplate = function(ctx) {
-        var problemTitle = ctx.show_title ? h('h2.problem-title', {innerHTML: ctx.title_html}) : null;
-        var problemHeader = ctx.show_problem_header ? h('h3.title1', gettext('Problem')) : null;
+        var problemTitle = ctx.show_title ? h('h3.problem-title', {innerHTML: ctx.title_html}) : null;
+        var problemHeader = ctx.show_problem_header ? h('h4.title1', gettext('Problem')) : null;
 
         // Render only items_in_bank and items_placed_unaligned here;
         // items placed in aligned zones will be rendered by zoneTemplate.
@@ -401,7 +465,7 @@ function DragAndDropTemplates(configuration) {
                     h('div.target',
                         {},
                         [
-                            popupTemplate(ctx),
+                            itemFeedbackPopupTemplate(ctx),
                             h('div.target-img-wrapper', [
                                 h('img.target-img', {src: ctx.target_img_src, alt: ctx.target_img_description}),
                             ]
@@ -428,6 +492,11 @@ function DragAndDropBlock(runtime, element, configuration) {
     DragAndDropBlock.STANDARD_MODE = 'standard';
     DragAndDropBlock.ASSESSMENT_MODE = 'assessment';
 
+    var Selector = {
+        popup_box: '.popup',
+        close_button: '.popup .close-feedback-popup-button'
+    };
+
     var renderView = DragAndDropTemplates(configuration);
 
     // Set up a mock for gettext if it isn't available in the client runtime:
@@ -482,7 +551,7 @@ function DragAndDropBlock(runtime, element, configuration) {
 
             // Set up event handlers:
 
-            $(document).on('keydown mousedown touchstart', closePopup);
+            $element.on('click', '.item-feedback-popup .close-feedback-popup-button', closePopupEventHandler);
             $element.on('click', '.submit-answer-button', doAttempt);
             $element.on('click', '.keyboard-help-button', showKeyboardHelp);
             $element.on('keydown', '.keyboard-help-button', function(evt) {
@@ -646,12 +715,23 @@ function DragAndDropBlock(runtime, element, configuration) {
     /**
      * Update the DOM to reflect 'state'.
      */
-    var applyState = function() {
+    var applyState = function(keepDraggableInit) {
+        sendFeedbackPopupEvents();
+        updateDOM();
+        if (!keepDraggableInit) {
+            destroyDraggable();
+            if (!state.finished) {
+                initDraggable();
+            }
+        }
+    };
+
+    var sendFeedbackPopupEvents = function() {
         // Has the feedback popup been closed?
         if (state.closing) {
             var data = {
                 event_type: 'edx.drag_and_drop_v2.feedback.closed',
-                content: previousFeedback || state.feedback,
+                content: concatenateFeedback(previousFeedback || state.feedback),
                 manually: state.manually_closed,
             };
             truncateField(data, 'content');
@@ -663,17 +743,15 @@ function DragAndDropBlock(runtime, element, configuration) {
         if (state.feedback) {
             var data = {
                 event_type: 'edx.drag_and_drop_v2.feedback.opened',
-                content: state.feedback,
+                content: concatenateFeedback(state.feedback),
             };
             truncateField(data, 'content');
             publishEvent(data);
         }
+    };
 
-        updateDOM();
-        destroyDraggable();
-        if (!state.finished) {
-            initDraggable();
-        }
+    var concatenateFeedback = function (feedback_msgs_list) {
+        return feedback_msgs_list.map(function(message) { return message.message; }).join('\n');
     };
 
     var updateDOM = function(state) {
@@ -739,6 +817,15 @@ function DragAndDropBlock(runtime, element, configuration) {
         $root.find('.item-bank .option').first().focus();
     };
 
+    var focusItemFeedbackPopup = function() {
+        var popup = $root.find('.item-feedback-popup');
+        if (popup.length && popup.is(":visible")) {
+            popup.focus();
+            return true;
+        }
+        return false;
+    };
+
     var placeItem = function($zone, $item) {
         var item_id;
         if ($item !== undefined) {
@@ -753,7 +840,7 @@ function DragAndDropBlock(runtime, element, configuration) {
         var items_in_zone_count = countItemsInZone(zone, [item_id.toString()]);
         if (configuration.max_items_per_zone && configuration.max_items_per_zone <= items_in_zone_count) {
             state.last_action_correct = false;
-            state.feedback = gettext("You cannot add any more items to this zone.");
+            state.feedback = [{message: gettext("You cannot add any more items to this zone."), message_class: null}];
             applyState();
             return;
         }
@@ -763,6 +850,7 @@ function DragAndDropBlock(runtime, element, configuration) {
             zone_align: zone_align,
             submitting_location: true,
         };
+
         // Wrap in setTimeout to let the droppable event finish.
         setTimeout(function() {
             applyState();
@@ -895,13 +983,16 @@ function DragAndDropBlock(runtime, element, configuration) {
     var grabItem = function($item, interaction_type) {
         var item_id = $item.data('value');
         setGrabbedState(item_id, true, interaction_type);
-        updateDOM();
+        closePopup(false);
+        // applyState(true) skips destroying and initializing draggable
+        applyState(true);
     };
 
     var releaseItem = function($item) {
         var item_id = $item.data('value');
         setGrabbedState(item_id, false);
-        updateDOM();
+        // applyState(true) skips destroying and initializing draggable
+        applyState(true);
     };
 
     var setGrabbedState = function(item_id, grabbed, interaction_type) {
@@ -967,31 +1058,31 @@ function DragAndDropBlock(runtime, element, configuration) {
             });
     };
 
-    var closePopup = function(evt) {
+    var closePopupEventHandler = function(evt) {
         if (!state.feedback) {
             return;
         }
 
         var target = $(evt.target);
-        var popup_box = '.xblock--drag-and-drop .popup';
-        var close_button = '.xblock--drag-and-drop .popup .close';
 
-        if (target.is(popup_box)) {
+        if (target.is(Selector.popup_box)) {
             return;
         }
-        if (target.parents(popup_box).length && !target.is(close_button)) {
+        if (target.parents(Selector.popup_box).length && !target.parent().is(Selector.close_button) && !target.is(Selector.close_button)) {
             return;
         }
 
+        closePopup(target.is(Selector.close_button) || target.parent().is(Selector.close_button));
+        applyState();
+    };
+
+    var closePopup = function(manually_closed) {
+        // do not apply state here - callers are responsible to call it when other appropriate state changes are applied
+        if ($root.find(Selector.popup_box).is(":visible")) {
             state.closing = true;
             previousFeedback = state.feedback;
-        if (target.is(close_button)) {
-            state.manually_closed = true;
-        } else {
-            state.manually_closed = false;
+            state.manually_closed = manually_closed;
         }
-
-        applyState();
     };
 
     var resetProblem = function(evt) {
@@ -1018,7 +1109,10 @@ function DragAndDropBlock(runtime, element, configuration) {
             data: '{}'
         }).done(function(data){
             state.attempts = data.attempts;
+            state.feedback = data.feedback;
             state.overall_feedback = data.overall_feedback;
+            state.last_action_correct = data.correct;
+
             if (attemptsRemain()) {
                 data.misplaced_items.forEach(function(misplaced_item_id) {
                     delete state.items[misplaced_item_id]
@@ -1026,15 +1120,15 @@ function DragAndDropBlock(runtime, element, configuration) {
             } else {
                 state.finished = true;
             }
-            focusFirstDraggable();
         }).always(function() {
             state.submit_spinner = false;
             applyState();
+            focusItemFeedbackPopup() || focusFirstDraggable();
         });
     };
 
     var canSubmitAttempt = function() {
-        return Object.keys(state.items).length > 0 && attemptsRemain();
+        return Object.keys(state.items).length > 0 && attemptsRemain() && !submittingLocation();
     };
 
     var canReset = function() {
@@ -1057,6 +1151,15 @@ function DragAndDropBlock(runtime, element, configuration) {
         return !configuration.max_attempts || configuration.max_attempts > state.attempts;
     };
 
+    var submittingLocation = function() {
+        var result = false;
+        Object.keys(state.items).forEach(function(item_id) {
+            var item = state.items[item_id];
+            result = result || item.submitting_location;
+        });
+        return result;
+    }
+
     var render = function() {
         var items = configuration.items.map(function(item) {
             var item_user_state = state.items[item.id];
@@ -1114,7 +1217,6 @@ function DragAndDropBlock(runtime, element, configuration) {
             show_title: configuration.show_title,
             mode: configuration.mode,
             max_attempts: configuration.max_attempts,
-            attempts: state.attempts,
             problem_html: configuration.problem_text,
             show_problem_header: configuration.show_problem_header,
             show_submit_answer: configuration.mode == DragAndDropBlock.ASSESSMENT_MODE,
@@ -1125,9 +1227,10 @@ function DragAndDropBlock(runtime, element, configuration) {
             zones: configuration.zones,
             items: items,
             // state - parts that can change:
+            attempts: state.attempts,
             last_action_correct: state.last_action_correct,
             item_bank_focusable: item_bank_focusable,
-            popup_html: state.feedback || '',
+            feedback_messages: state.feedback,
             overall_feedback_messages: state.overall_feedback,
             disable_reset_button: !canReset(),
             disable_submit_button: !canSubmitAttempt(),

drag_and_drop_v2/translations/en/LC_MESSAGES/text.po

@@ -499,6 +499,10 @@ msgstr ""
 msgid "You have used {used} of {total} attempts."
 msgstr ""
 
+#: public/js/drag_and_drop.js
+msgid "Some of your answers were not correct."
+msgstr ""
+
 #: public/js/drag_and_drop_edit.js
 msgid "There was an error with your form."
 msgstr ""
@@ -511,12 +515,12 @@ msgstr ""
 msgid "None"
 msgstr ""
 
-#: utils.py:18
-msgid "Final attempt was used, highest score is {score}"
+#: public/js/drag_and_drop_edit.js
+msgid "Close item feedback popup"
 msgstr ""
 
-#: utils.py:19
-msgid "Misplaced items were returned to item bank."
+#: utils.py:18
+msgid "Final attempt was used, highest score is {score}"
 msgstr ""
 
 #: utils.py:24
@@ -526,8 +530,8 @@ msgstr[0] ""
 msgstr[1] ""
 
 #: utils.py:32
-msgid "Misplaced {misplaced_count} item."
-msgid_plural "Misplaced {misplaced_count} items."
+msgid "Misplaced {misplaced_count} item. Misplaced item was returned to item bank."
+msgid_plural "Misplaced {misplaced_count} items. Misplaced items were returned to item bank."
 msgstr[0] ""
 msgstr[1] ""
 

drag_and_drop_v2/translations/eo/LC_MESSAGES/text.po

@@ -585,6 +585,10 @@ msgstr ""
 msgid "You have used {used} of {total} attempts."
 msgstr "Ýöü hävé üséd {used} öf {total} ättémpts. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєт#"
 
+#: public/js/drag_and_drop.js
+msgid "Some of your answers were not correct."
+msgstr "Sömé öf ýöür änswérs wéré nöt cörréct. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєт#"
+
 #: public/js/drag_and_drop_edit.js
 msgid "There was an error with your form."
 msgstr ""
@@ -599,28 +603,28 @@ msgstr ""
 msgid "None"
 msgstr "Nöné Ⱡ'σяєм ι#"
 
-#: utils.py:18
-msgid "Fïnäl ättémpt wäs üséd, hïghést sçöré ïs {score} Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
-msgstr ""
+#: public/js/drag_and_drop_edit.js
+msgid "Close item feedback popup"
+msgstr "Çlösé ïtém féédßäçk pöpüp Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕ#"
 
-#: utils.py:19
-msgid "Mïspläçéd ïtéms wéré rétürnéd tö ïtém ßänk. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
-msgstr ""
+#: utils.py:18
+msgid "Final attempt was used, highest score is {score}"
+msgstr "Fïnäl ättémpt wäs üséd, hïghést sçöré ïs {score} Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢тєтυя #"
 
 #: utils.py:24
-msgid "Çörréçtlý pläçéd {correct_count} ïtém. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕ#"
-msgid_plural "Çörréçtlý pläçéd {correct_count} ïtéms. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє#"
-msgstr[0] ""
-msgstr[1] ""
+msgid "Correctly placed {correct_count} item."
+msgid_plural "Correctly placed {correct_count} items."
+msgstr[0] "Çörréçtlý pläçéd {correct_count} ïtém. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕ#"
+msgstr[1] "Çörréçtlý pläçéd {correct_count} ïtéms. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє#"
 
 #: utils.py:32
-msgid "Mïspläçéd {misplaced_count} ïtém. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт,#"
-msgid_plural "Mïspläçéd {misplaced_count} ïtéms. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, #"
-msgstr[0] ""
-msgstr[1] ""
+msgid "Misplaced {misplaced_count} item. Misplaced item was returned to item bank."
+msgid_plural "Misplaced {misplaced_count} items. Misplaced items were returned to item bank."
+msgstr[0] "Mïspläçéd {misplaced_count} ïtém. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт,#"
+msgstr[1] "Mïspläçéd {misplaced_count} ïtéms. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, #"
 
 #: utils.py:40
-msgid "Dïd nöt pläçé {missing_count} réqüïréd ïtém. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢#"
-msgid_plural "Dïd nöt pläçé {missing_count} réqüïréd ïtéms. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢т#"
-msgstr[0] ""
-msgstr[1] ""
+msgid "Did not place {missing_count} required item."
+msgid_plural "Did not place {missing_count} required items."
+msgstr[0] "Dïd nöt pläçé {missing_count} réqüïréd ïtém. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢#"
+msgstr[1] "Dïd nöt pläçé {missing_count} réqüïréd ïtéms. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢т#"

drag_and_drop_v2/utils.py

@@ -42,7 +42,6 @@ class FeedbackMessages(object):
         NOT_PLACED = INCORRECT_SOLUTION
 
     FINAL_ATTEMPT_TPL = _('Final attempt was used, highest score is {score}')
-    MISPLACED_ITEMS_RETURNED = _('Misplaced item(s) were returned to item bank.')
 
     @staticmethod
     def correctly_placed(number, ngettext=ngettext_fallback):

tests/integration/test_base.py

@@ -214,6 +214,8 @@ class DefaultDataTestMixin(object):
 
 
 class InteractionTestBase(object):
+    POPUP_ERROR_CLASS = "popup-incorrect"
+
     @classmethod
     def _get_items_with_zone(cls, items_map):
         return {
@@ -309,7 +311,7 @@ class InteractionTestBase(object):
             u"Spinner should not be in {}".format(elem.get_attribute('innerHTML'))
         )
 
-    def place_item(self, item_value, zone_id, action_key=None):
+    def place_item(self, item_value, zone_id, action_key=None, wait=True):
         """
         Place item with ID of item_value into zone with ID of zone_id.
         zone_id=None means place item back to the item bank.
@@ -319,6 +321,7 @@ class InteractionTestBase(object):
             self.drag_item_to_zone(item_value, zone_id)
         else:
             self.move_item_to_zone(item_value, zone_id, action_key)
+        if wait:
             self.wait_for_ajax()
 
     def drag_item_to_zone(self, item_value, zone_id):
@@ -429,3 +432,12 @@ class InteractionTestBase(object):
         """ Only needed if there are multiple blocks on the page. """
         self._page = self.browser.find_elements_by_css_selector(self.default_css_selector)[idx]
         self.scroll_down(0)
+
+    def assert_popup_correct(self, popup):
+        self.assertNotIn(self.POPUP_ERROR_CLASS, popup.get_attribute('class'))
+
+    def assert_popup_incorrect(self, popup):
+        self.assertIn(self.POPUP_ERROR_CLASS, popup.get_attribute('class'))
+
+    def assert_button_enabled(self, submit_button, enabled=True):
+        self.assertEqual(submit_button.is_enabled(), enabled)

tests/integration/test_events.py

-from ddt import ddt, data, unpack
+from ddt import data, ddt, unpack
 from mock import Mock, patch
+
 from workbench.runtime import WorkbenchRuntime
 
-from drag_and_drop_v2.default_data import TOP_ZONE_TITLE, TOP_ZONE_ID, ITEM_CORRECT_FEEDBACK
+from drag_and_drop_v2.default_data import (
+    TOP_ZONE_TITLE, TOP_ZONE_ID, MIDDLE_ZONE_TITLE, MIDDLE_ZONE_ID, ITEM_CORRECT_FEEDBACK, ITEM_INCORRECT_FEEDBACK
+)
+from tests.integration.test_base import BaseIntegrationTest, DefaultDataTestMixin, InteractionTestBase
+from tests.integration.test_interaction import DefaultDataTestMixin, ParameterizedTestsMixin
+from tests.integration.test_interaction_assessment import DefaultAssessmentDataTestMixin, AssessmentTestMixin
+
 
-from .test_base import BaseIntegrationTest, DefaultDataTestMixin
-from .test_interaction import ParameterizedTestsMixin
-from tests.integration.test_base import InteractionTestBase
+class BaseEventsTests(InteractionTestBase, BaseIntegrationTest):
+    def setUp(self):
+        mock = Mock()
+        context = patch.object(WorkbenchRuntime, 'publish', mock)
+        context.start()
+        self.addCleanup(context.stop)
+        self.publish = mock
+        super(BaseEventsTests, self).setUp()
 
 
 @ddt
-class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, InteractionTestBase, BaseIntegrationTest):
+class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, BaseEventsTests):
     """
     Tests that the analytics events are fired and in the proper order.
     """
@@ -54,14 +66,6 @@ class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, Interaction
         },
     )
 
-    def setUp(self):
-        mock = Mock()
-        context = patch.object(WorkbenchRuntime, 'publish', mock)
-        context.start()
-        self.addCleanup(context.stop)
-        self.publish = mock
-        super(EventsFiredTest, self).setUp()
-
     def _get_scenario_xml(self):  # pylint: disable=no-self-use
         return "<vertical_demo><drag-and-drop-v2/></vertical_demo>"
 
@@ -71,6 +75,68 @@ class EventsFiredTest(DefaultDataTestMixin, ParameterizedTestsMixin, Interaction
         self.parameterized_item_positive_feedback_on_good_move(self.items_map)
         dummy, name, published_data = self.publish.call_args_list[index][0]
         self.assertEqual(name, event['name'])
-        self.assertEqual(
-                published_data, event['data']
+        self.assertEqual(published_data, event['data'])
+
+
+@ddt
+class AssessmentEventsFiredTest(
+    DefaultAssessmentDataTestMixin, AssessmentTestMixin, BaseEventsTests
+):
+    scenarios = (
+        {
+            'name': 'edx.drag_and_drop_v2.loaded',
+            'data': {},
+        },
+        {
+            'name': 'edx.drag_and_drop_v2.item.picked_up',
+            'data': {'item_id': 0},
+        },
+        {
+            'name': 'edx.drag_and_drop_v2.item.dropped',
+            'data': {
+                'is_correct': False,
+                'item_id': 0,
+                'location': MIDDLE_ZONE_TITLE,
+                'location_id': MIDDLE_ZONE_ID,
+            },
+        },
+        {
+            'name': 'edx.drag_and_drop_v2.item.picked_up',
+            'data': {'item_id': 1},
+        },
+        {
+            'name': 'edx.drag_and_drop_v2.item.dropped',
+            'data': {
+                'is_correct': False,
+                'item_id': 1,
+                'location': TOP_ZONE_TITLE,
+                'location_id': TOP_ZONE_ID,
+            },
+        },
+        {
+            'name': 'grade',
+            'data': {'max_value': 1, 'value': (1.0 / 5)},
+        },
+        {
+            'name': 'edx.drag_and_drop_v2.feedback.opened',
+            'data': {
+                'content': "\n".join([ITEM_INCORRECT_FEEDBACK, ITEM_INCORRECT_FEEDBACK]),
+                'truncated': False,
+            },
+        },
     )
+
+    def test_event(self):
+        self.scroll_down(pixels=100)
+
+        self.place_item(0, MIDDLE_ZONE_ID)
+        self.wait_until_ondrop_xhr_finished(self._get_item_by_value(0))
+        self.place_item(1, TOP_ZONE_ID)
+        self.wait_until_ondrop_xhr_finished(self._get_item_by_value(0))
+
+        self.click_submit()
+        self.wait_for_ajax()
+        for index, event in enumerate(self.scenarios):
+            dummy, name, published_data = self.publish.call_args_list[index][0]
+            self.assertEqual(name, event['name'])
+            self.assertEqual(published_data, event['data'])

tests/integration/test_interaction.py

@@ -42,8 +42,8 @@ class ParameterizedTestsMixin(object):
                 self.assertEqual(feedback_popup_html, '')
                 self.assertFalse(popup.is_displayed())
             else:
-                self.assertEqual(feedback_popup_html, definition.feedback_positive)
-                self.assertEqual(popup.get_attribute('class'), 'popup')
+                self.assertEqual(feedback_popup_html, "<p>{}</p>".format(definition.feedback_positive))
+                self.assert_popup_correct(popup)
                 self.assertTrue(popup.is_displayed())
 
     def parameterized_item_negative_feedback_on_bad_move(
@@ -74,7 +74,7 @@ class ParameterizedTestsMixin(object):
                     self.assert_placed_item(definition.item_id, zone_title, assessment_mode=True)
                 else:
                     self.wait_until_html_in(definition.feedback_negative, feedback_popup_content)
-                    self.assertEqual(popup.get_attribute('class'), 'popup popup-incorrect')
+                    self.assert_popup_incorrect(popup)
                     self.assertTrue(popup.is_displayed())
                     self.assert_reverted_item(definition.item_id)
 
@@ -288,7 +288,7 @@ class MultipleValidOptionsInteractionTest(DefaultDataTestMixin, InteractionTestB
             for i, zone in enumerate(item.zone_ids):
                 self.place_item(item.item_id, zone, None)
                 self.wait_until_html_in(item.feedback_positive[i], feedback_popup_content)
-                self.assertEqual(popup.get_attribute('class'), 'popup')
+                self.assert_popup_correct(popup)
                 self.assert_placed_item(item.item_id, item.zone_title[i])
                 reset.click()
                 self.wait_until_disabled(reset)
@@ -534,12 +534,15 @@ class TestMaxItemsPerZone(InteractionTestBase, BaseIntegrationTest):
         self.assertTrue(feedback_popup.is_displayed())
 
         feedback_popup_content = self._get_popup_content()
-        self.assertEqual(
-            feedback_popup_content.get_attribute('innerHTML'),
-            "You cannot add any more items to this zone."
+        self.assertIn(
+            "You cannot add any more items to this zone.",
+            feedback_popup_content.get_attribute('innerHTML')
         )
 
     def test_item_returned_to_bank_after_refresh(self):
+        """
+        Tests that an item returned to the bank stays there after page refresh
+        """
         zone_id = "Zone Left Align"
         self.place_item(6, zone_id)
         self.place_item(7, zone_id)

tests/integration/test_interaction_assessment.py

@@ -2,6 +2,7 @@
 
 from ddt import ddt, data
 from mock import Mock, patch
+import time
 
 from selenium.webdriver.support.ui import WebDriverWait
 from selenium.webdriver.common.keys import Keys
@@ -175,7 +176,6 @@ class AssessmentInteractionTest(
             FeedbackMessages.correctly_placed(1),
             FeedbackMessages.misplaced(1),
             FeedbackMessages.not_placed(2),
-            FeedbackMessages.MISPLACED_ITEMS_RETURNED,
             START_FEEDBACK
         ]
         expected_feedback = "\n".join(feedback_lines)
@@ -219,6 +219,44 @@ class AssessmentInteractionTest(
         expected_grade = {'max_value': 1, 'value': (1.0 / 5.0)}
         self.assertEqual(published_grade, expected_grade)
 
+    def test_per_item_feedback_multiple_misplaced(self):
+        self.place_item(0, MIDDLE_ZONE_ID, Keys.RETURN)
+        self.place_item(1, BOTTOM_ZONE_ID, Keys.RETURN)
+        self.place_item(2, TOP_ZONE_ID, Keys.RETURN)
+
+        self.click_submit()
+
+        placed_item_definitions = [self.items_map[item_id] for item_id in (1, 2, 3)]
+
+        expected_message_elements = [
+            "<li>{msg}</li>".format(msg=definition.feedback_negative)
+            for definition in placed_item_definitions
+        ]
+
+        for message_element in expected_message_elements:
+            self.assertIn(message_element, self._get_popup_content().get_attribute('innerHTML'))
+
+    def test_submit_disabled_during_drop_item(self):
+        def delayed_drop_item(item_attempt, suffix=''):  # pylint: disable=unused-argument
+            # some delay to allow selenium check submit button disabled status while "drop_item"
+            # XHR is still executing
+            time.sleep(0.1)
+            return {}
+
+        self.place_item(0, TOP_ZONE_ID)
+        self.assert_placed_item(0, TOP_ZONE_TITLE, assessment_mode=True)
+
+        submit_button = self._get_submit_button()
+        self.assert_button_enabled(submit_button)  # precondition check
+        with patch('drag_and_drop_v2.DragAndDropBlock._drop_item_assessment', Mock(side_effect=delayed_drop_item)):
+            item_id = 1
+            self.place_item(item_id, MIDDLE_ZONE_ID, wait=False)
+            # do not wait for XHR to complete
+            self.assert_button_enabled(submit_button, enabled=False)
+            self.wait_until_ondrop_xhr_finished(self._get_placed_item_by_value(item_id))
+
+            self.assert_button_enabled(submit_button, enabled=True)
+
 
 class TestMaxItemsPerZoneAssessment(TestMaxItemsPerZone):
     assessment_mode = True
@@ -228,6 +266,9 @@ class TestMaxItemsPerZoneAssessment(TestMaxItemsPerZone):
         return self._make_scenario_xml(data=scenario_data, max_items_per_zone=2, mode=Constants.ASSESSMENT_MODE)
 
     def test_drop_item_to_same_zone_does_not_show_popup(self):
+        """
+        Tests that picking item from saturated zone and dropping it back again does not trigger error popup
+        """
         zone_id = "Zone Left Align"
         self.place_item(6, zone_id)
         self.place_item(7, zone_id)

tests/integration/test_render.py

@@ -209,7 +209,7 @@ class TestDragAndDropRender(BaseIntegrationTest):
         popup_wrapper = self._get_popup_wrapper()
         popup_content = self._get_popup_content()
         self.assertFalse(popup.is_displayed())
-        self.assertEqual(popup.get_attribute('class'), 'popup')
+        self.assertIn('popup', popup.get_attribute('class'))
         self.assertEqual(popup_content.text, "")
         self.assertEqual(popup_wrapper.get_attribute('aria-live'), 'polite')
 

tests/integration/test_title_and_question.py

@@ -26,7 +26,7 @@ class TestDragAndDropTitleAndProblem(BaseIntegrationTest):
         self.addCleanup(scenarios.remove_scenario, const_page_id)
 
         page = self.go_to_page(const_page_name)
-        is_problem_header_visible = len(page.find_elements_by_css_selector('section.problem > h3')) > 0
+        is_problem_header_visible = len(page.find_elements_by_css_selector('section.problem > h4')) > 0
         self.assertEqual(is_problem_header_visible, show_problem_header)
 
         problem = page.find_element_by_css_selector('section.problem > p')
@@ -52,8 +52,8 @@ class TestDragAndDropTitleAndProblem(BaseIntegrationTest):
 
         page = self.go_to_page(const_page_name)
         if show_title:
-            problem_header = page.find_element_by_css_selector('h2.problem-title')
+            problem_header = page.find_element_by_css_selector('h3.problem-title')
             self.assertEqual(self.get_element_html(problem_header), display_name)
         else:
             with self.assertRaises(NoSuchElementException):
-                page.find_element_by_css_selector('h2.problem-title')
+                page.find_element_by_css_selector('h3.problem-title')

tests/unit/data/assessment/config_out.json

@@ -9,7 +9,6 @@
     "target_img_description": "This describes the target image",
     "item_background_color": null,
     "item_text_color": null,
-    "initial_feedback": "This is the initial feedback.",
     "display_zone_borders": false,
     "display_zone_labels": false,
     "url_name": "test",

tests/unit/data/html/config_out.json

@@ -9,7 +9,6 @@
     "target_img_description": "This describes the target image",
     "item_background_color": "white",
     "item_text_color": "#000080",
-    "initial_feedback": "HTML <strong>Intro</strong> Feed",
     "display_zone_borders": false,
     "display_zone_labels": false,
     "url_name": "unique_name",

tests/unit/data/old/config_out.json

@@ -9,7 +9,6 @@
     "target_img_description": "This describes the target image",
     "item_background_color": null,
     "item_text_color": null,
-    "initial_feedback": "Intro Feed",
     "display_zone_borders": false,
     "display_zone_labels": false,
     "url_name": "",

tests/unit/data/plain/config_out.json

@@ -9,7 +9,6 @@
     "target_img_description": "This describes the target image",
     "item_background_color": null,
     "item_text_color": null,
-    "initial_feedback": "This is the initial feedback.",
     "display_zone_borders": false,
     "display_zone_labels": false,
     "url_name": "test",

tests/unit/test_advanced.py

@@ -25,6 +25,7 @@ class BaseDragAndDropAjaxFixture(TestCaseMixin):
     ZONE_2 = None
 
     OVERALL_FEEDBACK_KEY = 'overall_feedback'
+    FEEDBACK_KEY = 'feedback'
 
     FEEDBACK = {
         0: {"correct": None, "incorrect": None},
@@ -32,6 +33,7 @@ class BaseDragAndDropAjaxFixture(TestCaseMixin):
         2: {"correct": None, "incorrect": None}
     }
 
+    START_FEEDBACK = None
     FINAL_FEEDBACK = None
 
     FOLDER = None
@@ -60,11 +62,6 @@ class BaseDragAndDropAjaxFixture(TestCaseMixin):
     def expected_configuration(cls):
         return json.loads(loader.load_unicode('data/{}/config_out.json'.format(cls.FOLDER)))
 
-    @classmethod
-    def initial_feedback(cls):
-        """ The initial overall_feedback value """
-        return cls.expected_configuration()["initial_feedback"]
-
     def test_get_configuration(self):
         self.assertEqual(self.block.get_configuration(), self.expected_configuration())
 
@@ -73,37 +70,65 @@ class StandardModeFixture(BaseDragAndDropAjaxFixture):
     """
     Common tests for drag and drop in standard mode
     """
+    def _make_item_feedback_message(self, item_id, key="incorrect"):
+        if self.FEEDBACK[item_id][key]:
+            return self._make_feedback_message(self.FEEDBACK[item_id][key])
+        else:
+            return None
+
+    def test_reset_no_item_feedback(self):
+        data = {"val": 1, "zone": self.ZONE_1, "x_percent": "33%", "y_percent": "11%"}
+        self.call_handler(self.DROP_ITEM_HANDLER, data)
+
+        res = self.call_handler(self.RESET_HANDLER, data={})
+        expected_overall_feedback = [self._make_feedback_message(message=self.INITIAL_FEEDBACK)]
+        self.assertEqual(res[self.OVERALL_FEEDBACK_KEY], expected_overall_feedback)
+
+    def test_user_state_no_item_state(self):
+        res = self.call_handler(self.USER_STATE_HANDLER, data={})
+        expected_overall_feedback = [self._make_feedback_message(message=self.INITIAL_FEEDBACK)]
+        self.assertEqual(res[self.OVERALL_FEEDBACK_KEY], expected_overall_feedback)
+
     def test_drop_item_wrong_with_feedback(self):
         item_id, zone_id = 0, self.ZONE_2
         data = {"val": item_id, "zone": zone_id}
         res = self.call_handler(self.DROP_ITEM_HANDLER, data)
+        item_feedback_message = self._make_item_feedback_message(item_id)
+        expected_feedback = [item_feedback_message] if item_feedback_message else []
+
         self.assertEqual(res, {
             "overall_feedback": [self._make_feedback_message(message=self.INITIAL_FEEDBACK)],
             "finished": False,
             "correct": False,
-            "feedback": self.FEEDBACK[item_id]["incorrect"]
+            "feedback": expected_feedback
         })
 
     def test_drop_item_wrong_without_feedback(self):
         item_id, zone_id = 2, self.ZONE_1
         data = {"val": item_id, "zone": zone_id}
         res = self.call_handler(self.DROP_ITEM_HANDLER, data)
+        item_feedback_message = self._make_item_feedback_message(item_id)
+        expected_feedback = [item_feedback_message] if item_feedback_message else []
+
         self.assertEqual(res, {
             "overall_feedback": [self._make_feedback_message(message=self.INITIAL_FEEDBACK)],
             "finished": False,
             "correct": False,
-            "feedback": self.FEEDBACK[item_id]["incorrect"]
+            "feedback": expected_feedback
         })
 
     def test_drop_item_correct(self):
         item_id, zone_id = 0, self.ZONE_1
         data = {"val": item_id, "zone": zone_id}
         res = self.call_handler(self.DROP_ITEM_HANDLER, data)
+        item_feedback_message = self._make_item_feedback_message(item_id, key="correct")
+        expected_feedback = [item_feedback_message] if item_feedback_message else []
+
         self.assertEqual(res, {
             "overall_feedback": [self._make_feedback_message(message=self.INITIAL_FEEDBACK)],
             "finished": False,
             "correct": True,
-            "feedback": self.FEEDBACK[item_id]["correct"]
+            "feedback": expected_feedback
         })
 
     def test_grading(self):
@@ -143,7 +168,7 @@ class StandardModeFixture(BaseDragAndDropAjaxFixture):
             "overall_feedback": [self._make_feedback_message(message=self.FINAL_FEEDBACK)],
             "finished": True,
             "correct": True,
-            "feedback": self.FEEDBACK[1]["correct"]
+            "feedback": [self._make_feedback_message(self.FEEDBACK[1]["correct"])]
         })
 
         expected_state = {
@@ -194,6 +219,9 @@ class AssessmentModeFixture(BaseDragAndDropAjaxFixture):
         self.block.max_attempts = 5
         self.block.attempts = 4
 
+    def _do_attempt(self):
+        return self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+
     def test_multiple_drop_item(self):
         item_zone_map = {0: self.ZONE_1, 1: self.ZONE_2}
         for item_id, zone_id in item_zone_map.iteritems():
@@ -220,6 +248,14 @@ class AssessmentModeFixture(BaseDragAndDropAjaxFixture):
         ]
         self.assertEqual(res[self.OVERALL_FEEDBACK_KEY], expected_feedback)
 
+    def test_reset_no_item_feedback(self):
+        self.block.attempts = 1
+        self._submit_partial_solution()
+        res = self.call_handler(self.RESET_HANDLER, data={})
+
+        expected_overall_feedback = [self._make_feedback_message(message=self.INITIAL_FEEDBACK)]
+        self.assertEqual(res[self.OVERALL_FEEDBACK_KEY], expected_overall_feedback)
+
     # pylint: disable=star-args
     @ddt.data(
         (None, 10, False),
@@ -250,25 +286,27 @@ class AssessmentModeFixture(BaseDragAndDropAjaxFixture):
         self._submit_complete_solution()
 
         with mock.patch('workbench.runtime.WorkbenchRuntime.publish', mock.Mock()) as patched_publish:
-            self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+            res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
 
             self.assertTrue(self.block.completed)
             patched_publish.assert_called_once_with(self.block, 'grade', {
                 'value': self.block.weight,
                 'max_value': self.block.weight,
             })
+            self.assertTrue(res['correct'])
 
     def test_do_attempt_incorrect_publish_grade(self):
         correctness = self._submit_partial_solution()
 
         with mock.patch('workbench.runtime.WorkbenchRuntime.publish', mock.Mock()) as patched_publish:
-            self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+            res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
 
             self.assertFalse(self.block.completed)
             patched_publish.assert_called_once_with(self.block, 'grade', {
                 'value': self.block.weight * correctness,
                 'max_value': self.block.weight,
             })
+            self.assertFalse(res['correct'])
 
     def test_do_attempt_post_correct_no_publish_grade(self):
         self._submit_complete_solution()
@@ -286,7 +324,7 @@ class AssessmentModeFixture(BaseDragAndDropAjaxFixture):
     def test_get_user_state_finished_after_final_attempt(self):
         self._set_final_attempt()
         self._submit_partial_solution()
-        self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+        self._do_attempt()
 
         self.assertFalse(self.block.attempts_remain)  # precondition check
 
@@ -343,10 +381,6 @@ class AssessmentModeFixture(BaseDragAndDropAjaxFixture):
 
         res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
         self.assertTrue(res['misplaced_items'], misplaced_ids)
-        self.assertIn(
-            self._make_feedback_message(FeedbackMessages.MISPLACED_ITEMS_RETURNED),
-            res[self.OVERALL_FEEDBACK_KEY]
-        )
 
     def test_do_attempt_shows_final_feedback_at_last_attempt(self):
         self._set_final_attempt()
@@ -423,12 +457,17 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
     FEEDBACK = {
         0: {"correct": "Yes 1", "incorrect": "No 1"},
         1: {"correct": "Yes 2", "incorrect": "No 2"},
-        2: {"correct": "", "incorrect": ""}
+        2: {"correct": "", "incorrect": ""},
+        3: {"correct": "", "incorrect": ""}
     }
 
     INITIAL_FEEDBACK = "This is the initial feedback."
     FINAL_FEEDBACK = "This is the final feedback."
 
+    def _assert_item_and_overall_feedback(self, res, expected_item_feedback, expected_overall_feedback):
+        self.assertEqual(res[self.FEEDBACK_KEY], expected_item_feedback)
+        self.assertEqual(res[self.OVERALL_FEEDBACK_KEY], expected_overall_feedback)
+
     def _submit_complete_solution(self):
         self._submit_solution({0: self.ZONE_1, 1: self.ZONE_2, 2: self.ZONE_2})
 
@@ -442,59 +481,70 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
 
     def test_do_attempt_feedback_incorrect_not_placed(self):
         self._submit_solution({0: self.ZONE_2, 1: self.ZONE_2})
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
-        overall_feedback = res[self.OVERALL_FEEDBACK_KEY]
+        res = self._do_attempt()
+
+        expected_item_feedback = [self._make_feedback_message(self.FEEDBACK[0]['incorrect'])]
         expected_overall_feedback = [
             self._make_feedback_message(
                 FeedbackMessages.correctly_placed(1), FeedbackMessages.MessageClasses.CORRECTLY_PLACED
             ),
             self._make_feedback_message(FeedbackMessages.misplaced(1), FeedbackMessages.MessageClasses.MISPLACED),
             self._make_feedback_message(FeedbackMessages.not_placed(1), FeedbackMessages.MessageClasses.NOT_PLACED),
-            self._make_feedback_message(FeedbackMessages.MISPLACED_ITEMS_RETURNED, None),
             self._make_feedback_message(self.INITIAL_FEEDBACK, None),
         ]
-        self.assertEqual(overall_feedback, expected_overall_feedback)
 
-    def test_do_attempt_feedback_not_placed(self):
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
-        overall_feedback = res[self.OVERALL_FEEDBACK_KEY]
+        self._assert_item_and_overall_feedback(res, expected_item_feedback, expected_overall_feedback)
+
+    def test_do_attempt_no_item_state(self):
+        """
+        Test do_attempt overall feedback when no item state is saved - no items were ever dropped.
+        """
+        res = self._do_attempt()
+        self.assertEqual(res[self.FEEDBACK_KEY], [])
+
+        expected_item_feedback = []
         expected_overall_feedback = [
             self._make_feedback_message(FeedbackMessages.not_placed(3), FeedbackMessages.MessageClasses.NOT_PLACED),
             self._make_feedback_message(self.INITIAL_FEEDBACK, None),
         ]
-        self.assertEqual(overall_feedback, expected_overall_feedback)
+
+        self._assert_item_and_overall_feedback(res, expected_item_feedback, expected_overall_feedback)
 
     def test_do_attempt_feedback_correct_and_decoy(self):
         self._submit_solution({0: self.ZONE_1, 1: self.ZONE_2, 3: self.ZONE_2})  # incorrect solution - decoy placed
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
-        overall_feedback = res[self.OVERALL_FEEDBACK_KEY]
+        res = self._do_attempt()
+
+        expected_item_feedback = []
         expected_overall_feedback = [
             self._make_feedback_message(
                 FeedbackMessages.correctly_placed(2), FeedbackMessages.MessageClasses.CORRECTLY_PLACED
             ),
             self._make_feedback_message(FeedbackMessages.misplaced(1), FeedbackMessages.MessageClasses.MISPLACED),
             self._make_feedback_message(FeedbackMessages.not_placed(1), FeedbackMessages.MessageClasses.NOT_PLACED),
-            self._make_feedback_message(FeedbackMessages.MISPLACED_ITEMS_RETURNED, None),
             self._make_feedback_message(self.INITIAL_FEEDBACK, None),
         ]
-        self.assertEqual(overall_feedback, expected_overall_feedback)
+
+        self._assert_item_and_overall_feedback(res, expected_item_feedback, expected_overall_feedback)
 
     def test_do_attempt_feedback_correct(self):
         self._submit_solution({0: self.ZONE_1, 1: self.ZONE_2, 2: self.ZONE_2})  # correct solution
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
-        overall_feedback = res[self.OVERALL_FEEDBACK_KEY]
+        res = self._do_attempt()
+
+        expected_item_feedback = []
         expected_overall_feedback = [
             self._make_feedback_message(
                 FeedbackMessages.correctly_placed(3), FeedbackMessages.MessageClasses.CORRECTLY_PLACED
             ),
             self._make_feedback_message(self.FINAL_FEEDBACK, FeedbackMessages.MessageClasses.CORRECT_SOLUTION),
         ]
-        self.assertEqual(overall_feedback, expected_overall_feedback)
+
+        self._assert_item_and_overall_feedback(res, expected_item_feedback, expected_overall_feedback)
 
     def test_do_attempt_feedback_partial(self):
         self._submit_solution({0: self.ZONE_1})  # partial solution
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
-        overall_feedback = res[self.OVERALL_FEEDBACK_KEY]
+        res = self._do_attempt()
+
+        expected_item_feedback = []
         expected_overall_feedback = [
             self._make_feedback_message(
                 FeedbackMessages.correctly_placed(1), FeedbackMessages.MessageClasses.CORRECTLY_PLACED
@@ -502,14 +552,15 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
             self._make_feedback_message(FeedbackMessages.not_placed(2), FeedbackMessages.MessageClasses.NOT_PLACED),
             self._make_feedback_message(self.INITIAL_FEEDBACK, None),
         ]
-        self.assertEqual(overall_feedback, expected_overall_feedback)
+
+        self._assert_item_and_overall_feedback(res, expected_item_feedback, expected_overall_feedback)
 
     def test_do_attempt_keeps_highest_score(self):
         self.assertFalse(self.block.completed)  # precondition check
         expected_score = 4.0 / 5.0
 
         self._submit_solution({0: self.ZONE_1, 1: self.ZONE_2})  # partial solution, 0.8 score
-        self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+        self._do_attempt()
         self.assertEqual(self.block.grade, expected_score)
 
         self._reset_problem()
@@ -517,13 +568,14 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
         self._set_final_attempt()
 
         self._submit_solution({0: self.ZONE_1})  # partial solution, 0.6 score
-        res = self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+        res = self._do_attempt()
         self.assertEqual(self.block.grade, expected_score)
 
         expected_feedback = self._make_feedback_message(
             FeedbackMessages.FINAL_ATTEMPT_TPL.format(score=expected_score),
             FeedbackMessages.MessageClasses.PARTIAL_SOLUTION
         )
+
         self.assertIn(expected_feedback, res[self.OVERALL_FEEDBACK_KEY])
 
     def test_do_attempt_check_score_with_decoy(self):
@@ -535,7 +587,7 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
             2: self.ZONE_2,
             3: self.ZONE_1,
         })  # incorrect solution, 0.8 score
-        self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+        self._do_attempt()
         self.assertEqual(self.block.grade, expected_score)
 
     def test_do_attempt_zero_score_with_all_decoys(self):
@@ -545,5 +597,11 @@ class TestDragAndDropAssessmentData(AssessmentModeFixture, unittest.TestCase):
             3: self.ZONE_1,
             4: self.ZONE_2,
         })  # incorrect solution, 0 score
-        self.call_handler(self.DO_ATTEMPT_HANDLER, data={})
+        self._do_attempt()
         self.assertEqual(self.block.grade, expected_score)
+
+    def test_do_attempt_correct_takes_decoy_into_account(self):
+        self._submit_solution({0: self.ZONE_1, 1: self.ZONE_2, 2: self.ZONE_2, 3: self.ZONE_2})
+        res = self._do_attempt()
+
+        self.assertFalse(res['correct'])

tests/unit/test_basics.py

@@ -69,7 +69,6 @@ class BasicTests(TestCaseMixin, unittest.TestCase):
             "target_img_description": TARGET_IMG_DESCRIPTION,
             "item_background_color": None,
             "item_text_color": None,
-            "initial_feedback": START_FEEDBACK,
             "url_name": "",
         })
         self.assertEqual(zones, DEFAULT_DATA["zones"])