Adresses TNL-3096

Add acceptance test for TNL-3096

Adress comments on pull request

Fix test failing and img issue

Modify tests

common/lib/capa/capa/responsetypes.py

@@ -43,7 +43,7 @@ from datetime import datetime
 from pytz import UTC
 from .util import (
     compare_with_tolerance, contextualize_text, convert_files_to_filenames,
-    is_list_of_files, find_with_default, default_tolerance
+    is_list_of_files, find_with_default, default_tolerance, get_inner_html_from_xpath
 )
 from lxml import etree
 from lxml.html.soupparser import fromstring as fromstring_bs     # uses Beautiful Soup!!! FIXME?
@@ -312,9 +312,10 @@ class LoncapaResponse(object):
         # 1. Establish the hint_texts
         # This can lead to early-exit if the hint is blank.
         if not hint_log:
-            if hint_node is None or hint_node.text is None:  # .text can be None, maybe just in testing
+            # .text can be None when node has immediate children nodes
+            if hint_node is None or (hint_node.text is None and len(hint_node.getchildren()) == 0):
                 return ''
-            hint_text = hint_node.text.strip()
+            hint_text = get_inner_html_from_xpath(hint_node)
             if not hint_text:
                 return ''
             hint_log = [{'text': hint_text}]
@@ -1051,7 +1052,7 @@ class ChoiceResponse(LoncapaResponse):
             hint_nodes = choice.findall('./choicehint')
             for hint_node in hint_nodes:
                 if hint_node.get('selected', '').lower() == selector:
-                    text = hint_node.text.strip()
+                    text = get_inner_html_from_xpath(hint_node)
                     if hint_node.get('label') is not None:  # tricky: label '' vs None is significant
                         label = hint_node.get('label')
                         label_count += 1

common/lib/capa/capa/tests/test_files/extended_hints_multiple_choice_with_html.xml

+<problem>
+<p>Select the fruit from the list</p>
+<multiplechoiceresponse>
+  <choicegroup label="Select the fruit from the list" type="MultipleChoice">
+    <choice correct="false">Mushroom
+        <choicehint>Mushroom <img src="#" ale="#"/>is a fungus, not a fruit.</choicehint>
+    </choice>
+    <choice correct="false">Potato
+        <choicehint>Potato is <img src="#" ale="#"/> not a fruit.</choicehint></choice>
+    <choice correct="true">Apple
+        <choicehint><a href="#">Apple</a> is a fruit.</choicehint>
+    </choice>
+  </choicegroup>
+</multiplechoiceresponse>
+</problem>

common/lib/capa/capa/tests/test_hint_functionality.py

@@ -65,21 +65,21 @@ class TextInputHintsTest(HintTest):
 
     @data(
         {'problem_id': u'1_2_1', u'choice': u'GermanyΩ',
-         'expected_string': u'<div class="feedback-hint-incorrect"><div class="hint-label">Incorrect: </div><div class="hint-text">I do not think so.Ω</div></div>'},
+         'expected_string': u'<div class="feedback-hint-incorrect"><div class="hint-label">Incorrect: </div><div class="hint-text">I do not think so.&#937;</div></div>'},
         {'problem_id': u'1_2_1', u'choice': u'franceΩ',
-         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Viva la France!Ω</div></div>'},
+         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Viva la France!&#937;</div></div>'},
         {'problem_id': u'1_2_1', u'choice': u'FranceΩ',
-         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Viva la France!Ω</div></div>'},
+         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Viva la France!&#937;</div></div>'},
         {'problem_id': u'1_2_1', u'choice': u'Mexico',
          'expected_string': ''},
         {'problem_id': u'1_2_1', u'choice': u'USAΩ',
-         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Less well known, but yes, there is a Paris, Texas.Ω</div></div>'},
+         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Less well known, but yes, there is a Paris, Texas.&#937;</div></div>'},
         {'problem_id': u'1_2_1', u'choice': u'usaΩ',
-         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Less well known, but yes, there is a Paris, Texas.Ω</div></div>'},
+         'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">Less well known, but yes, there is a Paris, Texas.&#937;</div></div>'},
         {'problem_id': u'1_2_1', u'choice': u'uSAxΩ',
          'expected_string': u''},
         {'problem_id': u'1_2_1', u'choice': u'NICKLANDΩ',
-         'expected_string': u'<div class="feedback-hint-incorrect"><div class="hint-label">Incorrect: </div><div class="hint-text">The country name does not end in LANDΩ</div></div>'},
+         'expected_string': u'<div class="feedback-hint-incorrect"><div class="hint-label">Incorrect: </div><div class="hint-text">The country name does not end in LAND&#937;</div></div>'},
         {'problem_id': u'1_3_1', u'choice': u'Blue',
          'expected_string': u'<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text">The red light is scattered by water molecules leaving only blue light.</div></div>'},
         {'problem_id': u'1_3_1', u'choice': u'blue',
@@ -451,6 +451,40 @@ class MultpleChoiceHintsTest(HintTest):
 
 
 @ddt
+class MultpleChoiceHintsWithHtmlTest(HintTest):
+    """
+    This class consists of a suite of test cases to be run on the multiple choice problem represented by the XML below.
+
+    """
+    xml = load_fixture('extended_hints_multiple_choice_with_html.xml')
+    problem = new_loncapa_problem(xml)
+
+    def test_tracking_log(self):
+        """Test that the tracking log comes out right."""
+        self.problem.capa_module.reset_mock()
+        self.get_hint(u'1_2_1', u'choice_0')
+        self.problem.capa_module.runtime.track_function.assert_called_with(
+            'edx.problem.hint.feedback_displayed',
+            {'module_id': 'i4x://Foo/bar/mock/abc', 'problem_part_id': '1_1', 'trigger_type': 'single',
+             'student_answer': [u'choice_0'], 'correctness': False, 'question_type': 'multiplechoiceresponse',
+             'hint_label': 'Incorrect', 'hints': [{'text': 'Mushroom <img src="#" ale="#"/>is a fungus, not a fruit.'}]}
+        )
+
+    @data(
+        {'problem_id': u'1_2_1', 'choice': u'choice_0',
+         'expected_string': '<div class="feedback-hint-incorrect"><div class="hint-label">Incorrect: </div><div class="hint-text">Mushroom <img src="#" ale="#"/>is a fungus, not a fruit.</div></div>'},
+        {'problem_id': u'1_2_1', 'choice': u'choice_1',
+         'expected_string': '<div class="feedback-hint-incorrect"><div class="hint-label">Incorrect: </div><div class="hint-text">Potato is <img src="#" ale="#"/> not a fruit.</div></div>'},
+        {'problem_id': u'1_2_1', 'choice': u'choice_2',
+         'expected_string': '<div class="feedback-hint-correct"><div class="hint-label">Correct: </div><div class="hint-text"><a href="#">Apple</a> is a fruit.</div></div>'}
+    )
+    @unpack
+    def test_multiplechoice_hints(self, problem_id, choice, expected_string):
+        hint = self.get_hint(problem_id, choice)
+        self.assertEqual(hint, expected_string)
+
+
+@ddt
 class DropdownHintsTest(HintTest):
     """
     This class consists of a suite of test cases to be run on the drop down problem represented by the XML below.

common/lib/capa/capa/tests/test_util.py

@@ -2,9 +2,10 @@
 Tests capa util
 """
 import unittest
+from lxml import etree
 
 from . import test_capa_system
-from capa.util import compare_with_tolerance, sanitize_html
+from capa.util import compare_with_tolerance, sanitize_html, get_inner_html_from_xpath
 
 
 class UtilTest(unittest.TestCase):
@@ -118,3 +119,10 @@ class UtilTest(unittest.TestCase):
         queue_msg = "<{0}>Test message</{0}>".format(not_allowed_tag)
         expected = "&lt;script&gt;Test message&lt;/script&gt;"
         self.assertEqual(sanitize_html(queue_msg), expected)
+
+    def test_get_inner_html_from_xpath(self):
+        """
+        Test for getting inner html as string from xpath node.
+        """
+        xpath_node = etree.XML('<hint style="smtng">aa<a href="#">bb</a>cc</hint>')  # pylint: disable=no-member
+        self.assertEqual(get_inner_html_from_xpath(xpath_node), 'aa<a href="#">bb</a>cc')

common/lib/capa/capa/util.py

@@ -6,6 +6,8 @@ from decimal import Decimal
 
 from calc import evaluator
 from cmath import isinf, isnan
+import re
+from lxml import etree
 #-----------------------------------------------------------------------------
 #
 # Utility functions used in CAPA responsetypes
@@ -181,3 +183,15 @@ def sanitize_html(html_code):
         attributes=attributes
     )
     return output
+
+
+def get_inner_html_from_xpath(xpath_node):
+    """
+    Returns inner html as string from xpath node.
+
+    """
+    # returns string from xpath node
+    html = etree.tostring(xpath_node).strip()  # pylint: disable=no-member
+    # strips outer tag from html string
+    inner_html = re.sub('(?ms)<%s[^>]*>(.*)</%s>' % (xpath_node.tag, xpath_node.tag), '\\1', html)
+    return inner_html.strip()

common/lib/xmodule/xmodule/capa_base.py

@@ -21,7 +21,7 @@ except ImportError:
 from capa.capa_problem import LoncapaProblem, LoncapaSystem
 from capa.responsetypes import StudentInputError, \
     ResponseError, LoncapaProblemError
-from capa.util import convert_files_to_filenames
+from capa.util import convert_files_to_filenames, get_inner_html_from_xpath
 from .progress import Progress
 from xmodule.exceptions import NotFoundError
 from xblock.fields import Scope, String, Boolean, Dict, Integer, Float
@@ -606,7 +606,7 @@ class CapaMixin(CapaFields):
 
         _ = self.runtime.service(self, "i18n").ugettext  # pylint: disable=redefined-outer-name
         hint_element = demand_hints[hint_index]
-        hint_text = hint_element.text.strip()
+        hint_text = get_inner_html_from_xpath(hint_element)
         if len(demand_hints) == 1:
             prefix = _('Hint: ')
         else:

common/test/acceptance/tests/lms/test_lms_problems.py

@@ -166,6 +166,91 @@ class ProblemExtendedHintTest(ProblemsTest, EventsTestMixin):
             actual_events)
 
 
+class ProblemHintWithHtmlTest(ProblemsTest, EventsTestMixin):
+    """
+    Tests that hints containing html get rendered properly
+    """
+
+    def get_problem(self):
+        """
+        Problem with extended hint features.
+        """
+        xml = dedent("""
+            <problem>
+            <p>question text</p>
+            <stringresponse answer="A">
+                <stringequalhint answer="B">aa <a href="#">bb</a> cc</stringequalhint>
+                <stringequalhint answer="C"><a href="#">aa bb</a> cc</stringequalhint>
+                <textline size="20"/>
+            </stringresponse>
+            <demandhint>
+              <hint>aa <a href="#">bb</a> cc</hint>
+              <hint><a href="#">dd  ee</a> ff</hint>
+            </demandhint>
+            </problem>
+        """)
+        return XBlockFixtureDesc('problem', 'PROBLEM HTML HINT TEST', data=xml)
+
+    def test_check_hint(self):
+        """
+        Test clicking Check shows the extended hint in the problem message.
+        """
+        self.courseware_page.visit()
+        problem_page = ProblemPage(self.browser)
+        self.assertEqual(problem_page.problem_text[0], u'question text')
+        problem_page.fill_answer('B')
+        problem_page.click_check()
+        self.assertEqual(problem_page.message_text, u'Incorrect: aa bb cc')
+        problem_page.fill_answer('C')
+        problem_page.click_check()
+        self.assertEqual(problem_page.message_text, u'Incorrect: aa bb cc')
+        # Check for corresponding tracking event
+        actual_events = self.wait_for_events(
+            event_filter={'event_type': 'edx.problem.hint.feedback_displayed'},
+            number_of_matches=2
+        )
+        self.assert_events_match(
+            [{'event': {'hint_label': u'Incorrect',
+                        'trigger_type': 'single',
+                        'student_answer': [u'B'],
+                        'correctness': False,
+                        'question_type': 'stringresponse',
+                        'hints': [{'text': 'aa <a href="#">bb</a> cc'}]}},
+             {'event': {'hint_label': u'Incorrect',
+                        'trigger_type': 'single',
+                        'student_answer': [u'C'],
+                        'correctness': False,
+                        'question_type': 'stringresponse',
+                        'hints': [{'text': '<a href="#">aa bb</a> cc'}]}}],
+            actual_events)
+
+    def test_demand_hint(self):
+        """
+        Test clicking hint button shows the demand hint in its div.
+        """
+        self.courseware_page.visit()
+        problem_page = ProblemPage(self.browser)
+        # The hint button rotates through multiple hints
+        problem_page.click_hint()
+        self.assertEqual(problem_page.hint_text, u'Hint (1 of 2): aa bb cc')
+        problem_page.click_hint()
+        self.assertEqual(problem_page.hint_text, u'Hint (2 of 2): dd ee ff')
+        problem_page.click_hint()
+        self.assertEqual(problem_page.hint_text, u'Hint (1 of 2): aa bb cc')
+        # Check corresponding tracking events
+        actual_events = self.wait_for_events(
+            event_filter={'event_type': 'edx.problem.hint.demandhint_displayed'},
+            number_of_matches=3
+        )
+        self.assert_events_match(
+            [
+                {'event': {u'hint_index': 0, u'hint_len': 2, u'hint_text': u'aa <a href="#">bb</a> cc'}},
+                {'event': {u'hint_index': 1, u'hint_len': 2, u'hint_text': u'<a href="#">dd  ee</a> ff'}},
+                {'event': {u'hint_index': 0, u'hint_len': 2, u'hint_text': u'aa <a href="#">bb</a> cc'}}
+            ],
+            actual_events)
+
+
 class ProblemWithMathjax(ProblemsTest):
     """
     Tests the <MathJax> used in problem