diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 9025fc3609..e4e0fbc6e2 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/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: diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 7292dd6d0d..678e67a343 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/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 ", - 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 <script>alert('test')</script>", - u"text": u'text "<>&\'', - 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, } diff --git a/lms/static/js/spec/edxnotes/views/note_item_spec.js b/lms/static/js/spec/edxnotes/views/note_item_spec.js index 27e7018518..84ea75a39f 100644 --- a/lms/static/js/spec/edxnotes/views/note_item_spec.js +++ b/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( + 'Second' + ); + expect($.trim(view.$('.note-comment-p').html())).toBe('Sample'); + }); + + it('should escape html for tags & comments', function() { + var view = getView({ + tags: ["First", "Second", "ȗnicode"] + }, {}, "Sample"); + 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( + '<b>Second</b>' + ); + expect($.trim($(view.$('span.reference-tags')[2]).html())).toBe('ȗnicode'); + expect($.trim(view.$('.note-comment-p').html())).toBe('<b>Sample</b>'); + }); + xit('should handle a click event on the tag', function() { var scrollToTagSpy = { scrollToTag: function (tagName){} diff --git a/lms/templates/edxnotes/note-item.underscore b/lms/templates/edxnotes/note-item.underscore index df571989f5..9aaceb98d1 100644 --- a/lms/templates/edxnotes/note-item.underscore +++ b/lms/templates/edxnotes/note-item.underscore @@ -1,7 +1,7 @@
<%= message %> +
<%- message %> <% if (show_link) { %> <% if (is_expanded) { %> <%- gettext('Less') %> @@ -17,7 +17,12 @@
<%- gettext("You commented...") %>
-<%= text %>
++ <%= interpolate_text(_.escape(text), { + elasticsearch_highlight_start: '', + elasticsearch_highlight_end: '' + })%> +
<%- gettext("Tags:") %>
<% for (var i = 0; i < tags.length; i++) { %> - + <% } %> <% } %>