From 4360a78142df2d7aadb1ff2846dfffef6fcee535 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Tue, 16 Dec 2014 14:42:33 -0500 Subject: [PATCH] Studio render XBlockAside views PLAT-280 --- cms/djangoapps/contentstore/views/item.py | 3 +-- cms/djangoapps/contentstore/views/preview.py | 12 ++++++++--- .../contentstore/views/tests/test_preview.py | 13 ++++++++++-- cms/lib/xblock/runtime.py | 8 -------- .../xmodule/xmodule/modulestore/mongo/base.py | 4 ++++ .../xmodule/modulestore/tests/test_asides.py | 2 +- .../modulestore/tests/test_libraries.py | 2 +- common/lib/xmodule/xmodule/x_module.py | 20 ++++--------------- .../instructor/views/instructor_dashboard.py | 6 +++--- lms/djangoapps/lms_xblock/runtime.py | 1 - 10 files changed, 34 insertions(+), 37 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 6de67226df..28c9a5e480 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -45,7 +45,7 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primar from contentstore.views.preview import get_preview_fragment from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel -from cms.lib.xblock.runtime import handler_url, local_resource_url, applicable_aside_types +from cms.lib.xblock.runtime import handler_url, local_resource_url from opaque_keys.edx.keys import UsageKey, CourseKey __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -64,7 +64,6 @@ ALWAYS = lambda x: True # TODO: Remove this code when Runtimes are no longer created by modulestores xmodule.x_module.descriptor_global_handler_url = handler_url xmodule.x_module.descriptor_global_local_resource_url = local_resource_url -xmodule.x_module.descriptor_global_applicable_aside_types = applicable_aside_types def hash_resource(resource): diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 4dc7e07422..f6fba13c68 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -87,7 +87,7 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): return reverse('preview_handler', kwargs={ - 'usage_key_string': unicode(block.location), + 'usage_key_string': unicode(block.scope_ids.usage_id), 'handler': handler_name, 'suffix': suffix, }) + '?' + query @@ -96,8 +96,14 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method return local_resource_url(block, uri) def applicable_aside_types(self, block): - # TODO: Implement this to enable XBlockAsides on previews in Studio - return [] + """ + Remove acid_aside + """ + return [ + aside_type + for aside_type in super(PreviewModuleSystem, self).applicable_aside_types(block) + if aside_type != 'acid_aside' + ] class StudioUserService(object): diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index b661c97e87..6631daace4 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -9,6 +9,10 @@ from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from contentstore.views.preview import get_preview_fragment +from xmodule.modulestore import ModuleStoreEnum +from xblock.core import XBlockAside +from xmodule.modulestore.tests.test_asides import AsideTestType +import re class GetPreviewHtmlTestCase(TestCase): @@ -19,6 +23,7 @@ class GetPreviewHtmlTestCase(TestCase): get_preview_fragment via the xblock RESTful API. """ + @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') def test_preview_fragment(self): """ Test for calling get_preview_html. @@ -26,7 +31,7 @@ class GetPreviewHtmlTestCase(TestCase): This test used to be specifically about Locators (ensuring that they did not get translated to Locations). The test now has questionable value. """ - course = CourseFactory.create() + course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) html = ItemFactory.create( parent_location=course.location, category="html", @@ -45,9 +50,13 @@ class GetPreviewHtmlTestCase(TestCase): html = get_preview_fragment(request, html, context).content # Verify student view html is returned, and the usage ID is as expected. - html_pattern = unicode(course.id.make_usage_key('html', 'html_')).replace('html_', r'html_[0-9]*') + html_pattern = re.escape(unicode(course.id.make_usage_key('html', 'replaceme'))).replace('replaceme', r'html_[0-9]*') self.assertRegexpMatches( html, 'data-usage-id="{}"'.format(html_pattern) ) self.assertRegexpMatches(html, 'foobar') + self.assertRegexpMatches(html, r"data-block-type=[\"\']test_aside[\"\']") + self.assertRegexpMatches(html, "Aside rendered") + # Now ensure the acid_aside is not in the result + self.assertNotRegexpMatches(html, r"data-block-type=[\"\']acid_aside[\"\']") diff --git a/cms/lib/xblock/runtime.py b/cms/lib/xblock/runtime.py index 5048f422b3..b460f175c2 100644 --- a/cms/lib/xblock/runtime.py +++ b/cms/lib/xblock/runtime.py @@ -33,11 +33,3 @@ def local_resource_url(block, uri): 'block_type': block.scope_ids.block_type, 'uri': uri, }) - - -def applicable_aside_types(block): # pylint: disable=unused-argument - """ - Get the application-relative list of aside types for this type of block. - """ - # TODO: Implement this method to make XBlockAsides for editing views in Studio - return [] diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 04123f0579..f417086274 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -356,6 +356,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): """ return xblock._edit_info.get('published_date') + def applicable_aside_types(self, block): + # "old" mongo does support asides yet + return [] + # The only thing using this w/ wildcards is contentstore.mongo for asset retrieval def location_to_query(location, wildcard=True, tag='i4x'): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py b/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py index 5aa86cb59b..4a9e98114c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py @@ -28,7 +28,7 @@ class TestAsidesXmlStore(TestCase): """ Test Asides sourced from xml store """ - @patch('xmodule.x_module.descriptor_global_applicable_aside_types', lambda block: ['test_aside']) + @patch('xmodule.modulestore.xml.ImportSystem.applicable_aside_types', lambda self, block: ['test_aside']) @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') def test_xml_aside(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py index 0e61e325be..252d984622 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -203,6 +203,6 @@ class TestLibraries(MixedSplitTestCase): message = u"Hello world" hello_render = lambda _, context: Fragment(message) with patch('xmodule.html_module.HtmlDescriptor.author_view', hello_render, create=True): - with patch('xmodule.x_module.descriptor_global_applicable_aside_types', lambda block: []): + with patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []): result = library.render(AUTHOR_VIEW, context) self.assertIn(message, result.content) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index e660211fd5..a4943fc83d 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1133,15 +1133,6 @@ def descriptor_global_local_resource_url(block, uri): # pylint: disable=invalid raise NotImplementedError("Applications must monkey-patch this function before using local_resource_url for studio_view") -# pylint: disable=invalid-name -def descriptor_global_applicable_aside_types(block): # pylint: disable=unused-argument - """ - See :meth:`xblock.runtime.Runtime.applicable_aside_types`. - """ - raise NotImplementedError("Applications must monkey-patch this function before using applicable_aside_types" - " from a DescriptorSystem.") - - class MetricsMixin(object): """ Mixin for adding metric logging for render and handle methods in the DescriptorSystem and ModuleSystem. @@ -1320,14 +1311,11 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p """ See :meth:`xblock.runtime.Runtime:applicable_aside_types` for documentation. """ + potential_set = set(super(DescriptorSystem, self).applicable_aside_types(block)) if getattr(block, 'xmodule_runtime', None) is not None: - return block.xmodule_runtime.applicable_aside_types(block) - else: - # Currently, Modulestore is responsible for instantiating DescriptorSystems - # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem - # that implements the correct get_asides. So, for now, instead, we will reference a - # global function that the application can override. - return descriptor_global_applicable_aside_types(block) + application_set = set(block.xmodule_runtime.applicable_aside_types(block)) + return list(potential_set.intersection(application_set)) + return list(potential_set) def resource_url(self, resource): """ diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 6499886398..a32c864f95 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -326,7 +326,7 @@ def _section_data_download(course, access): def null_applicable_aside_types(block): # pylint: disable=unused-argument """ - get_aside method for monkey-patching into descriptor_global_applicable_aside_types + get_aside method for monkey-patching into applicable_aside_types while rendering an HtmlDescriptor for email text editing. This returns an empty list. """ @@ -337,8 +337,8 @@ def _section_send_email(course, access): """ Provide data for the corresponding bulk email section """ course_key = course.id - # Monkey-patch descriptor_global_applicable_aside_types to return no asides for the duration of this render - with patch('xmodule.x_module.descriptor_global_applicable_aside_types', null_applicable_aside_types): + # Monkey-patch applicable_aside_types to return no asides for the duration of this render + with patch.object(course.runtime, 'applicable_aside_types', null_applicable_aside_types): # This HtmlDescriptor is only being used to generate a nice text editor. html_module = HtmlDescriptor( course.system, diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index c319370e99..c85e39eeeb 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -9,7 +9,6 @@ from django.core.urlresolvers import reverse from django.conf import settings from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from openedx.core.djangoapps.user_api.api import course_tag as user_course_tag_api -from xblock.core import XBlockAside from xmodule.modulestore.django import modulestore from xmodule.x_module import ModuleSystem from xmodule.partitions.partitions_service import PartitionService