StepBuilder: Fix inconsistent feedback for MCQs.

problem_builder/public/js/questionnaire.js

@@ -195,7 +195,7 @@ function MRQBlock(runtime, element) {
 
         handleReview: function(result) {
             $.each(result.submissions, function (index, value) {
-                $('input[type="checkbox"][value="' + value + '"]').prop('checked', true)
+                $('input[type="checkbox"][value="' + value + '"]').prop('checked', true);
             });
             $('input', element).prop('disabled', true);
         },

problem_builder/public/js/step.js

@@ -89,8 +89,15 @@ function MentoringStepBlock(runtime, element) {
                 var child = children[i];
                 if (child && child.name !== undefined) { // Check if we are dealing with a question
                     var result = results[child.name];
-                    callIfExists(child, 'handleSubmit', result, options);
+                    // Call handleReview first to ensure that choice-level feedback for MCQs is displayed:
+                    // Before displaying feedback for a given choice, handleSubmit checks if it is selected.
+                    // (If it isn't, we don't want to display feedback for it.)
+                    // handleReview is responsible for setting the "checked" property to true
+                    // for each choice that the student selected as part of their most recent submission.
+                    // If it is called after handleSubmit, the check mentioned above will fail,
+                    // and no feedback will be displayed.
                     callIfExists(child, 'handleReview', result);
+                    callIfExists(child, 'handleSubmit', result, options);
                 }
             }
         },
@@ -100,7 +107,7 @@ function MentoringStepBlock(runtime, element) {
         },
 
         hasQuestion: function() {
-            return $('.sb-step', element).data('has-question')
+            return $('.sb-step', element).data('has-question');
         },
 
         updateChildren: function() {

problem_builder/tests/integration/test_step_builder.py

 from mock import patch
-from ddt import ddt, data
+from ddt import ddt, data, unpack
 from selenium.common.exceptions import NoSuchElementException
 from selenium.webdriver.support.ui import WebDriverWait
 
@@ -497,6 +497,64 @@ class StepBuilderTest(MentoringAssessmentBaseTest, MultipleSliderBlocksTestMixin
         if extended_feedback:
             self.extended_feedback_checks(step_builder, controls, expected_results)
 
+    @data(
+        (0, "Yes", "Great!", True, CORRECT),
+        (1, "Maybe not", "Ah, damn.", False, INCORRECT),
+        (2, "I don't understand", "Really?", False, INCORRECT),
+    )
+    @unpack
+    def test_mcq_feedback(self, choice_index, choice_text, expected_feedback, correct, step_result):
+        params = {
+            "max_attempts": 1,
+            "extended_feedback": True,
+        }
+        step_builder, controls = self.load_assessment_scenario("step_builder_mcq_feedback.xml", params)
+
+        # Step 1
+        # Submit MCQ, go to review step
+        self.single_choice_question(None, step_builder, controls, choice_text, step_result, last=True)
+
+        # Check MCQ feedback
+        self.review_mcq(step_builder, choice_index, expected_feedback, correct)
+
+        # Reload page
+        self.browser.execute_script("$(document).html(' ');")
+        step_builder, controls = self.go_to_assessment()
+
+        # Check MCQ feedback
+        self.review_mcq(step_builder, choice_index, expected_feedback, correct)
+
+        # Go back to review step
+        controls.review_link.click()
+
+        # Check MCQ feedback
+        self.review_mcq(step_builder, choice_index, expected_feedback, correct)
+
+    def review_mcq(self, step_builder, choice_index, expected_feedback, correct):
+        correctness = 'correct' if correct else 'incorrect'
+        mcq_link = step_builder.find_elements_by_css_selector('.{}-list li a'.format(correctness))[0]
+        mcq_link.click()
+        mcq = step_builder.find_element_by_css_selector(".xblock-v1[data-name='mcq_1_1']")
+        self.assert_choice_feedback(mcq, choice_index, expected_feedback, correct)
+
+    def assert_choice_feedback(self, mcq, choice_index, expected_text, correct=True):
+        """
+        Asserts that feedback for given element contains particular text
+        """
+        choice = mcq.find_elements_by_css_selector(".choices-list .choice")[choice_index]
+        choice_result = choice.find_element_by_css_selector('.choice-result')
+        feedback_popup = choice.find_element_by_css_selector(".choice-tips")
+        checkmark_class = 'checkmark-correct' if correct else 'checkmark-incorrect'
+
+        self.wait_until_visible(feedback_popup)
+        self.assertEqual(feedback_popup.text, expected_text)
+        self.assert_choice_result(choice_result, checkmark_class=checkmark_class)
+
+    def assert_choice_result(self, choice_result, checkmark_class):
+        result_classes = choice_result.get_attribute('class').split()
+        self.wait_until_visible(choice_result)
+        self.assertIn(checkmark_class, result_classes)
+
     def test_review_tips(self):
         params = {
             "max_attempts": 3,

problem_builder/tests/integration/xml_templates/step_builder_mcq_feedback.xml

+<step-builder url_name="step-builder" display_name="Step Builder"
+              max_attempts="{{max_attempts}}" extended_feedback="{{extended_feedback}}">
+
+  <sb-step display_name="First step">
+    <pb-mcq name="mcq_1_1" question="Do you like this MCQ?" correct_choices='["yes"]'>
+        <pb-choice value="yes">Yes</pb-choice>
+        <pb-choice value="maybenot">Maybe not</pb-choice>
+        <pb-choice value="understand">I don't understand</pb-choice>
+
+        <pb-tip values='["yes"]'>Great!</pb-tip>
+        <pb-tip values='["maybenot"]'>Ah, damn.</pb-tip>
+        <pb-tip values='["understand"]'><div id="test-custom-html">Really?</div></pb-tip>
+    </pb-mcq>
+  </sb-step>
+
+  <sb-review-step>
+    <sb-review-score/>
+  </sb-review-step>
+
+</step-builder>