Remove sanitization from python & adding it to the templates.

TNL-4012

lms/djangoapps/edxnotes/helpers.py

@@ -20,7 +20,6 @@ from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext as _
 
 from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable
-from capa.util import sanitize_html
 from courseware.views import get_current_child
 from courseware.access import has_access
 from openedx.core.lib.token_utils import get_id_token
@@ -31,8 +30,6 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
 
 
 log = logging.getLogger(__name__)
-HIGHLIGHT_TAG = "span"
-HIGHLIGHT_CLASS = "note-highlight"
 # OAuth2 Client name for edxnotes
 CLIENT_NAME = "edx-notes"
 DEFAULT_PAGE = 1
@@ -92,9 +89,7 @@ def send_request(user, course_id, page, page_size, path="", text=None):
     if text:
         params.update({
             "text": text,
-            "highlight": True,
-            "highlight_tag": HIGHLIGHT_TAG,
-            "highlight_class": HIGHLIGHT_CLASS,
+            "highlight": True
         })
 
     try:
@@ -146,12 +141,9 @@ def preprocess_collection(user, course, collection):
     with store.bulk_operations(course.id):
         for model in collection:
             update = {
-                u"text": sanitize_html(model["text"]),
-                u"quote": sanitize_html(model["quote"]),
                 u"updated": dateutil_parse(model["updated"]),
             }
-            if "tags" in model:
-                update[u"tags"] = [sanitize_html(tag) for tag in model["tags"]]
+
             model.update(update)
             usage_id = model["usage_id"]
             if usage_id in cache:

lms/djangoapps/edxnotes/tests.py

@@ -525,43 +525,6 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
             json.loads(helpers.get_notes(self.request, self.course))
         )
 
-    def test_preprocess_collection_escaping(self):
-        """
-        Tests the result if appropriate module is not found.
-        """
-        initial_collection = [{
-            u"quote": u"test <script>alert('test')</script>",
-            u"text": u"text \"<>&'",
-            u"usage_id": unicode(self.html_module_1.location),
-            u"updated": datetime(2014, 11, 19, 8, 5, 16, 00000).isoformat()
-        }]
-
-        self.assertItemsEqual(
-            [{
-                u"quote": u"test &lt;script&gt;alert('test')&lt;/script&gt;",
-                u"text": u'text "&lt;&gt;&amp;\'',
-                u"chapter": {
-                    u"display_name": self.chapter.display_name_with_default_escaped,
-                    u"index": 0,
-                    u"location": unicode(self.chapter.location),
-                    u"children": [unicode(self.sequential.location)]
-                },
-                u"section": {
-                    u"display_name": self.sequential.display_name_with_default_escaped,
-                    u"location": unicode(self.sequential.location),
-                    u"children": [unicode(self.vertical.location), unicode(self.vertical_with_container.location)]
-                },
-                u"unit": {
-                    u"url": self._get_unit_url(self.course, self.chapter, self.sequential),
-                    u"display_name": self.vertical.display_name_with_default_escaped,
-                    u"location": unicode(self.vertical.location),
-                },
-                u"usage_id": unicode(self.html_module_1.location),
-                u"updated": datetime(2014, 11, 19, 8, 5, 16, 00000),
-            }],
-            helpers.preprocess_collection(self.user, self.course, initial_collection)
-        )
-
     def test_preprocess_collection_no_item(self):
         """
         Tests the result if appropriate module is not found.
@@ -766,8 +729,6 @@ class EdxNotesHelpersTest(ModuleStoreTestCase):
                 "course_id": unicode(self.course.id),
                 "text": "text",
                 "highlight": True,
-                "highlight_tag": "span",
-                "highlight_class": "note-highlight",
                 'page': 1,
                 'page_size': 10,
             }

lms/static/js/spec/edxnotes/views/note_item_spec.js

@@ -9,14 +9,14 @@ define([
 ) {
     'use strict';
     describe('EdxNotes NoteItemView', function() {
-        var getView = function (model, scrollToTag) {
+        var getView = function (model, scrollToTag, formattedText) {
             model = new NoteModel(_.defaults(model || {}, {
                 id: 'id-123',
                 user: 'user-123',
                 usage_id: 'usage_id-123',
                 created: 'December 11, 2014 at 11:12AM',
                 updated: 'December 11, 2014 at 11:12AM',
-                text: 'Third added model',
+                text: formattedText || 'Third added model',
                 quote: Helpers.LONG_TEXT,
                 unit: {
                     url: 'http://example.com/'
@@ -72,6 +72,36 @@ define([
             expect(view.$('span.reference-tags')[1]).toContainText('Second');
         });
 
+        it('should highlight tags & text if they have elasticsearch formatter', function() {
+            var view = getView({
+                tags: ["First", "{elasticsearch_highlight_start}Second{elasticsearch_highlight_end}"]
+            }, {}, "{elasticsearch_highlight_start}Sample{elasticsearch_highlight_end}");
+            expect(view.$('.reference-title').length).toBe(3);
+            expect(view.$('.reference-title')[2]).toContainText('Tags:');
+            expect(view.$('span.reference-tags').length).toBe(2);
+            expect(view.$('span.reference-tags')[0]).toContainText('First');
+            // highlighted tag & text
+            expect($.trim($(view.$('span.reference-tags')[1]).html())).toBe(
+                '<span class="note-highlight">Second</span>'
+            );
+            expect($.trim(view.$('.note-comment-p').html())).toBe('<span class="note-highlight">Sample</span>');
+        });
+
+        it('should escape html for tags & comments', function() {
+            var view = getView({
+                tags: ["First", "<b>Second</b>", "ȗnicode"]
+            }, {}, "<b>Sample</b>");
+            expect(view.$('.reference-title').length).toBe(3);
+            expect(view.$('.reference-title')[2]).toContainText('Tags:');
+            expect(view.$('span.reference-tags').length).toBe(3);
+            expect(view.$('span.reference-tags')[0]).toContainText('First');
+            expect($.trim($(view.$('span.reference-tags')[1]).html())).toBe(
+                '&lt;b&gt;Second&lt;/b&gt;'
+            );
+            expect($.trim($(view.$('span.reference-tags')[2]).html())).toBe('ȗnicode');
+            expect($.trim(view.$('.note-comment-p').html())).toBe('&lt;b&gt;Sample&lt;/b&gt;');
+        });
+
         xit('should handle a click event on the tag', function() {
             var scrollToTagSpy = {
                 scrollToTag: function (tagName){}

lms/templates/edxnotes/note-item.underscore

@@ -1,7 +1,7 @@
 <div class="wrapper-note-excerpts">
     <% if (message) { %>
     <div class="note-excerpt" role="region" aria-label="<%- gettext('Highlighted text') %>">
-        <p class="note-excerpt-p"><%= message %>
+        <p class="note-excerpt-p"><%- message %>
           <% if (show_link) { %>
           <% if (is_expanded) { %>
               <a href="#" class="note-excerpt-more-link"><%- gettext('Less') %></a>
@@ -17,7 +17,12 @@
     <ol class="note-comments" role="region" aria-label="<%- gettext('Note') %>">
         <li class="note-comment">
             <p class="note-comment-title"><%- gettext("You commented...") %></p>
-            <p class="note-comment-p"><%= text %></p>
+            <p class="note-comment-p">
+                <%= interpolate_text(_.escape(text), {
+                    elasticsearch_highlight_start: '<span class="note-highlight">',
+                    elasticsearch_highlight_end: '</span>'
+                })%>
+            </p>
         </li>
     </ol>
     <% } %>
@@ -38,7 +43,12 @@
         <% if (tags.length > 0) { %>
             <p class="reference-title"><%- gettext("Tags:") %></p>
             <% for (var i = 0; i < tags.length; i++) { %>
-                <span class="reference-meta reference-tags"><%= tags[i] %></span>
+                <span class="reference-meta reference-tags">
+                    <%= interpolate_text(_.escape(tags[i]), {
+                        elasticsearch_highlight_start: '<span class="note-highlight">',
+                        elasticsearch_highlight_end: '</span>'
+                    })%>
+                </span>
             <% } %>
         <% } %>
     </div>