Merge pull request #6296 from edx/dhm/studio_asides

make asides render in studio
openedx · Dec 18, 2014 · d73eca4 · d73eca4
2 parents 8e14c9f + d49c82f
commit d73eca4
Show file tree

Hide file tree

Showing 18 changed files with 201 additions and 42 deletions.
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
@@ -45,7 +45,7 @@
 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 @@
 # 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
@@ -33,6 +33,7 @@
 from .helpers import render_from_lms
 
 from contentstore.views.access import get_user_role
+from cms.djangoapps.xblock_config.models import StudioConfig
 
 __all__ = ['preview_handler']
 
@@ -87,7 +88,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 +97,16 @@ def local_resource_url(self, block, uri):
         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 and honor the config record
+        """
+        if not StudioConfig.asides_enabled(block.scope_ids.block_type):
+            return []
+        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
@@ -1,14 +1,21 @@
 """
 Tests for contentstore.views.preview.py
 """
+import re
+
 from django.test import TestCase
 from django.test.client import RequestFactory
 
+from xblock.core import XBlockAside
 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 xmodule.modulestore.tests.test_asides import AsideTestType
+from cms.djangoapps.xblock_config.models import StudioConfig
+from xmodule.modulestore.django import modulestore
 
 
 class GetPreviewHtmlTestCase(TestCase):
@@ -18,21 +25,23 @@ class GetPreviewHtmlTestCase(TestCase):
     Note that there are other existing test cases in test_contentstore that indirectly execute
     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.
-
-        This test used to be specifically about Locators (ensuring that they did not
-        get translated to Locations). The test now has questionable value.
+        Test for calling get_preview_html. Ensures data-usage-id is correctly set and
+        asides are correctly included.
         """
-        course = CourseFactory.create()
+        course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split)
         html = ItemFactory.create(
             parent_location=course.location,
             category="html",
             data={'data': "<html>foobar</html>"}
         )
 
+        config = StudioConfig.current()
+        config.enabled = True
+        config.save()
+
         request = RequestFactory().get('/dummy-url')
         request.user = UserFactory()
         request.session = {}
@@ -45,9 +54,50 @@ def test_preview_fragment(self):
         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, '<html>foobar</html>')
+        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[\"\']")
+
+        # Ensure about pages don't have asides
+        about = modulestore().get_item(course.id.make_usage_key('about', 'overview'))
+        html = get_preview_fragment(request, about, context).content
+        self.assertNotRegexpMatches(html, r"data-block-type=[\"\']test_aside[\"\']")
+        self.assertNotRegexpMatches(html, "Aside rendered")
+
+    @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside')
+    def test_preview_no_asides(self):
+        """
+        Test for calling get_preview_html. Ensures data-usage-id is correctly set and
+        asides are correctly excluded because they are not enabled.
+        """
+        course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split)
+        html = ItemFactory.create(
+            parent_location=course.location,
+            category="html",
+            data={'data': "<html>foobar</html>"}
+        )
+
+        config = StudioConfig.current()
+        config.enabled = False
+        config.save()
+
+        request = RequestFactory().get('/dummy-url')
+        request.user = UserFactory()
+        request.session = {}
+
+        # Call get_preview_fragment directly.
+        context = {
+            'reorderable_items': set(),
+            'read_only': True
+        }
+        html = get_preview_fragment(request, html, context).content
+
+        self.assertNotRegexpMatches(html, r"data-block-type=[\"\']test_aside[\"\']")
+        self.assertNotRegexpMatches(html, "Aside rendered")
diff --git a/cms/djangoapps/xblock_config/__init__.py b/cms/djangoapps/xblock_config/__init__.py
diff --git a/cms/djangoapps/xblock_config/admin.py b/cms/djangoapps/xblock_config/admin.py
@@ -0,0 +1,9 @@
+"""
+Django admin dashboard configuration for LMS XBlock infrastructure.
+"""
+
+from django.contrib import admin
+from config_models.admin import ConfigurationModelAdmin
+from cms.djangoapps.xblock_config.models import StudioConfig
+
+admin.site.register(StudioConfig, ConfigurationModelAdmin)
diff --git a/cms/djangoapps/xblock_config/migrations/0001_initial.py b/cms/djangoapps/xblock_config/migrations/0001_initial.py
@@ -0,0 +1,74 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding model 'StudioConfig'
+        db.create_table('xblock_config_studioconfig', (
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('change_date', self.gf('django.db.models.fields.DateTimeField')(auto_now_add=True, blank=True)),
+            ('changed_by', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'], null=True, on_delete=models.PROTECT)),
+            ('enabled', self.gf('django.db.models.fields.BooleanField')(default=False)),
+            ('disabled_blocks', self.gf('django.db.models.fields.TextField')(default='about course_info static_tab')),
+        ))
+        db.send_create_signal('xblock_config', ['StudioConfig'])
+
+
+    def backwards(self, orm):
+        # Deleting model 'StudioConfig'
+        db.delete_table('xblock_config_studioconfig')
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'xblock_config.studioconfig': {
+            'Meta': {'object_name': 'StudioConfig'},
+            'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}),
+            'disabled_blocks': ('django.db.models.fields.TextField', [], {'default': "'about course_info static_tab'"}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        }
+    }
+
+    complete_apps = ['xblock_config']
diff --git a/cms/djangoapps/xblock_config/migrations/__init__.py b/cms/djangoapps/xblock_config/migrations/__init__.py
diff --git a/cms/djangoapps/xblock_config/models.py b/cms/djangoapps/xblock_config/models.py
@@ -0,0 +1,28 @@
+"""
+Models used by Studio XBlock infrastructure.
+
+Includes:
+    StudioConfig: A ConfigurationModel for managing Studio.
+"""
+
+from django.db.models import TextField
+
+from config_models.models import ConfigurationModel
+
+
+class StudioConfig(ConfigurationModel):
+    """
+    Configuration for XBlockAsides.
+    """
+    disabled_blocks = TextField(
+        default="about course_info static_tab",
+        help_text="Space-separated list of XBlocks on which XBlockAsides should never render in studio",
+    )
+
+    @classmethod
+    def asides_enabled(cls, block_type):
+        """
+        Return True if asides are enabled for this type of block in studio
+        """
+        studio_config = cls.current()
+        return studio_config.enabled and block_type not in studio_config.disabled_blocks.split()
diff --git a/cms/envs/common.py b/cms/envs/common.py
@@ -581,6 +581,7 @@
     'course_creators',
     'student',  # misleading name due to sharing with lms
     'openedx.core.djangoapps.course_groups',  # not used in cms (yet), but tests run
+    'xblock_config',
 
     # Tracking
     'track',

diff --git 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
@@ -356,6 +356,10 @@ def get_published_on(self, xblock):
         """
         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
@@ -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
@@ -203,6 +203,6 @@ def test_library_author_view(self):
         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
@@ -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 @@ def applicable_aside_types(self, block):
         """
         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
@@ -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
@@ -9,7 +9,6 @@
 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

diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py
@@ -122,10 +122,13 @@ def _setup_acceptance_db(self):
 
             # Run migrations to update the db, starting from its cached state
             sh("./manage.py lms --settings acceptance migrate --traceback --noinput")
+            sh("./manage.py cms --settings acceptance migrate --traceback --noinput")
         else:
             # If no cached database exists, syncdb before migrating, then create the cache
             sh("./manage.py lms --settings acceptance syncdb --traceback --noinput")
+            sh("./manage.py cms --settings acceptance syncdb --traceback --noinput")
             sh("./manage.py lms --settings acceptance migrate --traceback --noinput")
+            sh("./manage.py cms --settings acceptance migrate --traceback --noinput")
 
             # Create the cache if it doesn't already exist
             sh("cp {db} {db_cache}".format(db_cache=self.db_cache, db=self.db))