From b28af4fdaa9599be9ca2986aade108dfe85b14fe Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 4 Sep 2014 17:20:09 -0400 Subject: [PATCH 1/5] Temporarily disable bulk operations b/c old mongo does too much work on exit and doesn't handle nesting --- cms/djangoapps/contentstore/views/course.py | 2 +- common/lib/xmodule/xmodule/modulestore/__init__.py | 8 ++++++++ common/lib/xmodule/xmodule/modulestore/search.py | 3 ++- .../xmodule/modulestore/tests/test_mixed_modulestore.py | 3 ++- lms/djangoapps/courseware/model_data.py | 3 ++- 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 8ba35628c2..93f0c9c6cf 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -423,7 +423,7 @@ def course_index(request, course_key): """ # A depth of None implies the whole course. The course outline needs this in order to compute has_changes. # A unit may not have a draft version, but one of its components could, and hence the unit itself has changes. - with modulestore().bulk_operations(course_key): + with modulestore().bulk_temp_noop_operations(course_key): # FIXME course_module = _get_course_module(course_key, request.user, depth=None) lms_link = get_lms_link_for_item(course_module.location) sections = course_module.get_children() diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 2bacbd6814..01b370d62c 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -566,6 +566,14 @@ class ModuleStoreReadBase(ModuleStoreRead): finally: self._end_bulk_operation(course_id) + @contextmanager + def bulk_temp_noop_operations(self, course_id): + """ + A hotfix noop b/c old mongo does not properly handle nested bulk operations and does unnecessary work + if the bulk operation only reads data. Replace with bulk_operations once fixed (or don't merge to master) + """ + yield + def _begin_bulk_operation(self, course_id): """ Begin a bulk write operation on course_id. diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py index 8854aa1865..43fe6878b9 100644 --- a/common/lib/xmodule/xmodule/modulestore/search.py +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -68,7 +68,8 @@ def path_to_location(modulestore, usage_key): newpath = (next_usage, path) queue.append((parent, newpath)) - with modulestore.bulk_operations(usage_key.course_key): + # FIXME replace with bulk_operations once it's fixed for old mongo + with modulestore.bulk_temp_noop_operations(usage_key.course_key): if not modulestore.has_item(usage_key): raise ItemNotFoundError(usage_key) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 4686ec3890..0b26953974 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -714,7 +714,8 @@ class TestMixedModuleStore(unittest.TestCase): # TODO: LMS-11220: Document why draft send count is 5 # TODO: LMS-11220: Document why draft find count is [19, 6] # TODO: LMS-11220: Document why split find count is [2, 2] - @ddt.data(('draft', [19, 6], 0), ('split', [2, 2], 0)) +# @ddt.data(('draft', [19, 6], 0), ('split', [2, 2], 0)) + @ddt.data(('draft', [18, 6], 0), ('split', [16, 2], 0)) # FIXME, replace w/ above when bulk reenabled @ddt.unpack def test_path_to_location(self, default_ms, num_finds, num_sends): """ diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index b7bb361a57..b3345513cd 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -110,7 +110,8 @@ class FieldDataCache(object): return descriptors - with modulestore().bulk_operations(descriptor.location.course_key): + # FIXME + with modulestore().bulk_temp_noop_operations(descriptor.location.course_key): descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) return FieldDataCache(descriptors, course_id, user, select_for_update) From 041d72ccc632c9791768a5705c5958b4a1f63240 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 4 Sep 2014 18:14:13 -0400 Subject: [PATCH 2/5] Update mongo counts. --- .../xmodule/modulestore/tests/test_mixed_modulestore.py | 2 +- lms/djangoapps/courseware/tests/test_module_render.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 0b26953974..6d0b59491d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -715,7 +715,7 @@ class TestMixedModuleStore(unittest.TestCase): # TODO: LMS-11220: Document why draft find count is [19, 6] # TODO: LMS-11220: Document why split find count is [2, 2] # @ddt.data(('draft', [19, 6], 0), ('split', [2, 2], 0)) - @ddt.data(('draft', [18, 6], 0), ('split', [16, 2], 0)) # FIXME, replace w/ above when bulk reenabled + @ddt.data(('draft', [18, 5], 0), ('split', [16, 6], 0)) # FIXME, replace w/ above when bulk reenabled @ddt.unpack def test_path_to_location(self, default_ms, num_finds, num_sends): """ diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index aa223d724f..35b33e18f7 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -335,7 +335,7 @@ class TestTOC(ModuleStoreTestCase): # TODO: LMS-11220: Document why split find count is 9 # TODO: LMS-11220: Document why mongo find count is 4 - @ddt.data((ModuleStoreEnum.Type.mongo, 4, 0), (ModuleStoreEnum.Type.split, 9, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0), (ModuleStoreEnum.Type.split, 21, 0)) @ddt.unpack def test_toc_toy_from_chapter(self, default_ms, num_finds, num_sends): with self.store.default_store(default_ms): @@ -364,7 +364,7 @@ class TestTOC(ModuleStoreTestCase): # TODO: LMS-11220: Document why split find count is 9 # TODO: LMS-11220: Document why mongo find count is 4 - @ddt.data((ModuleStoreEnum.Type.mongo, 4, 0), (ModuleStoreEnum.Type.split, 9, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0), (ModuleStoreEnum.Type.split, 21, 0)) @ddt.unpack def test_toc_toy_from_section(self, default_ms, num_finds, num_sends): with self.store.default_store(default_ms): From 568ef4184a7faceb4594221777027622856da2bb Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Sep 2014 15:57:32 -0400 Subject: [PATCH 3/5] remove course-id visual column from the list of coupons as it's useless information here and prevents overflows from happening --- lms/templates/instructor/instructor_dashboard_2/e-commerce.html | 2 -- 1 file changed, 2 deletions(-) diff --git a/lms/templates/instructor/instructor_dashboard_2/e-commerce.html b/lms/templates/instructor/instructor_dashboard_2/e-commerce.html index 055256f68b..ba306a323d 100644 --- a/lms/templates/instructor/instructor_dashboard_2/e-commerce.html +++ b/lms/templates/instructor/instructor_dashboard_2/e-commerce.html @@ -99,7 +99,6 @@ ${_("Code")} ${_("Description")} - ${_("Course_id")} ${_("Discount (%)")} ${_("Count")} ${_("Actions")} @@ -114,7 +113,6 @@ %endif ${coupon.code} ${coupon.description} - ${coupon.course_id.to_deprecated_string()} ${coupon.percentage_discount} ${ coupon.couponredemption_set.all().count() } From 405baab82af086389d34ae9282815bbef9310d00 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 4 Sep 2014 15:32:43 -0400 Subject: [PATCH 4/5] don't hard code the shoppingcart order confirmation page to use settings.xxx, allow for microsites to override --- lms/templates/shoppingcart/receipt.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/templates/shoppingcart/receipt.html b/lms/templates/shoppingcart/receipt.html index f86df03bae..7a35bc014f 100644 --- a/lms/templates/shoppingcart/receipt.html +++ b/lms/templates/shoppingcart/receipt.html @@ -1,6 +1,7 @@ <%! from django.utils.translation import ugettext as _ %> <%! from django.core.urlresolvers import reverse %> <%! from django.conf import settings %> +<%! from microsite_configuration import microsite %> <%inherit file="../main.html" /> @@ -22,7 +23,7 @@
-

${_(settings.PLATFORM_NAME + " (" + settings.SITE_NAME + ")" + " Electronic Receipt")}

+

${_("{platform_name} ({site_name}) Electronic Receipt").format(platform_name=microsite.get_value('platform_name', settings.PLATFORM_NAME), site_name=microsite.get_value('SITE_NAME', settings.SITE_NAME))}

From 58553a7fb8fbc33dfd40c8e7ca39b0f896fb5b32 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Wed, 3 Sep 2014 13:32:35 -0400 Subject: [PATCH 5/5] escape html for inline discussions (TNL-182) --- .../view/discussion_thread_view_spec.coffee | 26 +++++++++++++++++++ .../views/discussion_thread_view.coffee | 4 +-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee index 9c42456d86..a6d018b9bf 100644 --- a/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee +++ b/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee @@ -124,6 +124,32 @@ describe "DiscussionThreadView", -> expect($(".post-body").text()).toEqual(expectedAbbreviation) expect(DiscussionThreadShowView.prototype.convertMath).toHaveBeenCalled() + it "strips script tags appropriately", -> + DiscussionViewSpecHelper.setNextResponseContent({resp_total: 0, children: []}) + longMaliciousBody = new Array(100).join("\n") + @thread.set("body", longMaliciousBody) + maliciousAbbreviation = DiscussionUtil.abbreviateString(@thread.get('body'), 140) + + # The nodes' html should be different than the strings, but + # their texts should be the same, indicating that they've been + # properly escaped. To be safe, make sure the string " beforeEach -> @thread.set("thread_type", "question") diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee index 5d312051ed..eacbd5e866 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -62,7 +62,7 @@ if Backbone? if event event.preventDefault() @$el.addClass("expanded") - @$el.find(".post-body").html(@model.get("body")) + @$el.find(".post-body").text(@model.get("body")) @showView.convertMath() @$el.find(".forum-thread-expand").hide() @$el.find(".forum-thread-collapse").show() @@ -74,7 +74,7 @@ if Backbone? if event event.preventDefault() @$el.removeClass("expanded") - @$el.find(".post-body").html(@getAbbreviatedBody()) + @$el.find(".post-body").text(@getAbbreviatedBody()) @showView.convertMath() @$el.find(".forum-thread-expand").show() @$el.find(".forum-thread-collapse").hide()