Merge pull request #101 from open-craft/jbzdak/show-popup-always-on-mcq

OC-1387 Always show tooltips feedback if tips are present.

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

@@ -80,6 +80,12 @@ class ProblemBuilderBaseTest(SeleniumXBlockTest, PopupCheckMixin):
         if load_immediately:
             return self.go_to_view("student_view")
 
+    def go_to_view(self, view_name):
+        """ Eliminate errors that come from the Workbench banner overlapping elements """
+        element = super(ProblemBuilderBaseTest, self).go_to_view(view_name)
+        self.browser.execute_script('document.querySelectorAll("header.banner")[0].style.display="none";')
+        return element
+
     def click_submit(self, mentoring):
         """ Click the submit button and wait for the response """
         submit = mentoring.find_element_by_css_selector('.submit input.input-main')
@@ -102,6 +108,12 @@ class MentoringBaseTest(SeleniumBaseTest, PopupCheckMixin):
 
     __asides_patch = None
 
+    def go_to_page(self, page_title, **kwargs):
+        """ Eliminate errors that come from the Workbench banner overlapping elements """
+        element = super(MentoringBaseTest, self).go_to_page(page_title, **kwargs)
+        self.browser.execute_script('document.querySelectorAll("header.banner")[0].style.display="none";')
+        return element
+
     @classmethod
     def setUpClass(cls):
         super(MentoringBaseTest, cls).setUpClass()

problem_builder/tests/integration/test_answer.py

@@ -40,8 +40,6 @@ class AnswerBlockTest(MentoringBaseTest):
 
         # Another answer with the same name
         mentoring = self.go_to_page('Answer Edit 1')
-        header1 = self.browser.find_element_by_css_selector('h1')
-        self.assertEqual(header1.text, 'XBlock: Answer Edit 1')
 
         # Check <html> child
         p = mentoring.find_element_by_css_selector('p')

problem_builder/tests/integration/test_assessment.py

@@ -23,28 +23,6 @@ from .base_test import CORRECT, INCORRECT, PARTIAL, MentoringAssessmentBaseTest,
 
 @ddt
 class MentoringAssessmentTest(MentoringAssessmentBaseTest):
-    def _selenium_bug_workaround_scroll_to(self, mentoring, question):
-        """Workaround for selenium bug:
-
-        Some version of Selenium has a bug that prevents scrolling
-        to radiobuttons before being clicked. The click not taking
-        place, when it's outside the view.
-
-        Since the bug does not affect other content, asking Selenium
-        to click on the legend first, will properly scroll it.
-
-        It also have it's fair share of issues with the workbench header.
-
-        For this reason we click on the bottom-most element, scrolling to it.
-        Then, click on the title of the question (also scrolling to it)
-        hopefully, this gives us enough room for the full step with the
-        control buttons to fit.
-        """
-        controls = mentoring.find_element_by_css_selector("div.submit")
-        title = question.find_element_by_css_selector("h3.question-title")
-        controls.click()
-        title.click()
-
     def assert_persistent_elements_present(self, mentoring):
         self.assertIn("A Simple Assessment", mentoring.text)
         self.assertIn("This paragraph is shared between all questions.", mentoring.text)
@@ -53,9 +31,8 @@ class MentoringAssessmentTest(MentoringAssessmentBaseTest):
         self.browser.get(self.live_server_url)
 
     def freeform_answer(self, number, mentoring, controls, text_input, result, saved_value="", last=False):
-        question = self.expect_question_visible(number, mentoring)
+        self.expect_question_visible(number, mentoring)
         self.assert_persistent_elements_present(mentoring)
-        self._selenium_bug_workaround_scroll_to(mentoring, question)
 
         answer = mentoring.find_element_by_css_selector("textarea.answer.editable")
 
@@ -112,9 +89,8 @@ class MentoringAssessmentTest(MentoringAssessmentBaseTest):
         self.do_post(controls, last)
 
     def rating_question(self, number, mentoring, controls, choice_name, result, last=False):
-        question = self.expect_question_visible(number, mentoring)
+        self.expect_question_visible(number, mentoring)
         self.assert_persistent_elements_present(mentoring)
-        self._selenium_bug_workaround_scroll_to(mentoring, question)
         self.assertIn("How much do you rate this MCQ?", mentoring.text)
 
         self.assert_disabled(controls.submit)

problem_builder/tests/integration/test_mentoring.py

@@ -205,7 +205,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):
@@ -354,3 +354,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

@@ -32,19 +32,6 @@ from .base_test import MentoringBaseTest
 
 @ddt.ddt
 class QuestionnaireBlockTest(MentoringBaseTest):
-
-    def _selenium_bug_workaround_scroll_to(self, mcq_legend):
-        """Workaround for selenium bug:
-
-        Some version of Selenium has a bug that prevents scrolling
-        to radiobuttons before being clicked. The click not taking
-        place, when it's outside the view.
-
-        Since the bug does not affect other content, asking Selenium
-        to click on the legend first, will properly scroll it.
-        """
-        mcq_legend.click()
-
     def _get_choice_label_text(self, choice):
         return choice.find_element_by_css_selector('label').text
 
@@ -75,9 +62,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,8 +87,10 @@ class QuestionnaireBlockTest(MentoringBaseTest):
             ['1', '2', '3', '4', '5', 'notwant']
         )
 
-        def submit_answer_and_assert_messages(mcq1_answer, mcq2_answer, item_feedback1, item_feedback2):
-            self._selenium_bug_workaround_scroll_to(mcq1)
+        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"):
 
             mcq1_choices_input[mcq1_answer].click()
             mcq2_choices_input[mcq2_answer].click()
@@ -112,28 +98,19 @@ 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())
 
         # Submit button stays disabled when there are unfinished mcqs
-        self._selenium_bug_workaround_scroll_to(mcq1)
         mcq1_choices_input[1].click()
         self.assertFalse(submit.is_enabled())
 
@@ -142,6 +119,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)
@@ -244,7 +229,6 @@ class QuestionnaireBlockTest(MentoringBaseTest):
         self.assertFalse(submit.is_enabled())
 
         inputs = choices_list.find_elements_by_css_selector('.choice-selector input')
-        self._selenium_bug_workaround_scroll_to(choices_list)
         inputs[0].click()
         inputs[1].click()
         inputs[2].click()

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>