OC-1387 Show tooltip feedback if tips are present straightaway

problem_builder/mcq.py

@@ -51,6 +51,16 @@ class MCQBlock(SubmittingXBlockMixin, QuestionnaireAbstractBlock):
     CATEGORY = 'pb-mcq'
     STUDIO_LABEL = _(u"Multiple Choice Question")
 
+    message = String(
+        display_name=_("Message"),
+        help=_(
+            "General feedback provided when submitting. "
+            "(This is not shown if there is a more specific feedback tip for the choice selected by the learner.)"
+        ),
+        scope=Scope.content,
+        default=""
+    )
+
     student_choice = String(
         # {Last input submitted by the student
         default="",
@@ -64,7 +74,7 @@ class MCQBlock(SubmittingXBlockMixin, QuestionnaireAbstractBlock):
         list_values_provider=QuestionnaireAbstractBlock.choice_values_provider,
         list_style='set',  # Underered, unique items. Affects the UI editor.
     )
-    editable_fields = QuestionnaireAbstractBlock.editable_fields + ('correct_choices', )
+    editable_fields = QuestionnaireAbstractBlock.editable_fields + ('message', 'correct_choices', )
 
     def describe_choice_correctness(self, choice_value):
         if choice_value in self.correct_choices:

problem_builder/mrq.py

@@ -22,7 +22,7 @@
 
 import logging
 
-from xblock.fields import List, Scope, Boolean
+from xblock.fields import List, Scope, Boolean, String
 from xblock.validation import ValidationMessage
 from .questionnaire import QuestionnaireAbstractBlock
 from xblockutils.resources import ResourceLoader
@@ -71,6 +71,12 @@ class MRQBlock(QuestionnaireAbstractBlock):
         list_style='set',  # Underered, unique items. Affects the UI editor.
         default=[],
     )
+    message = String(
+        display_name=_("Message"),
+        help=_("General feedback provided when submitting"),
+        scope=Scope.content,
+        default=""
+    )
     hide_results = Boolean(display_name="Hide results", scope=Scope.content, default=False)
     editable_fields = (
         'question', 'required_choices', 'ignored_choices', 'message', 'display_name',

problem_builder/public/js/questionnaire.js

@@ -123,43 +123,30 @@ function MCQBlock(runtime, element) {
 
             var messageView = MessageView(element, mentoring);
 
-            if (result.message) {
-                var msg = '<div class="message-content">' + result.message + '</div>' +
-                          '<div class="close icon-remove-sign fa-times-circle"></div>';
-                messageView.showMessage(msg);
-            } else { messageView.clearResult(); }
-
-            display_message(result.message, messageView, options.checkmark);
+            messageView.clearResult();
 
-            var choiceInputs = $('.choice-selector input', element);
-            $.each(choiceInputs, function(index, choiceInput) {
-                var choiceInputDOM = $(choiceInput);
+            var choiceInputDOM = $('.choice-selector input[value="'+ result.submission +'"]');
             var choiceDOM = choiceInputDOM.closest('.choice');
             var choiceResultDOM = $('.choice-result', choiceDOM);
             var choiceTipsDOM = $('.choice-tips', choiceDOM);
 
-                if (choiceInputDOM.prop('checked')) {  // We're showing previous answers,
-                                                       // so go ahead and display results as well
-                    if (result.status === "correct" && choiceInputDOM.val() === result.submission) {
-                        choiceDOM.addClass('correct');
+            // We're showing previous answers, so go ahead and display results as well
+            if (choiceInputDOM.prop('checked')) {
+                display_message(result.message, messageView, options.checkmark);
+                if (result.status === "correct") {
+                    choiceInputDOM.addClass('correct');
                     choiceResultDOM.addClass('checkmark-correct icon-ok fa-check');
-                    }
-                    else if (choiceInputDOM.val() === result.submission || _.isNull(result.submission)) {
+                } else {
                     choiceDOM.addClass('incorrect');
                     choiceResultDOM.addClass('checkmark-incorrect icon-exclamation fa-exclamation');
                 }
 
-                    if (result.tips && choiceInputDOM.val() === result.submission) {
+                if (result.tips) {
                     mentoring.setContent(choiceTipsDOM, result.tips);
-                    }
-
-                    choiceResultDOM.off('click').on('click', function() {
-                        if (choiceTipsDOM.html() !== '') {
                     messageView.showMessage(choiceTipsDOM);
                 }
-                    });
             }
-            });
+
 
             if (_.isNull(result.submission)) {
                 messageView.showMessage('<div class="message-content">You have not provided an answer.</div>' +

problem_builder/questionnaire.py

@@ -68,13 +68,8 @@ class QuestionnaireAbstractBlock(
         default="",
         multiline_editor=True,
     )
-    message = String(
-        display_name=_("Message"),
-        help=_("General feedback provided when submiting"),
-        scope=Scope.content,
-        default=""
-    )
-    editable_fields = ('question', 'message', 'weight', 'display_name', 'show_title')
+
+    editable_fields = ('question', 'weight', 'display_name', 'show_title')
     has_children = True
     answerable = True
 

problem_builder/tests/integration/base_test.py

@@ -63,7 +63,24 @@ class PopupCheckMixin(object):
             self.assertFalse(item_feedback_popup.is_displayed())
 
 
-class ProblemBuilderBaseTest(SeleniumXBlockTest, PopupCheckMixin):
+class ScrollToMixin(object):
+
+    def scroll_to(self, component, offset=100):
+        """
+        Scrolls browser viewport so component is visible. In rare cases you might
+        need to provide an offset, which will change position by some amount
+        of pixels.
+        :return:
+        """
+        self.browser.execute_script(
+                "return window.scrollTo(0, arguments[0]);",
+                component.location['y']+offset)
+        self.browser.execute_script(
+                "return window.scrollTo(0, arguments[0]);",
+                component.location['y']-offset)
+
+
+class ProblemBuilderBaseTest(SeleniumXBlockTest, PopupCheckMixin, ScrollToMixin):
     """
     The new base class for integration tests.
     Scenarios can be loaded and edited on the fly.

problem_builder/tests/integration/test_mentoring.py

@@ -137,12 +137,16 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         self.assertFalse(choice_input.is_selected())
 
     def _standard_filling(self, answer, mcq, mrq, rating):
+        self.scroll_to(answer)
         answer.send_keys('This is the answer')
+        self.scroll_to(mcq)
         self.click_choice(mcq, "Yes")
         # 1st, 3rd and 4th options, first three are correct, i.e. two mistakes: 2nd and 4th
+        self.scroll_to(mrq, 300)
         self.click_choice(mrq, "Its elegance")
         self.click_choice(mrq, "Its gracefulness")
         self.click_choice(mrq, "Its bugs")
+        self.scroll_to(rating)
         self.click_choice(rating, "4")
 
     # mcq and rating can't be reset easily, but it's not required; listing them here to keep method signature similar
@@ -153,8 +157,11 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
                 checkbox.click()
 
     def _standard_checks(self, answer, mcq, mrq, rating, messages):
+        self.scroll_to(answer)
         self.assertEqual(answer.get_attribute('value'), 'This is the answer')
+        self.scroll_to(mcq)
         self._assert_feedback_showed(mcq, 0, "Great!", click_choice_result=True)
+        self.scroll_to(mrq, 300)
         self._assert_feedback_showed(
             mrq, 0, "This is something everyone has to like about this MRQ",
             click_choice_result=True
@@ -165,18 +172,23 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         )
         self._assert_feedback_showed(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True)
         self._assert_feedback_showed(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False)
+        self.scroll_to(rating)
         self._assert_feedback_showed(rating, 3, "I love good grades.", click_choice_result=True)
         self.assertTrue(messages.is_displayed())
+        self.scroll_to(messages)
         self.assertEqual(messages.text, "FEEDBACK\nNot done yet")
 
     def _feedback_customized_checks(self, answer, mcq, mrq, rating, messages):
         # Long answer: Previous answer and feedback visible
+        self.scroll_to(answer)
         self.assertEqual(answer.get_attribute('value'), 'This is the answer')
         # MCQ: Previous answer and feedback hidden
+        self.scroll_to(mcq)
         for i in range(3):
             self._assert_feedback_hidden(mcq, i)
             self._assert_not_checked(mcq, i)
         # MRQ: Previous answer and feedback visible
+        self.scroll_to(mrq, 300)
         self._assert_feedback_showed(
             mrq, 0, "This is something everyone has to like about this MRQ",
             click_choice_result=True
@@ -188,11 +200,13 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         self._assert_feedback_showed(mrq, 2, "This MRQ is indeed very graceful", click_choice_result=True)
         self._assert_feedback_showed(mrq, 3, "Nah, there aren't any!", click_choice_result=True, success=False)
         # Rating: Previous answer and feedback hidden
+        self.scroll_to(rating)
         for i in range(5):
             self._assert_feedback_hidden(rating, i)
             self._assert_not_checked(rating, i)
         # Messages
         self.assertTrue(messages.is_displayed())
+        self.scroll_to(messages)
         self.assertEqual(messages.text, "FEEDBACK\nNot done yet")
 
     def reload_student_view(self):
@@ -205,7 +219,7 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
             return title and title.text == "XBlock scenarios"
         wait.until(did_load_homepage, u"Workbench home page should have loaded")
         mentoring = self.go_to_view("student_view")
-        self.wait_until_visible(self._get_messages_element(mentoring))
+        self.wait_until_visible(self._get_xblock(mentoring, "feedback_mcq_2"))
         return mentoring
 
     def test_feedbacks_and_messages_is_not_shown_on_first_load(self):
@@ -271,8 +285,11 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
             self.assertFalse(submit.is_enabled())
 
             # ... until student answers MCQs again
+            self.scroll_to(mcq)
             self.click_choice(mcq, "Maybe not")
+            self.scroll_to(rating)
             self.click_choice(rating, "2")
+            self.scroll_to(submit)
             self.assertTrue(submit.is_enabled())
 
     def test_given_perfect_score_in_past_loads_current_result(self):
@@ -354,3 +371,26 @@ class ProblemBuilderQuestionnaireBlockTest(ProblemBuilderBaseTest):
         answer, mcq, mrq, rating = self._get_controls(mentoring)
         messages = self._get_messages_element(mentoring)
         assert_state(answer, mcq, mrq, rating, messages)
+
+    @ddt.unpack
+    @ddt.data(
+        # MCQ with tips
+        ("feedback_persistence_mcq_tips.xml", '.choice-tips'),
+        # Like the above but instead of tips in MCQ
+        # has a question level feedback. This feedback should also be suppressed.
+        ("feedback_persistence_mcq_no_tips.xml", '.feedback')
+    )
+    def test_feedback_persistence_tips(self, scenario, tips_selector):
+        # Tests whether feedback is hidden on reload.
+        with mock.patch("problem_builder.mentoring.MentoringBlock.get_options") as patched_options:
+            patched_options.return_value = {'pb_mcq_hide_previous_answer': True}
+            mentoring = self.load_scenario(scenario)
+            mcq = self._get_xblock(mentoring, "feedback_mcq_2")
+            messages = mentoring.find_element_by_css_selector(tips_selector)
+            self.assertFalse(messages.is_displayed())
+            self.click_choice(mcq, "Yes")
+            self.click_submit(mentoring)
+            self.assertTrue(messages.is_displayed())
+            mentoring = self.reload_student_view()
+            messages = mentoring.find_element_by_css_selector(tips_selector)
+            self.assertFalse(messages.is_displayed())

problem_builder/tests/integration/test_questionnaire.py

@@ -75,9 +75,6 @@ class QuestionnaireBlockTest(MentoringBaseTest):
         self.assertEqual(mcq1.find_element_by_css_selector('legend').text, 'Question 1\nDo you like this MCQ?')
         self.assertEqual(mcq2.find_element_by_css_selector('legend').text, 'Question 2\nHow do you rate this MCQ?')
 
-        mcq1_feedback = mcq1.find_element_by_css_selector('.feedback')
-        mcq2_feedback = mcq2.find_element_by_css_selector('.feedback')
-
         mcq1_choices = mcq1.find_elements_by_css_selector('.choices .choice')
         mcq2_choices = mcq2.find_elements_by_css_selector('.rating .choice')
 
@@ -103,7 +100,10 @@ class QuestionnaireBlockTest(MentoringBaseTest):
             ['1', '2', '3', '4', '5', 'notwant']
         )
 
-        def submit_answer_and_assert_messages(mcq1_answer, mcq2_answer, item_feedback1, item_feedback2):
+        def submit_answer_and_assert_messages(
+                mcq1_answer, mcq2_answer, item_feedback1, item_feedback2,
+                feedback1_selector=".choice-tips .tip p",
+                feedback2_selector=".choice-tips .tip p"):
             self._selenium_bug_workaround_scroll_to(mcq1)
 
             mcq1_choices_input[mcq1_answer].click()
@@ -112,22 +112,14 @@ class QuestionnaireBlockTest(MentoringBaseTest):
             submit.click()
             self.wait_until_disabled(submit)
 
-            mcq1_tips = mcq1.find_element_by_css_selector(".choice-tips .tip p")
-            mcq2_tips = mcq2.find_element_by_css_selector(".choice-tips .tip p")
+            mcq1_feedback = mcq1.find_element_by_css_selector(feedback1_selector)
+            mcq2_feedback = mcq2.find_element_by_css_selector(feedback2_selector)
 
+            self.assertEqual(mcq1_feedback.text, item_feedback1)
             self.assertTrue(mcq1_feedback.is_displayed())
-            self.assertEqual(mcq1_feedback.text, "Feedback message 1")
-            self.assertTrue(mcq2_feedback.is_displayed())
-            self.assertEqual(mcq2_feedback.text, "Feedback message 2")
-            self.assertFalse(mcq1_tips.is_displayed())
-            self.assertFalse(mcq2_tips.is_displayed())
 
-            self._click_result_icon(mcq1_choices[mcq1_answer])
-            self.assertEqual(mcq1_tips.text, item_feedback1)
-            self.assertTrue(mcq1_tips.is_displayed())
-            self._click_result_icon(mcq2_choices[mcq2_answer])
-            self.assertEqual(mcq2_tips.text, item_feedback2)
-            self.assertTrue(mcq2_tips.is_displayed())
+            self.assertEqual(mcq2_feedback.text, item_feedback2)
+            self.assertTrue(mcq2_feedback.is_displayed())
 
         # Submit button disabled without selecting anything
         self.assertFalse(submit.is_enabled())
@@ -142,6 +134,14 @@ class QuestionnaireBlockTest(MentoringBaseTest):
         self.assertEqual(messages.text, '')
         self.assertFalse(messages.is_displayed())
 
+        # When selected answers have no tips display generic feedback message
+        submit_answer_and_assert_messages(
+            1, 5, 'Feedback message 1', 'Feedback message 2',
+            ".feedback .message-content", ".feedback .message-content"
+        )
+        self.assertEqual(messages.text, '')
+        self.assertFalse(messages.is_displayed())
+
         # Should show full completion when the right answers are selected
         submit_answer_and_assert_messages(0, 3, 'Great!', 'I love good grades.')
         self.assertIn('All is good now...\nCongratulations!', messages.text)

problem_builder/tests/integration/test_titles.py

@@ -72,7 +72,7 @@ class StepTitlesTest(SeleniumXBlockTest):
     )
 
     mcq_template = """
-        <problem-builder mode="{{mode}}">
+        <problem-builder mode="{mode}">
             <pb-mcq name="mcq_1_1" question="Who was your favorite character?"
               correct_choices="[gaius,adama,starbuck,roslin,six,lee]"
               {display_name_attr} {show_title_attr}
@@ -88,7 +88,7 @@ class StepTitlesTest(SeleniumXBlockTest):
     """
 
     mrq_template = """
-        <problem-builder mode="{{mode}}">
+        <problem-builder mode="{mode}">
             <pb-mrq name="mrq_1_1" question="What makes a great MRQ?"
               ignored_choices="[1,2,3]"
               {display_name_attr} {show_title_attr}
@@ -101,7 +101,7 @@ class StepTitlesTest(SeleniumXBlockTest):
     """
 
     rating_template = """
-        <problem-builder mode="{{mode}}">
+        <problem-builder mode="{mode}">
             <pb-rating name="rating_1_1" question="How do you rate Battlestar Galactica?"
               correct_choices="[5,6]"
               {display_name_attr} {show_title_attr}
@@ -112,7 +112,7 @@ class StepTitlesTest(SeleniumXBlockTest):
     """
 
     long_answer_template = """
-        <problem-builder mode="{{mode}}">
+        <problem-builder mode="{mode}">
             <pb-answer name="answer_1_1" question="What did you think of the ending?"
               {display_name_attr} {show_title_attr} />
         </problem-builder>

problem_builder/tests/integration/xml/mcq_1.xml

@@ -6,7 +6,6 @@
             <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>
 
@@ -17,7 +16,6 @@
 
             <pb-tip values='["4","5"]'>I love good grades.</pb-tip>
             <pb-tip values='["1","2","3"]'>Will do better next time...</pb-tip>
-            <pb-tip values='["notwant"]'>Your loss!</pb-tip>
         </pb-rating>
 
         <pb-message type="completed">

problem_builder/tests/integration/xml_templates/feedback_persistence_mcq_no_tips.xml

+<vertical_demo>
+    <problem-builder url_name="feedback_tips" enforce_dependency="false">
+
+        <pb-mcq name="feedback_mcq_2" question="Do you like this MCQ?" correct_choices='["yes"]' message="Question level Feedback">
+            <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-mcq>
+
+    </problem-builder>
+</vertical_demo>

problem_builder/tests/integration/xml_templates/feedback_persistence_mcq_tips.xml

+<vertical_demo>
+    <problem-builder url_name="feedback_no_tips" enforce_dependency="false">
+
+        <pb-mcq name="feedback_mcq_2" 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>
+
+    </problem-builder>
+</vertical_demo>