Merge pull request #6296 from edx/dhm/studio_asides

make asides render in studio
This commit is contained in:
Don Mitchell
2014-12-18 15:41:58 -05:00
18 changed files with 201 additions and 42 deletions

View File

@@ -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):

View File

@@ -33,6 +33,7 @@ from .session_kv_store import SessionKeyValueStore
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 @@ 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 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):

View File

@@ -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 @@ 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, '<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")

View File

View File

@@ -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)

View File

@@ -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']

View File

@@ -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()

View File

@@ -581,6 +581,7 @@ INSTALLED_APPS = (
'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',

View File

@@ -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 []

View File

@@ -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'):

View File

@@ -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):
"""

View File

@@ -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)

View File

@@ -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):
"""

View File

@@ -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,

View File

@@ -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

View File

@@ -122,10 +122,13 @@ class AcceptanceTestSuite(TestSuite):
# 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))

View File

@@ -39,6 +39,7 @@ if [[ -f $DB_CACHE_DIR/bok_choy_schema.sql && -f $DB_CACHE_DIR/bok_choy_data.jso
# Re-run migrations to ensure we are up-to-date
./manage.py lms --settings bok_choy migrate --traceback --noinput
./manage.py cms --settings bok_choy migrate --traceback --noinput
# Otherwise, update the test database and update the cache
else
@@ -48,7 +49,9 @@ else
# Re-run migrations on the test database
./manage.py lms --settings bok_choy syncdb --traceback --noinput
./manage.py cms --settings bok_choy syncdb --traceback --noinput
./manage.py lms --settings bok_choy migrate --traceback --noinput
./manage.py cms --settings bok_choy migrate --traceback --noinput
# Dump the schema and data to the cache
./manage.py lms --settings bok_choy dumpdata > $DB_CACHE_DIR/bok_choy_data.json