diff --git a/README.md b/README.md index 8c0584284..4bba87a98 100644 --- a/README.md +++ b/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. diff --git a/drag_and_drop_v2/drag_and_drop_v2.py b/drag_and_drop_v2/drag_and_drop_v2.py index bb15de080..54bdb2328 100644 --- a/drag_and_drop_v2/drag_and_drop_v2.py +++ b/drag_and_drop_v2/drag_and_drop_v2.py @@ -246,7 +246,6 @@ def items_without_answers(): "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 @@ def do_attempt(self, data, suffix=''): 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 @@ def _validate_do_attempt(self): 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,16 +521,14 @@ def _add_msg_if_exists(ids_list, message_template, message_class): message = message_template(len(ids_list), self.i18n_service.ngettext) feedback_msgs.append(FeedbackMessage(message, message_class)) - _add_msg_if_exists( - 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.item_state or include_item_feedback: + _add_msg_if_exists( + 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 self.attempts_remain and (misplaced_ids or missing_ids): problem_feedback_message = self.data['feedback']['start'] @@ -539,7 +548,7 @@ def _add_msg_if_exists(ids_list, message_template, message_class): 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 @@ def _drop_item_standard(self, item_attempt): 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 @@ def _mark_complete_and_publish_grade(self): """ 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 @@ def _get_user_state(self): '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 @@ def _get_grade(self): """ 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 @@ def _answer_correctness(self): * 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 diff --git a/drag_and_drop_v2/public/css/drag_and_drop.css b/drag_and_drop_v2/public/css/drag_and_drop.css index 08dd1db4a..6e4992034 100644 --- a/drag_and_drop_v2/public/css/drag_and_drop.css +++ b/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; } diff --git a/drag_and_drop_v2/public/js/drag_and_drop.js b/drag_and_drop_v2/public/js/drag_and_drop.js index 7b763cf06..24b6e2b5e 100644 --- a/drag_and_drop_v2/public/js/drag_and_drop.js +++ b/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,33 +1058,33 @@ 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; } - state.closing = true; - previousFeedback = state.feedback; - if (target.is(close_button)) { - state.manually_closed = true; - } else { - state.manually_closed = false; - } - + 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; + state.manually_closed = manually_closed; + } + }; + var resetProblem = function(evt) { evt.preventDefault(); $.ajax({ @@ -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(), diff --git a/drag_and_drop_v2/translations/en/LC_MESSAGES/text.po b/drag_and_drop_v2/translations/en/LC_MESSAGES/text.po index fd55b0a32..bd8661ccc 100644 --- a/drag_and_drop_v2/translations/en/LC_MESSAGES/text.po +++ b/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] "" diff --git a/drag_and_drop_v2/translations/eo/LC_MESSAGES/text.po b/drag_and_drop_v2/translations/eo/LC_MESSAGES/text.po index d4a6d7bfb..d982bc9df 100644 --- a/drag_and_drop_v2/translations/eo/LC_MESSAGES/text.po +++ b/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. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє¢т#" diff --git a/drag_and_drop_v2/utils.py b/drag_and_drop_v2/utils.py index d9ffab2be..f35e50739 100644 --- a/drag_and_drop_v2/utils.py +++ b/drag_and_drop_v2/utils.py @@ -42,7 +42,6 @@ class MessageClasses(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): diff --git a/tests/integration/test_base.py b/tests/integration/test_base.py index 65812b488..43bdeed5d 100644 --- a/tests/integration/test_base.py +++ b/tests/integration/test_base.py @@ -214,6 +214,8 @@ def _get_scenario_xml(self): # pylint: disable=no-self-use class InteractionTestBase(object): + POPUP_ERROR_CLASS = "popup-incorrect" + @classmethod def _get_items_with_zone(cls, items_map): return { @@ -309,7 +311,7 @@ def wait_until_ondrop_xhr_finished(elem): 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,7 +321,8 @@ def place_item(self, item_value, zone_id, action_key=None): self.drag_item_to_zone(item_value, zone_id) else: self.move_item_to_zone(item_value, zone_id, action_key) - self.wait_for_ajax() + if wait: + self.wait_for_ajax() def drag_item_to_zone(self, item_value, zone_id): """ @@ -429,3 +432,12 @@ def _switch_to_block(self, idx): """ 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) diff --git a/tests/integration/test_events.py b/tests/integration/test_events.py index 9f71279db..d1c6ccc5e 100644 --- a/tests/integration/test_events.py +++ b/tests/integration/test_events.py @@ -1,16 +1,28 @@ -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 "" @@ -71,6 +75,68 @@ def test_event(self, index, event): 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']) diff --git a/tests/integration/test_interaction.py b/tests/integration/test_interaction.py index 5c0623b58..3fd8d83d0 100644 --- a/tests/integration/test_interaction.py +++ b/tests/integration/test_interaction.py @@ -42,8 +42,8 @@ def parameterized_item_positive_feedback_on_good_move( 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, "

{}

".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 @@ def parameterized_item_negative_feedback_on_bad_move( 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 @@ def test_multiple_positive_feedback(self): 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 @@ def test_item_returned_to_bank(self): 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) diff --git a/tests/integration/test_interaction_assessment.py b/tests/integration/test_interaction_assessment.py index 481d2438b..c6b21497a 100644 --- a/tests/integration/test_interaction_assessment.py +++ b/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 @@ def test_do_attempt_feedback_is_updated(self): 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 @@ def test_grade(self): 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 = [ + "
  • {msg}
  • ".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 @@ def _get_scenario_xml(self): 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) diff --git a/tests/integration/test_render.py b/tests/integration/test_render.py index 7eecee403..30326fc6b 100644 --- a/tests/integration/test_render.py +++ b/tests/integration/test_render.py @@ -209,7 +209,7 @@ def test_popup(self): 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') diff --git a/tests/integration/test_title_and_question.py b/tests/integration/test_title_and_question.py index 705fcda00..8a882feb4 100644 --- a/tests/integration/test_title_and_question.py +++ b/tests/integration/test_title_and_question.py @@ -26,7 +26,7 @@ def test_problem_parameters(self, problem_text, show_problem_header): 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 @@ def test_title_parameters(self, _, display_name, show_title): 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') diff --git a/tests/unit/data/assessment/config_out.json b/tests/unit/data/assessment/config_out.json index e4b9d3066..f44b8a969 100644 --- a/tests/unit/data/assessment/config_out.json +++ b/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", diff --git a/tests/unit/data/html/config_out.json b/tests/unit/data/html/config_out.json index 87f54a06e..28c1d4545 100644 --- a/tests/unit/data/html/config_out.json +++ b/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 Intro Feed", "display_zone_borders": false, "display_zone_labels": false, "url_name": "unique_name", diff --git a/tests/unit/data/old/config_out.json b/tests/unit/data/old/config_out.json index 8cc1e5100..9a673e45c 100644 --- a/tests/unit/data/old/config_out.json +++ b/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": "", diff --git a/tests/unit/data/plain/config_out.json b/tests/unit/data/plain/config_out.json index 8957e22a4..129f63631 100644 --- a/tests/unit/data/plain/config_out.json +++ b/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", diff --git a/tests/unit/test_advanced.py b/tests/unit/test_advanced.py index dc134c223..14bb4c458 100644 --- a/tests/unit/test_advanced.py +++ b/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 @@ def initial_settings(cls): 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 @@ def test_drop_item_final(self): "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 @@ def _set_final_attempt(self): 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 @@ def test_get_user_state_no_attempts(self): ] 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 @@ def test_do_attempt_correct_mark_complete_and_publish_grade(self): 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 @@ def test_do_attempt_post_correct_no_publish_grade(self): 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 @@ def test_do_attempt_misplaced_ids(self): 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 @@ def _submit_incorrect_solution(self): 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 @@ def test_do_attempt_feedback_partial(self): 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 @@ def test_do_attempt_keeps_highest_score(self): 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 @@ def test_do_attempt_check_score_with_decoy(self): 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 @@ def test_do_attempt_zero_score_with_all_decoys(self): 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']) diff --git a/tests/unit/test_basics.py b/tests/unit/test_basics.py index 5d21c6de0..1abfd9ff6 100644 --- a/tests/unit/test_basics.py +++ b/tests/unit/test_basics.py @@ -69,7 +69,6 @@ def test_get_configuration(self): "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"])